[scraper helper] improve ease of use of scraper controller settings #7951
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #7951 +/- ##
==========================================
- Coverage 90.96% 90.89% -0.07%
==========================================
Files 300 301 +1
Lines 15090 15126 +36
==========================================
+ Hits 13726 13749 +23
- Misses 1091 1105 +14
+ Partials 273 272 -1
☔ View full report in Codecov by Sentry. |
|
With making a breaking change that impacts contrib, what is the best way to order the PRs? Should those be raised after this has been merged? |
|
I think we should follow example 3 from https://github.com/open-telemetry/opentelemetry-collector/blob/main/CONTRIBUTING.md#example-3---changing-the-arguments-of-a-function |
MovieStoreGuy
force-pushed
the
msg/cleanup-scraper-helper
branch
from
6bfff54 to
d373a31
Compare
|
Updated PR based on comments in SIG meeting @mx-psi, @astencel-sumo . I will add Juraci's comment regarding internal metrics to track the scrape duration in the another PR. |
MovieStoreGuy
force-pushed
the
msg/cleanup-scraper-helper
branch
2 times, most recently
from
b2037ee to
86db65d
Compare
MovieStoreGuy
force-pushed
the
msg/cleanup-scraper-helper
branch
from
86db65d to
822463e
Compare
This timeout field can help ensure that scrapers have a max execution time.
MovieStoreGuy
force-pushed
the
msg/cleanup-scraper-helper
branch
from
822463e to
2dc6d98
Compare
|
Hey @djaglowski , I've updated the PR to reflect our discussion above, please let me know if there anything else I've missed. Once this is merged in, I can create a PR in contrib updating the implementing scrapers' docs. |
| } | ||
|
|
||
| func (set *ScraperControllerSettings) Validate() (errs error) { | ||
| if set.CollectionInterval <= 0 { |
There was a problem hiding this comment.
This makes the PR a breaking change, technically.
Signed-off-by: Juraci Paixão Kröhling <[email protected]>
**Description:** With the most recent changes in open-telemetry/opentelemetry-collector#7951 caused a bunch of tests to fail on upgrade. This just adds in the scraper settings where needed.
With the upcoming changes in open-telemetry/opentelemetry-collector#7951 where it includes timeout as an explicit field, chrony receiver is no longer required to explicitly call it.
Description:
Link to tracking Issue:
N/A
Testing:
Added tests for additional options
Documentation:
Documentation to be added to affected components in contrib