Skip to content

fix(span,sdk-logs): enforce event/link attribute limits, link count limit, and fix LogRecord droppedAttributesCount#6479

Merged
overbalance merged 5 commits intoopen-telemetry:mainfrom
embrace-io:overbalance/fix-limits
Mar 10, 2026
Merged

fix(span,sdk-logs): enforce event/link attribute limits, link count limit, and fix LogRecord droppedAttributesCount#6479
overbalance merged 5 commits intoopen-telemetry:mainfrom
embrace-io:overbalance/fix-limits

Conversation

@overbalance
Copy link
Copy Markdown
Contributor

@overbalance overbalance commented Mar 6, 2026

Which problem is this PR solving?

Found during a spec compliance review: several SpanLimits options are accepted but not enforced at runtime, and LogRecordImpl miscounts dropped attributes. These are unlikely to surface at default limits (128) but become incorrect when limits are explicitly configured to lower values.

  1. attributePerEventCountLimit and attributePerLinkCountLimit are not enforced. Event/link attributes pass through unbounded.
  2. attributeValueLengthLimit truncates span-level attributes but not event or link attribute values.
  3. linkCountLimit is not enforced. addLink()/addLinks()/constructor links accumulate without bound.
  4. LogRecordImpl.droppedAttributesCount is incorrectly incremented when an existing attribute key is updated.

Short description of the changes

Span.ts:

  • Enforce attributePerEventCountLimit, attributePerLinkCountLimit, and linkCountLimit
  • Truncate event/link attribute values via attributeValueLengthLimit
  • Route constructor and addLinks() through addLink() so limits apply uniformly
  • Add ended-span guard and dropped-links warning to match existing addEvent() patterns
  • Fix attributeValueLengthLimit coercion (|| to ??)

LogRecordImpl.ts:

  • Track _attributesCount and _droppedAttributesCount explicitly instead of deriving from Object.keys().length
  • Only count a drop when a new key is rejected, not when an existing key is updated

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

10 new unit tests covering event/link attribute count limits, event/link attribute value truncation, link count FIFO eviction, link count via constructor, link count = 0, addLinks() limit enforcement, and LogRecord droppedAttributesCount accuracy on key updates.

Checklist:

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

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.73%. Comparing base (e7ced9f) to head (471990a).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6479      +/-   ##
==========================================
+ Coverage   95.70%   95.73%   +0.03%     
==========================================
  Files         364      364              
  Lines       11779    11826      +47     
  Branches     2751     2765      +14     
==========================================
+ Hits        11273    11322      +49     
+ Misses        506      504       -2     
Files with missing lines Coverage Δ
...xperimental/packages/sdk-logs/src/LogRecordImpl.ts 99.31% <100.00%> (+0.01%) ⬆️
packages/opentelemetry-sdk-trace-base/src/Span.ts 99.12% <100.00%> (+1.28%) ⬆️
🚀 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.

@overbalance overbalance force-pushed the overbalance/fix-limits branch from 796f189 to 8ea74d8 Compare March 6, 2026 02:11
@overbalance overbalance force-pushed the overbalance/fix-limits branch from 8ea74d8 to 7c220b6 Compare March 6, 2026 02:11
@overbalance overbalance marked this pull request as ready for review March 6, 2026 02:12
@overbalance overbalance requested a review from a team as a code owner March 6, 2026 02:12
@pichlermarc pichlermarc added this to the Logs API/SDK GA milestone Mar 10, 2026
@overbalance overbalance added this pull request to the merge queue Mar 10, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Mar 10, 2026
@overbalance overbalance enabled auto-merge March 10, 2026 15:15
@overbalance overbalance added this pull request to the merge queue Mar 10, 2026
Merged via the queue into open-telemetry:main with commit de02086 Mar 10, 2026
35 of 37 checks passed
@overbalance overbalance deleted the overbalance/fix-limits branch March 10, 2026 15:31
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.

3 participants