[scraperhelper] Add extension-based scraper controller support by axw · Pull Request #15116 · open-telemetry/opentelemetry-collector

axw · 2026-04-15T03:22:15Z

Description

Implement the extensionscrapercontroller package and integrate it with the scraper controller, allowing extensions to trigger scrapes based on external events instead of (or in addition to) a fixed timer interval.

Add extensionscrapercontroller.ControllerExtension interface and DeregisterFunc helper in extension/xextension/extensionscrapercontroller
Add Controllers field to ControllerConfig for specifying extension IDs
Allow CollectionInterval=0 when Controllers are configured
Register scrapers with controller extensions on Start, deregister on Shutdown
Update RFC to remove scraperID parameter from RegisterScraper, and only disable ticker on zero interval (negative interval is now rejected as before)

Assisted-by: Claude Opus 4.5

Link to tracking issue

Closes #12449

Testing

Tests added.

Documentation

https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/rfcs/scraper-controller-extension.md

codspeed-hq · 2026-04-15T03:30:47Z

Merging this PR will create unknown performance changes

⚠️

Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⏩ 4 skipped benchmarks¹
🗄️ 2 archived benchmarks run²

_{Comparing axw:extensionscrapercontroller (808e47d) with main (60a4f0b)}

4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
2 benchmarks were run, but are now archived. If they were deleted in another branch, consider rebasing to remove them from the report. Instead if they were added back, click here to restore them. ↩

codecov · 2026-04-15T03:33:52Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.12%. Comparing base (60a4f0b) to head (808e47d).

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #15116      +/-   ##
==========================================
+ Coverage   91.09%   91.12%   +0.02%     
==========================================
  Files         725      725              
  Lines       48066    48133      +67     
==========================================
+ Hits        43788    43861      +73     
+ Misses       2979     2975       -4     
+ Partials     1299     1297       -2

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Add tests to the internal/controller package, so we have baseline coverage before making the changes in open-telemetry#15116.

#### Description Add tests to the internal/controller package, so we have baseline coverage before making the changes in #15116. #### Link to tracking issue N/A #### Testing Yes #### Documentation No Co-authored-by: Alex Boten <223565+codeboten@users.noreply.github.com>

#### Description Various changes to newTestController to simplify tests: - newTestController no longer accepts a ticker, it always calls NewController with a nil ticker so one is created internally; the tests now use synctest where relevant. - newTestController no longer conditionally creates a no-op scrapeFunc, we introduce a nopScrapeFunc function and update relevant tests to pass it in. - scrapers has been made the final parameter, and is now variadic. Tests no longer need to explicitly create a slice. TestStartScrapingWithNilTickerCh has been removed, since that is now covered by the use of synctest. Several tests has been updated to use newTestController rather than calling NewController directly. #### Link to tracking issue Prep for open-telemetry#15116 (trying to cut the diff down) #### Testing Yes #### Documentation No Co-authored-by: Alex Boten <223565+codeboten@users.noreply.github.com>

Implement the extensionscrapercontroller package and integrate it with the scraper controller, allowing extensions to trigger scrapes based on external events instead of (or in addition to) a fixed timer interval. - Add extensionscrapercontroller.ControllerExtension interface and DeregisterFunc helper in extension/xextension/extensionscrapercontroller - Add Controllers field to ControllerConfig for specifying extension IDs - Allow CollectionInterval=0 when Controllers are configured - Register scrapers with controller extensions on Start, deregister on Shutdown - Update RFC to remove scraperID parameter from RegisterScraper Assisted-by: Claude Opus 4.5

github-actions · 2026-05-19T04:35:49Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

jmacd · 2026-05-26T19:42:32Z

+
+// RegistrationHandle is returned by ControllerExtension.RegisterScraper and
+// is used to deregister the scraper during shutdown.
+type RegistrationHandle interface {


Suggest making this a closed interface so that you can add methods in the future, with a constructor method (as described in the RFC)

Hmm. I think that makes sense for something like Factory, which is a collection of methods. This interface is more akin to io.Reader, i.e. it has a singular purpose, so I don't think expanding methods is realistic.

Perhaps I should just make it a function type, so RegisterScraper becomes:

RegisterScraper(context.Context, ScrapeFunc) (DeregisterFunc, error)

WDYT?

I made this change, can easily revert if you're against it

…nscrapercontroller

- Create ScrapeFunc type - Replace RegistrationHandle type with DeregisterFunc type - Update collection_interval error message per review

- Fix timeout validation error message - Capture deregistration errors - Call host.GetExtensions() outside loop

- Deregister extensions concurrently in Shutdown so total wait time is bounded by the slowest single extension, not the sum of all - Capture scraper Shutdown errors in Start's failure-cleanup defer instead of silently discarding them - Fix TestStartScrapingNoCollectionInterval to use a valid config (zero CollectionInterval requires at least one controller) - Move MockHost and MockControllerExtension to internal/testhelper, eliminating the duplicate definitions across the two test packages; the shared mock correctly blocks DeregisterFunc on in-flight scrapes Assisted-by: Claude Sonnet 4.6

…el teardown - Document that RegisterScraper must return a non-nil DeregisterFunc when err is nil; add defensive nil guard in teardown - Fix TOCTOU race in MockControllerExtension.Scrape: hold mu across the nil/deregistered check and scrapeWG.Add so DeregisterFunc cannot observe a zero count between the two - Extract shared teardown helper used by both Start cleanup and Shutdown: deregisters extensions and shuts down scrapers concurrently, clearing deregFuncs first to prevent double-invocation - Fix TestShutdownScrapers: protect shared slice with mutex and use ElementsMatch since scraper shutdown order is now non-deterministic Assisted-by: Claude Sonnet 4.6

Not quite right, but not worth the hassle of breaking contrib tests.

github-actions · 2026-06-14T04:44:57Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

mx-psi · 2026-07-03T16:24:09Z

+		})
+	}
+	wg.Wait()
+	for _, scrp := range scrapers {


Should we guard this in a similar way to deregFuncs so we don't call Shutdown twice? (It could happen if Start fails since we call Shutdown in that case

opentelemetry-collector/otelcol/collector.go

Lines 264 to 265 in da5e6f6

if err = col.service.Start(ctx); err != nil {

return multierr.Combine(err, col.service.Shutdown(ctx))

)

On main we only ever shutdown the scrapers when the controller's Shutdown method is called, and not on when Start fails. I'll revert to that for scrapers, and only call deregFuncs when Start fails, so that there's nothing actively scraping after Start returns an error.

axw force-pushed the extensionscrapercontroller branch from 43fa995 to e904e60 Compare April 15, 2026 03:25

axw force-pushed the extensionscrapercontroller branch 2 times, most recently from ffacd96 to e6602b9 Compare April 15, 2026 06:27

axw added a commit to axw/opentelemetry-collector that referenced this pull request Apr 16, 2026

[scraperhelper]: add more unit tests

4183493

Add tests to the internal/controller package, so we have baseline coverage before making the changes in open-telemetry#15116.

axw mentioned this pull request Apr 16, 2026

[chore] [scraperhelper] add more unit tests #15125

Merged

axw force-pushed the extensionscrapercontroller branch 3 times, most recently from 3aa92a2 to 8aef2b0 Compare April 24, 2026 05:47

axw mentioned this pull request Apr 24, 2026

[chore] [scraperhelper] simplify tests #15188

Merged

axw force-pushed the extensionscrapercontroller branch from 8aef2b0 to 7a80b44 Compare April 28, 2026 08:29

axw force-pushed the extensionscrapercontroller branch from 7a80b44 to e126197 Compare April 30, 2026 07:59

axw marked this pull request as ready for review April 30, 2026 08:58

axw requested review from a team, bogdandrutu, codeboten, dmitryax and mx-psi as code owners April 30, 2026 08:58

Merge branch 'main' into extensionscrapercontroller

0821222

github-actions Bot added Stale and removed Stale labels May 19, 2026

jmacd requested changes May 26, 2026

View reviewed changes

axw added 3 commits May 27, 2026 10:01

Merge branch 'main' into extensionscrapercontroller

7d3f34f

Merge commit '082122255168501b25cac9ad44eba6684cd4b224' into extensio…

de565a5

…nscrapercontroller

Various changes

d278cbe

- Create ScrapeFunc type - Replace RegistrationHandle type with DeregisterFunc type - Update collection_interval error message per review

axw added 8 commits May 27, 2026 15:52

Tidy up

6441cb3

- Fix timeout validation error message - Capture deregistration errors - Call host.GetExtensions() outside loop

DeregisterFunc waits for in-flight scrapes

77d98b0

Fix formatting

d725ed1

Revert error message

0b66ebd

Not quite right, but not worth the hassle of breaking contrib tests.

One more attempt...

ca5b483

Merge branch 'main' into extensionscrapercontroller

b978909

axw requested a review from jmacd May 28, 2026 03:37

axw added 2 commits May 29, 2026 12:51

Merge branch 'main' into extensionscrapercontroller

500ab18

make gotidy

4565f08

github-actions Bot added the Stale label Jun 14, 2026

jmacd removed the Stale label Jun 17, 2026

Merge branch 'main' into extensionscrapercontroller

7b69d32

jmacd approved these changes Jun 22, 2026

View reviewed changes

axw added 4 commits June 23, 2026 09:24

Merge branch 'main' into extensionscrapercontroller

935edc6

Increase test coverage

b626ef4

Fix lint

1383327

Merge branch 'main' into extensionscrapercontroller

da5e6f6

mx-psi reviewed Jul 3, 2026

View reviewed changes

axw added 2 commits July 6, 2026 09:39

Merge branch 'main' into extensionscrapercontroller

b8e6d91

Only shut down scrapers on controller shutdown

3dd829d

atoulme approved these changes Jul 8, 2026

View reviewed changes

Merge branch 'main' into extensionscrapercontroller

808e47d

florianl approved these changes Jul 8, 2026

View reviewed changes

axw requested a review from mx-psi July 13, 2026 09:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[scraperhelper] Add extension-based scraper controller support#15116

[scraperhelper] Add extension-based scraper controller support#15116
axw wants to merge 23 commits into
open-telemetry:mainfrom
axw:extensionscrapercontroller

axw commented Apr 15, 2026 •

edited

Loading

Uh oh!

codspeed-hq Bot commented Apr 15, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Apr 15, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 19, 2026

Uh oh!

Uh oh!

jmacd May 26, 2026

Uh oh!

axw May 27, 2026

Uh oh!

axw May 27, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jun 14, 2026

Uh oh!

mx-psi Jul 3, 2026

Uh oh!

axw Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	if err = col.service.Start(ctx); err != nil {
	return multierr.Combine(err, col.service.Shutdown(ctx))

Uh oh!

Conversation

axw commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Link to tracking issue

Testing

Documentation

Uh oh!

codspeed-hq Bot commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will create unknown performance changes

Footnotes

Uh oh!

codecov Bot commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented May 19, 2026

Uh oh!

Uh oh!

jmacd May 26, 2026

Choose a reason for hiding this comment

Uh oh!

axw May 27, 2026

Choose a reason for hiding this comment

Uh oh!

axw May 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jun 14, 2026

Uh oh!

mx-psi Jul 3, 2026

Choose a reason for hiding this comment

Uh oh!

axw Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

axw commented Apr 15, 2026 •

edited

Loading

codspeed-hq Bot commented Apr 15, 2026 •

edited

Loading

codecov Bot commented Apr 15, 2026 •

edited

Loading