Skip to content

Sdk attribute limits - #8681

Closed
jack-berg wants to merge 3 commits into
open-telemetry:mainfrom
jack-berg:sdk-attribute-limits
Closed

Sdk attribute limits#8681
jack-berg wants to merge 3 commits into
open-telemetry:mainfrom
jack-berg:sdk-attribute-limits

Conversation

@jack-berg

Copy link
Copy Markdown
Member

Alternative to #8656 that removes all limits-related public API surface area from opentelemetry-sdk, putting it in opentelemetry-sdk-common instead.

The goal of this whole effort was to try to unify on a single attributes implementation (ArrayBackedAttributesBuilder,
ArrayBackedAttributes) which backs the default Attributes implementation from the public API.

But after a lot of experimenting I think this is a dead end:

  • The additional limits enforcement / dropped attribute counting requires a substantial amount of additional work / coordination such that maintaining a common abstraction in ArrayBackedAttributesBuilder, ArrayBackedAttributes is actually more work than maintaining two separate implementations.
  • The array based attribute implementation is a real impediment to performance compared to a HashMap based implementation. Even with attempts at optimization, I think we're still looking at a 5-30% performance hit in span / log recording by trying to standardize on the array based implementation. Hard to stomach.

I'm opening this PR as just as a record to look back on in case we ever start thinking in this direction again. I will close it right away and think we should go for a version of #8656 instead.

@jack-berg
jack-berg requested a review from a team as a code owner July 31, 2026 19:18
@jack-berg jack-berg closed this Jul 31, 2026
@otelbot otelbot Bot added the api-change Changes to public API surface area label Jul 31, 2026
@otelbot

otelbot Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

⚠️ API changes detected — additional maintainer review required

@jack-berg @jkwatson

This PR modifies the public API surface area of the following module(s):

  • opentelemetry-sdk-common

Please review the changes in docs/apidiffs/current_vs_latest/ carefully before approving.

@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Jul 31, 2026

Copy link
Copy Markdown

Pull request dashboard status

Closed · refreshed 2026-07-31 19:34 UTC

Status above doesn't look right?
  • Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.26756% with 59 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.43%. Comparing base (bc956bc) to head (237706f).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...io/opentelemetry/sdk/common/LimitedAttributes.java 74.20% 38 Missing and 19 partials ⚠️
.../main/java/io/opentelemetry/sdk/trace/SdkSpan.java 88.23% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8681      +/-   ##
============================================
- Coverage     91.47%   91.43%   -0.04%     
- Complexity    10465    10511      +46     
============================================
  Files          1021     1022       +1     
  Lines         27694    27810     +116     
  Branches       3247     3276      +29     
============================================
+ Hits          25333    25428      +95     
- Misses         1617     1630      +13     
- Partials        744      752       +8     

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-change Changes to public API surface area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant