Skip to content

Fix bound instruments pinning context-derived attributes - #8782

Open
jaydeluca wants to merge 2 commits into
open-telemetry:mainfrom
jaydeluca:bound-fix
Open

Fix bound instruments pinning context-derived attributes#8782
jaydeluca wants to merge 2 commits into
open-telemetry:mainfrom
jaydeluca:bound-fix

Conversation

@jaydeluca

@jaydeluca jaydeluca commented Sep 8, 2026

Copy link
Copy Markdown
Member

ExtendedLongCounter.bind(Attributes) and its sibling APIs currently resolve their target time series at bind time. As part of that lookup, each applicable view's AttributesProcessor runs against Context.current(). If a view derives attributes from context (the only currently supported examples are SdkMeterProviderUtil.appendAllBaggageAttributes and appendFilteredBaggageAttributes) then subsequent measurements through the bound instrument remain pinned to the baggage that was current at bind time, rather than using the context supplied at record time.

Depending on the usage pattern, this either drops the context-derived attributes entirely when binding at startup, misattributes later requests to an earlier request's baggage when a binding is reused, or eagerly creates one series per distinct bind-time baggage set when binding per request, eventually exhausting the cardinality limit.

AttributesProcessor.usesContext() prevents this eager binding and is still documented as doing so. Before bound instruments were removed, DefaultSynchronousMetricStorage.bind() returned a late-bound handle when usesContext() was true.

That guard and fallback handle were deleted from DefaultSynchronousMetricStorage in #5157.

When bound instruments returned in #8527, the new cumulative and delta bind paths processed attributes against Context.current() without restoring that check.

@codecov

codecov Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.30%. Comparing base (35636ae) to head (d6c7974).
⚠️ Report is 22 commits behind head on main.

Files with missing lines Patch % Lines
...nternal/state/DefaultSynchronousMetricStorage.java 75.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8782      +/-   ##
============================================
+ Coverage     91.27%   91.30%   +0.02%     
- Complexity    10472    10509      +37     
============================================
  Files          1006     1007       +1     
  Lines         28277    28387     +110     
  Branches       3569     3588      +19     
============================================
+ Hits          25811    25918     +107     
  Misses         1674     1674              
- Partials        792      795       +3     

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

@jaydeluca
jaydeluca marked this pull request as ready for review September 8, 2026 23:48
@jaydeluca
jaydeluca requested a review from a team as a code owner September 8, 2026 23:48
*
* <p>If the metric's view derives attributes from context (e.g. baggage), this optimization does
* not apply: each record call performs the same attribute processing and lookup as an unbound
* recording.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah this is why I want to go forward with #8719 - the machinery needed to maintain this unspecified feature is growing in size / complexity.

I think this PR is correct, but before we proceed, let's talk about #8719 at the next java SIG to see where that falls. If folks agree, then I believe we can skip this.

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.

2 participants