-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[receiver/windowsperfcountersreceiver] Update how metrics are established #8376
[receiver/windowsperfcountersreceiver] Update how metrics are established #8376
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally we would extract the metric metadata structs from mdatagen
so that this component is not just approximating the functionality. However, at a glance this appears not to be very straightforward. I think we should at least open a tracking issue to capture the intention though.
receiver/windowsperfcountersreceiver/windowsperfcounters_scraper.go
Outdated
Show resolved
Hide resolved
d0d278d
to
7ecedb6
Compare
receiver/windowsperfcountersreceiver/windowsperfcounters_scraper.go
Outdated
Show resolved
Hide resolved
receiver/windowsperfcountersreceiver/windowsperfcounters_scraper.go
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM. Thanks for all the iteration @Mrod1598
CHANGELOG.md
Outdated
@@ -29,6 +29,7 @@ | |||
|
|||
### 🛑 Breaking changes 🛑 | |||
|
|||
- `windowsperfcountersreceiver`: Added metrics configuration (#8376) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like I can't change this PR, so I can't fix this for you, but this now needs to be moved up to the "unreleased" section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
To the next reviewer: feel free to dismiss my review once the changelog is fixed. |
Description:
Updated the receiver to enable metrics configuration. Additionally, once the
mdatagen
package makes the applicable structs public, this receiver will be updated to use those.Link to tracking Issue:
#8259
Testing:
Along side the updated config options, additional tests were added to test the new config options. Also, updated the the scraper tests to test the different metric formats.
Documentation:
Updated the documentation to use the new config options and describe where to use them.