Add experimental OptIn instrument advice and View enabled configuration by dashpole · Pull Request #8979 · open-telemetry/opentelemetry-java

dashpole · 2026-10-27T15:37:09Z

Summary

Implements the experimental OptIn instrument advisory parameter and View enabled stream configuration defined in open-telemetry/opentelemetry-specification#4809:

Incubating Metrics API (opentelemetry-api-incubator):
- Adds setOptInAdvice(boolean) (default no-op) to all 8 Extended*Builder interfaces (ExtendedDoubleCounterBuilder, ExtendedDoubleGaugeBuilder, ExtendedDoubleHistogramBuilder, ExtendedDoubleUpDownCounterBuilder, ExtendedLongCounterBuilder, ExtendedLongGaugeBuilder, ExtendedLongHistogramBuilder, ExtendedLongUpDownCounterBuilder).
Metrics SDK (opentelemetry-sdk-metrics):
- Adds isOptIn() / setOptIn(boolean) to internal Advice / AdviceBuilder and wires setOptInAdvice(boolean) across all ExtendedSdk*Builder implementations (preserving optIn across .ofDoubles() / .ofLongs() builder swaps).
- Adds package-private ViewBuilder#setEnabled(boolean) and View#getEnabled(), exposed experimentally via SdkMeterProviderUtil#setEnabled(ViewBuilder, boolean).
- Updates ViewRegistry#findViews so that:
  - Instruments with OptIn=true use Aggregation.drop() by default when no view matches or when a matching view does not set enabled=true.
  - Matching views with enabled=false use Aggregation.drop() regardless of the view's configured aggregation.
  - Matching views with enabled=true enable OptIn=true instruments while still respecting other instrument advisory parameters (Attributes and ExplicitBucketBoundaries) unless overridden by the view.

Copilot

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

opentelemetry-pr-dashboard · 2026-10-27T15:42:32Z

Pull request dashboard status

Waiting on the author · refreshed 2026-11-03 16:15 UTC

Respond to 1 review item (e.g. link a commit, explain why not, ask a follow-up):

Top-level threads: 1

Status above doesn't look right?

Just replied or pushed? Anything around or after the refresh time above may not be picked up yet — give it a few minutes.
Should this be with reviewers? Comment /dashboard route:reviewers to route it to them.
Anything wrong — including the routing? Report it with what you expected; it helps us improve the dashboard.

codecov · 2026-10-27T16:04:26Z

Codecov Report

❌ Patch coverage is 92.77108% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.37%. Comparing base (69c38ee) to head (1eb12d5).

Files with missing lines	Patch %	Lines
...emetry/sdk/metrics/internal/view/ViewRegistry.java	89.74%	2 Missing and 2 partials ⚠️
...try/sdk/metrics/internal/SdkMeterProviderUtil.java	71.42%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #8979      +/-   ##
============================================
- Coverage     90.39%   90.37%   -0.02%     
- Complexity    10997    11018      +21     
============================================
  Files          1020     1020              
  Lines         30096    30170      +74     
  Branches       3929     3939      +10     
============================================
+ Hits          27204    27267      +63     
- Misses         1937     1942       +5     
- Partials        955      961       +6

☔ 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.

Copilot

🟢 Approval recommended

Static review found no blocking issues in the localized API additions, SDK enablement logic, or associated regression coverage.

No open issues 🎉

_{Review effort: Balanced}

psx95

When registering the associated storage for metrics in SdkMeter, if a view has drop aggregation, then the storage for that is skipped. This could probably cause an issue in the intended functionality because opentelemetry-java evaluates each matching RegisteredView independently in SdkMeter and simply skips (continue) any view whose aggregation is Aggregation.drop(), registering both:

InstrumentSelector("*") → enabled = true
InstrumentSelector("my.unwanted.instrument") → enabled = false and this will not disable "my.unwanted.instrument" since the second view turns into Aggregation.drop() and is skipped by SdkMeter, while the first view (*) still registers a live MetricStorage for "my.unwanted.instrument".

IIUC, this is a key use-case of the spec, right?

psx95 · 2026-10-27T19:45:03Z

+      UnaryOperator<Attributes> attributesFilter,
+      int cardinalityLimit,
+      SourceInfo viewSourceInfo,
+      @Nullable Boolean enabled) {


Why is this Nullable? We already have a constructor which can be used when the user does not wish to explicitly provide enabled.

SdkMeterProviderBuilder#registerView and ViewRegistry pass view.getEnabled() / registeredView.getEnabled() (which is @Nullable Boolean—null when unset, true, or false) directly into this 6-argument create method, so @Nullable avoids requiring a null check at each call site. The 5-argument overload is just a convenience for internal default views (DEFAULT_REGISTERED_VIEW, DROP_REGISTERED_VIEW) and tests that don't have a View with enabled.

psx95 · 2026-10-27T19:56:48Z

+    assertThat(counter2.isEnabled()).isFalse();
+
+    counter1.add(1);
+    counter2.add(1);


nit: Perhaps adding an assertion to reflect the expected behavior of updating a disabled counter might be good.

addressed in e5409e0 (satisfiesExactly asserts that collectAllMetrics() only contains counter1 and that counter2's measurement was dropped; added a comment above the assertion to make that explicit).

psx95 · 2026-10-27T20:12:37Z


  private static RegisteredView applyAdviceToDefaultView(
-      RegisteredView instrumentDefaultView, Advice advice) {
+      RegisteredView registeredView, Advice advice) {


Just curious: Why the name change?

Previously applyAdviceToDefaultView was only called on instrumentDefaultView when no view matched. Since it is now also called at line 128 on a matched RegisteredView (entry) when enabled=true and the view did not specify an attribute filter, I generalized the parameter name to registeredView.

psx95 · 2026-10-27T21:04:26Z

      if (matchesSelector(entry.getInstrumentSelector(), descriptor, meterScope)) {
+        if (Boolean.FALSE.equals(entry.getEnabled())) {
+          result.add(applyDropAggregation(entry));
+          continue;


This would force the next iteration without checking isCompatibleWithInstrument verification.

addressed in 2520e45

psx95 · 2026-10-28T02:18:10Z

+          if (descriptor.getAdvice().isOptIn() && !Boolean.TRUE.equals(entry.getEnabled())) {
+            result.add(applyDropAggregation(entry));
+          } else if (Boolean.TRUE.equals(entry.getEnabled())
+              && descriptor.getAdvice().hasAttributes()
+              && entry.getView().getAttributeFilter().equals(StringPredicates.ALL)) {
+            result.add(applyAdviceToDefaultView(entry, descriptor.getAdvice()));
+          } else {
+            result.add(entry);


Can we add minor comments on these if-else conditions to clarify the intent here?

Currently, it seems like on a standard instrument (OptIn = false) with AttributesAdvice would be treated differently depending on enabled being unset or true.

Setting enabled = true on a standard instrument would cause the advice to be applied, but leaving enabled unset would not. But it looks like this is a side effect of the current implementation rather than intention.

addressed in e942aa3. And yes, on OptIn=false instruments when enabled is unset, matching views without an attribute filter currently ignore Attributes advice due to #7134 (pre-dating open-telemetry/opentelemetry-specification#4389), whereas for enabled=true we preserve Attributes advice so enabling an OptIn instrument via a view respects its other advisory parameters per open-telemetry/opentelemetry-specification#4809.

dashpole

When registering the associated storage for metrics in SdkMeter, if a view has drop aggregation, then the storage for that is skipped... IIUC, this is a key use-case of the spec, right?

In the Metrics SDK specification (Measurement processing), that difference comes down to view_matching_mode:

In view_matching_mode: independent (the default, and what opentelemetry-java currently implements), each matching View is evaluated independently without merging across views—so just as registering InstrumentSelector("*") (with DefaultAggregation) alongside InstrumentSelector("my.unwanted.instrument") with Aggregation.drop() still exports "my.unwanted.instrument" from the first wildcard view today, enabled only controls the stream produced by that view. (In independent mode, you can disable all instruments by default with InstrumentSelector("*") -> enabled = false and selectively enable InstrumentSelector("my.wanted.instrument") -> enabled = true.)
Overriding InstrumentSelector("*") -> enabled = true with a subsequent InstrumentSelector("my.unwanted.instrument") -> enabled = false is supported under view_matching_mode: composable (open-telemetry/opentelemetry-specification#5173), which merges matching views with the same stream name in registration order using last-wins precedence for enabled, aggregation, description, and aggregation_cardinality_limit.

dashpole · 2026-10-28T20:17:29Z

+      UnaryOperator<Attributes> attributesFilter,
+      int cardinalityLimit,
+      SourceInfo viewSourceInfo,
+      @Nullable Boolean enabled) {


SdkMeterProviderBuilder#registerView and ViewRegistry pass view.getEnabled() / registeredView.getEnabled() (which is @Nullable Boolean—null when unset, true, or false) directly into this 6-argument create method, so @Nullable avoids requiring a null check at each call site. The 5-argument overload is just a convenience for internal default views (DEFAULT_REGISTERED_VIEW, DROP_REGISTERED_VIEW) and tests that don't have a View with enabled.

dashpole · 2026-10-28T20:17:31Z

+    assertThat(counter2.isEnabled()).isFalse();
+
+    counter1.add(1);
+    counter2.add(1);


addressed in e5409e0 (satisfiesExactly asserts that collectAllMetrics() only contains counter1 and that counter2's measurement was dropped; added a comment above the assertion to make that explicit).

dashpole · 2026-10-28T20:17:32Z


  private static RegisteredView applyAdviceToDefaultView(
-      RegisteredView instrumentDefaultView, Advice advice) {
+      RegisteredView registeredView, Advice advice) {


Previously applyAdviceToDefaultView was only called on instrumentDefaultView when no view matched. Since it is now also called at line 128 on a matched RegisteredView (entry) when enabled=true and the view did not specify an attribute filter, I generalized the parameter name to registeredView.

dashpole · 2026-10-28T20:17:34Z

      if (matchesSelector(entry.getInstrumentSelector(), descriptor, meterScope)) {
+        if (Boolean.FALSE.equals(entry.getEnabled())) {
+          result.add(applyDropAggregation(entry));
+          continue;


addressed in 2520e45

dashpole · 2026-10-28T20:17:36Z

+          if (descriptor.getAdvice().isOptIn() && !Boolean.TRUE.equals(entry.getEnabled())) {
+            result.add(applyDropAggregation(entry));
+          } else if (Boolean.TRUE.equals(entry.getEnabled())
+              && descriptor.getAdvice().hasAttributes()
+              && entry.getView().getAttributeFilter().equals(StringPredicates.ALL)) {
+            result.add(applyAdviceToDefaultView(entry, descriptor.getAdvice()));
+          } else {
+            result.add(entry);


addressed in e942aa3. And yes, on OptIn=false instruments when enabled is unset, matching views without an attribute filter currently ignore Attributes advice due to #7134 (pre-dating open-telemetry/opentelemetry-specification#4389), whereas for enabled=true we preserve Attributes advice so enabling an OptIn instrument via a view respects its other advisory parameters per open-telemetry/opentelemetry-specification#4809.

…dMetricsApiUsageTest

…g View enabled=false

dashpole requested a balanced review from Copilot October 27, 2026 15:37

Copilot AI reviewed Oct 27, 2026

opentelemetry-pr-dashboard Bot mentioned this pull request Oct 27, 2026

Pull Request Dashboard #8439

Open

psx95 requested a balanced review from Copilot October 27, 2026 19:04

Copilot started reviewing on behalf of psx95 October 27, 2026 19:04

View session

Copilot AI reviewed Oct 27, 2026

View reviewed changes

psx95 reviewed Oct 27, 2026

View reviewed changes

psx95 reviewed Oct 28, 2026

View reviewed changes

dashpole force-pushed the feat/metrics-opt-in-and-view-enabled branch from 7bbc2e1 to e942aa3 Compare October 28, 2026 20:17

dashpole commented Oct 28, 2026

View reviewed changes

dashpole marked this pull request as ready for review October 28, 2026 20:17

dashpole requested a review from a team as a code owner October 28, 2026 20:17

dashpole added 4 commits November 3, 2026 15:11

Add experimental OptIn instrument advice and View enabled configuration

ceb3f2c

Address review comment: clarify disabled counter assertion in Extende…

68c46b6

…dMetricsApiUsageTest

Address review comment: check instrument compatibility before applyin…

0cbc0cb

…g View enabled=false

Address review comment: clarify ViewRegistry enabled and advice branches

1eb12d5

dashpole force-pushed the feat/metrics-opt-in-and-view-enabled branch from e942aa3 to 1eb12d5 Compare November 3, 2026 15:12

Conversation

dashpole commented Oct 27, 2026

Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

opentelemetry-pr-dashboard Bot commented Oct 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request dashboard status

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

psx95 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dashpole left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

opentelemetry-pr-dashboard Bot commented Oct 27, 2026 •

edited

Loading

codecov Bot commented Oct 27, 2026 •

edited

Loading