Skip to content

feat(sdk-node)!: do not instantiate metrics SDK on empty metricReaders#6272

Merged
pichlermarc merged 3 commits intoopen-telemetry:mainfrom
dynatrace-oss-contrib:fix/empty-metric-reader
Jan 9, 2026
Merged

feat(sdk-node)!: do not instantiate metrics SDK on empty metricReaders#6272
pichlermarc merged 3 commits intoopen-telemetry:mainfrom
dynatrace-oss-contrib:fix/empty-metric-reader

Conversation

@pichlermarc
Copy link
Copy Markdown
Member

Which problem is this PR solving?

There's currently no programmatic way to tell the SDK not to instantiate a Metrics SDK. For traces this can be done via spanProcessors: []. We should follow the same behavior for metrics, where

  • metricReaders: [] -> instantiates no metrics SDK, no matter what's set in OTEL_METRICS_EXPORTER

This change is breaking only for users that use metricReaders: [] AND OTEL_METRICS_EXPORTER=... at the same time.

Ref #6092

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.39%. Comparing base (228cb92) to head (945a3af).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6272      +/-   ##
==========================================
- Coverage   95.40%   95.39%   -0.01%     
==========================================
  Files         316      316              
  Lines        9568     9558      -10     
  Branches     2214     2210       -4     
==========================================
- Hits         9128     9118      -10     
  Misses        440      440              
Files with missing lines Coverage Δ
...imental/packages/opentelemetry-sdk-node/src/sdk.ts 96.35% <100.00%> (-0.19%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

if (
this._meterProviderConfig?.readers &&
// only register if there is a reader, otherwise we waste compute/memory.
this._meterProviderConfig.readers.length > 0
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use optional chaining here and drop the first line of this check.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I found the optional chaining to be a bit more difficult to read compared so I went with the other thing.

(this._meterProviderConfig?.readers?.length ?? 0) > 0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this didn't actually get changed.

Copy link
Copy Markdown
Member

@JamieDanielson JamieDanielson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small update in wording for the changelog (typo)

Comment thread experimental/CHANGELOG.md Outdated
Co-authored-by: Jamie Danielson <jamiedanielson@honeycomb.io>
Comment thread experimental/CHANGELOG.md Outdated
@pichlermarc pichlermarc enabled auto-merge January 9, 2026 11:45
@pichlermarc pichlermarc added this pull request to the merge queue Jan 9, 2026
Merged via the queue into open-telemetry:main with commit 87a0b45 Jan 9, 2026
27 checks passed
@pichlermarc pichlermarc deleted the fix/empty-metric-reader branch January 9, 2026 11:55
@otelbot-js otelbot-js Bot mentioned this pull request Jan 9, 2026
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.

4 participants