Skip to content

Extend Attribute API with optional AttributesLimits, including count, length, and depth - #8656

Closed
jack-berg wants to merge 7 commits into
open-telemetry:mainfrom
jack-berg:api-attribute-limits
Closed

Extend Attribute API with optional AttributesLimits, including count, length, and depth#8656
jack-berg wants to merge 7 commits into
open-telemetry:mainfrom
jack-berg:api-attribute-limits

Conversation

@jack-berg

@jack-berg jack-berg commented Jul 23, 2026

Copy link
Copy Markdown
Member

Fixes #7897.

Embodies "approach 3" from this comment.

Adds AttributeLimits and Attributes.builder(AttributeLimits) to opentelemetry-api, giving users (and the SDK) a way to build attributes that enforce the OpenTelemetry common attribute-limits rules: count limit, value length limit, and (soon) value depth limit.

AttributeLimits limits =
    AttributeLimits.builder()
        .setCountLimit(128)
        .setValueLengthLimit(1024)
        .setValueDepthLimit(64)
        .build();

Attributes attrs =
    Attributes.builder(limits)
        .put(stringKey("http.method"), "GET")
        .put(stringKey("http.method"), "POST") // overwrites; last-value-wins
        .build();
  • AttributeLimits uses autovalue
  • Names follow the spec parameters, rather than SDK LogLimits / SpanLimits
    • getCountLimit vs. SpanLimits.getMaxNumberOfAttributes
    • getValueLengthLimit vs. SpanLimits.getMaxAttributeValueLength
    • getValueDepthLimit vs no current SpanLimits equivalent
  • Builder rejects negative counts and lengths. Depth must be >= 1 (depth is 1-indexed per spec).
  • By default, no limits are applied. I.e. Attributes.builder() is equivalent to Attributes.builder(AttributeLimits.noLimits()).
  • Bounded builder applies last-value-wins on put by key name (regardless of type), truncates over-length strings and byte arrays, replaces over-nested arrays and maps with empty containers, drops entries beyond the count limit. Overwrites do not consume against the count limit.

SDK integration:

  • AttributesMap in sdk-common is deleted. SdkSpan, SdkSpanBuilder, SdkLogRecordBuilder, SdkReadWriteLogRecord now store an AttributesBuilder obtained via Attributes.builder(AttributeLimits) and track totalAttributeCount locally.
  • SpanLimits.getMaxNumberOfAttributes / getMaxAttributeValueLength are still the source of truth for span attribute limits. Wiring depth through SpanLimits / LogLimits is deferred for now.

Performance:

Extended AttributesBenchmark with a few more relevant cases. See before and after below:

Details
Benchmark Baseline (ns/op) After (ns/op) Δ ns/op Δ %
builderOneItem 13 13 +0 +0.5%
builderTenItems 255 278 +23 +9.0%
computeHashCode 6 6 +0 +0.7%
ofFive 69 68 -1 -1.0%
ofFour 52 49 -4 -7.0%
ofOne 3 3 -0 -1.4%
ofThree 34 34 -0 -0.0%
ofTwo 6 6 +0 +0.7%
putAllTenItems 281 265 -16 -5.9%

@otelbot otelbot Bot added the api-change Changes to public API surface area label Jul 23, 2026
@otelbot

otelbot Bot commented Jul 23, 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-api

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

@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Jul 23, 2026

Copy link
Copy Markdown

Pull request dashboard status

Closed · refreshed 2026-07-31 19:20 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 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.13924% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.57%. Comparing base (6328897) to head (4c65a22).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...entelemetry/api/internal/AttributeValueLimits.java 87.50% 6 Missing and 6 partials ⚠️
...metry/api/common/ArrayBackedAttributesBuilder.java 89.09% 4 Missing and 2 partials ⚠️
.../main/java/io/opentelemetry/sdk/trace/SdkSpan.java 90.90% 0 Missing and 2 partials ⚠️
.../opentelemetry/sdk/logs/SdkReadWriteLogRecord.java 88.88% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8656      +/-   ##
============================================
+ Coverage     91.46%   91.57%   +0.10%     
- Complexity    10457    10492      +35     
============================================
  Files          1021     1023       +2     
  Lines         27647    27735      +88     
  Branches       3242     3251       +9     
============================================
+ Hits          25288    25397     +109     
+ Misses         1616     1599      -17     
+ Partials        743      739       -4     

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

private int countLimit = Integer.MAX_VALUE;
private int valueLengthLimit = Integer.MAX_VALUE;

// TODO(jack-berg): before merging, decide whether to default this to 64 (spec-recommended,

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.

Reminder to self

return result;
}

// TODO(jack-berg): convert to iterative traversal (explicit stack) to guarantee no

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.

Reminder to self

@jack-berg jack-berg mentioned this pull request Jul 31, 2026
@jack-berg

Copy link
Copy Markdown
Member Author

I think this is a dead end. Closing. See #8681 description for details.

@jack-berg jack-berg closed this Jul 31, 2026
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.

AttributeMap does not respect last-value-wins semantics for duplicate keys (SpanBuilder.setAttribute vs Attributes.builder)

1 participant