-
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/hostmetrics]: change uptime scraper to system scraper #36123
Conversation
This PR makes some changes that I had hoped to suggest on the PR that introduced the uptime scraper, however I was unable to make my comment in time before the PR was merged. * The `uptime` scraper is changed to be named `system` * The scraper is registered using the scraper name when adding to the scrapercontroller
I am ideally hoping that this change makes it in before a new OTel Collector is tagged so that no users start using |
I would like to skip changelog, because in this case what I did was modified the existing changelog to reflect this new change. |
@kevinnoel-be I am unable to officially add you as a reviewer on this PR, but if you have a second to take a look I would appreciate it! |
receiver/hostmetricsreceiver/internal/scraper/systemscraper/factory_test.go
Outdated
Show resolved
Hide resolved
@braydonk looks good to me except for the use of one (test) module where I think it should be testify instead? |
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.
ok, LGTM
…-telemetry#36123) <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description This PR makes some changes that I had hoped to suggest on open-telemetry#35954, however I was unable to make my comment in time before the PR was merged. The changes this PR makes are: * The `uptime` scraper is changed to be named `system` * The scraper is registered using the scraper name when adding to the scrapercontroller <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue Amends the fix to open-telemetry#31627 (following a comment I made on open-telemetry#35932) <!--Describe what testing was performed and which tests were added.--> #### Testing <!--Describe the documentation added.--> #### Documentation The documentation that was added for the `uptime` scraper was renamed accordingly, and the description adjusted. <!--Please delete paragraphs that you did not use before submitting.-->
…-telemetry#36123) <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description This PR makes some changes that I had hoped to suggest on open-telemetry#35954, however I was unable to make my comment in time before the PR was merged. The changes this PR makes are: * The `uptime` scraper is changed to be named `system` * The scraper is registered using the scraper name when adding to the scrapercontroller <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue Amends the fix to open-telemetry#31627 (following a comment I made on open-telemetry#35932) <!--Describe what testing was performed and which tests were added.--> #### Testing <!--Describe the documentation added.--> #### Documentation The documentation that was added for the `uptime` scraper was renamed accordingly, and the description adjusted. <!--Please delete paragraphs that you did not use before submitting.-->
Description
This PR makes some changes that I had hoped to suggest on #35954, however I was unable to make my comment in time before the PR was merged.
The changes this PR makes are:
uptime
scraper is changed to be namedsystem
Link to tracking issue
Amends the fix to #31627 (following a comment I made on #35932)
Testing
Documentation
The documentation that was added for the
uptime
scraper was renamed accordingly, and the description adjusted.