Skip to content

[CONFIGURATION] Add periodic metric reader builder support to registry#4277

Merged
dbarker merged 11 commits into
open-telemetry:mainfrom
dbarker:config_add_periodic_reader_builder
Jul 24, 2026
Merged

[CONFIGURATION] Add periodic metric reader builder support to registry#4277
dbarker merged 11 commits into
open-telemetry:mainfrom
dbarker:config_add_periodic_reader_builder

Conversation

@dbarker

@dbarker dbarker commented Jul 22, 2026

Copy link
Copy Markdown
Member

Followup to #4243 (comment)

The configuration tests instantiated the PeriodicMetricReader and call ForceFlush. This caused tests to timeout sporadically. This PR extends the config registry and SDK builder to support setting a builder for the periodic metric reader. A synchronous mock reader is then used in configuration tests to verify config components.

Changes

  • Add Set/Get methods for the PeriodicMetricReader to the registry and use them in the sdk builder
  • Move common config test components to a common header
  • Update config tests to use a simple sync metric reader in the periodic reader's registry slot.
  • Add an SDK builder test for PeriodicMetricReaderConfiguration
  • Disable config tests that use the batch processors - they'll be enabled in a followup PR.

For significant contributions please make sure you have completed the following items:

  • CHANGELOG.md updated for non-trivial changes
  • Unit tests have been added
  • Changes in public API reviewed

@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 63.15789% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.25%. Comparing base (d3c7a47) to head (e81602a).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
sdk/src/configuration/registry.cc 16.67% 5 Missing ⚠️
sdk/src/configuration/sdk_builder.cc 71.43% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4277      +/-   ##
==========================================
- Coverage   81.31%   81.25%   -0.06%     
==========================================
  Files         445      446       +1     
  Lines       18859    18872      +13     
==========================================
- Hits        15333    15332       -1     
- Misses       3526     3540      +14     
Files with missing lines Coverage Δ
...sdk/configuration/periodic_metric_reader_builder.h 100.00% <100.00%> (ø)
...include/opentelemetry/sdk/configuration/registry.h 16.13% <100.00%> (+12.43%) ⬆️
sdk/src/configuration/sdk_builder.cc 49.53% <71.43%> (+4.49%) ⬆️
sdk/src/configuration/registry.cc 60.18% <16.67%> (-2.43%) ⬇️

... and 14 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

… it may hang on force flush and timeout the tests
@dbarker
dbarker marked this pull request as ready for review July 23, 2026 02:52
@dbarker
dbarker requested a review from a team as a code owner July 23, 2026 02:52
Comment thread sdk/test/configuration/config_test_common.h Outdated
Comment thread sdk/test/configuration/config_test_common.h Outdated
@dbarker
dbarker requested a review from lalitb July 23, 2026 17:42
@dbarker
dbarker enabled auto-merge (squash) July 24, 2026 19:40
@dbarker
dbarker merged commit c0111db into open-telemetry:main Jul 24, 2026
71 checks passed
om7057 added a commit to om7057/opentelemetry-cpp that referenced this pull request Jul 25, 2026
The merge of main's registry-based PeriodicMetricReaderBuilder
(open-telemetry#4277) left CreatePeriodicMetricReader() with unreachable dead code
and a reference to the now-removed PeriodicExportingMetricReaderFactory
direct-construction path, breaking the build across all CI jobs.
Restore the registry builder flow and apply cardinality-limit
enforcement to the reader it produces.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
om7057 added a commit to om7057/opentelemetry-cpp that referenced this pull request Jul 25, 2026
The merge of main's registry-based PeriodicMetricReaderBuilder
(open-telemetry#4277) left CreatePeriodicMetricReader() with unreachable dead code
and a reference to the now-removed PeriodicExportingMetricReaderFactory
direct-construction path, breaking the build across all CI jobs.
Restore the registry builder flow and apply cardinality-limit
enforcement to the reader it produces.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants