Skip to content

perf(sdk-trace-base): use counter for attribute limit check in Span#6283

Merged
david-luna merged 3 commits intoopen-telemetry:mainfrom
AbhiPrasad:perf/span-setattribute-counter
Jan 29, 2026
Merged

perf(sdk-trace-base): use counter for attribute limit check in Span#6283
david-luna merged 3 commits intoopen-telemetry:mainfrom
AbhiPrasad:perf/span-setattribute-counter

Conversation

@AbhiPrasad
Copy link
Copy Markdown
Member

@AbhiPrasad AbhiPrasad commented Jan 12, 2026

Which problem is this PR solving?

Replace O(n) Object.keys().length call with O(1) counter lookup when checking attribute count limits in setAttribute().

Benchmark results show up to 22x improvement for spans with 128 attributes (worst case). From my experience at Sentry, most auto-instrumented Node.js spans tend to have ~50-60 attributes, so I'd expect atleast a 5x speedup in the average case.

Short description of the changes

  • Added _attributesCount private counter field to SpanImpl class to track the number of attributes
  • Replaced Object.keys(this.attributes).length (O(n)) with this._attributesCount (O(1)) in the attribute limit check
  • Counter is only incremented when adding a new key, not when overwriting an existing one

Type of change

Performance improvement

How Has This Been Tested?

Add a new benchmark for this (AI generated). Tested on a 2021 Apple M1 Pro 32 GB - MacOS 15.5 (24F74)

Scenario Before After Improvement
10 attributes 891,847 ops/sec 945,411 ops/sec 1.06x
50 attributes 66,547 ops/sec 312,021 ops/sec 4.7x
100 attributes 13,409 ops/sec 172,770 ops/sec 12.9x
128 attributes 6,599 ops/sec 147,883 ops/sec 22.4x
setAttributes bulk (100) 11,403 ops/sec 51,676 ops/sec 4.5x

Checklist:

  • Followed the style guidelines of this project
  • Unit tests have been added
  • Documentation has been updated

@AbhiPrasad AbhiPrasad requested a review from a team as a code owner January 12, 2026 22:38
@AbhiPrasad AbhiPrasad force-pushed the perf/span-setattribute-counter branch from 7e450b7 to 47e451a Compare January 12, 2026 22:39
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.68%. Comparing base (903739c) to head (ff7c343).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6283   +/-   ##
=======================================
  Coverage   95.68%   95.68%           
=======================================
  Files         313      313           
  Lines        9667     9671    +4     
  Branches     2226     2227    +1     
=======================================
+ Hits         9250     9254    +4     
  Misses        417      417           
Files with missing lines Coverage Δ
packages/opentelemetry-sdk-trace-base/src/Span.ts 97.75% <100.00%> (+0.05%) ⬆️
🚀 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.

@david-luna
Copy link
Copy Markdown
Contributor

@AbhiPrasad this PR has some conflicts. Could you please update? Thank you!

Replace O(n) Object.keys().length call with O(1) counter lookup
when checking attribute count limits in setAttribute(). This
eliminates O(n²) complexity when adding many attributes to a span.

Benchmark results show up to 22x improvement for spans with 128 attributes.
@AbhiPrasad AbhiPrasad force-pushed the perf/span-setattribute-counter branch from f3654ba to da51619 Compare January 29, 2026 15:58
@AbhiPrasad
Copy link
Copy Markdown
Member Author

I think the unit test failure is a flake

> opentelemetry@0.1.0 precompile
> npm run submodule && nx run-many -t protos:generate && nx run-many -t version


> opentelemetry@0.1.0 submodule
> git submodule sync --recursive && git submodule update --init --recursive

Submodule 'experimental/packages/otlp-transformer/protos' (https://github.com/open-telemetry/opentelemetry-proto.git) registered for path 'experimental/packages/otlp-transformer/protos'
Cloning into '/home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/otlp-transformer/protos'...
Submodule path 'experimental/packages/otlp-transformer/protos': checked out '8654ab7a5a43ca25fe8046e59dcd6935c3f76de0'
Segmentation fault (core dumped)
Error: Process completed with exit code 139.

@david-luna
Copy link
Copy Markdown
Contributor

Restarted the check. Let see if it goes through

@david-luna david-luna added this pull request to the merge queue Jan 29, 2026
Merged via the queue into open-telemetry:main with commit b97b488 Jan 29, 2026
36 of 37 checks passed
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