feat(traces): cap the attributes on a span event - #4792
Open
turnipdabeets wants to merge 7 commits into
Open
Conversation
maxAttributesPerSpan does not reach inside events, so a span's width was bounded but its events' was not. Adds maxAttributesPerEvent, default 128, reported per event as the OTLP dropped_attributes_count. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AAPCyRnC5HbukEZV9hbai
Contributor
|
Contributor
Contributor
|
Size Change: -626 kB (-2.95%) Total Size: 20.6 MB 📦 View Changed
ℹ️ View Unchanged
|
turnipdabeets
marked this pull request as ready for review
September 4, 2026 20:21
The encoder drops a nullish attribute, so charging one against maxAttributesPerEvent let a blanked value evict a real one — the rule _writeAttribute and the span half of applySpanLimits already follow. Also corrects a comment claiming a hook cannot rewrite the per-event drop count, and the two ResolvedTracesConfig fixtures that stopped typechecking. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AAPCyRnC5HbukEZV9hbai
Contributor
Prompt To Fix All With AI### Issue 1
packages/core/src/traces/span.ts:734-739
**Nullish attributes consume slots**
When an event reaches `maxAttributesPerEvent`, nullish values consume slots even though the public contract says `null` and `undefined` drop the key. For example, with a limit of 2, `{ a: undefined, b: undefined, c: 1 }` retains only `a` and `b`; encoding then removes both and reports `c` as dropped. Skip nullish entries while selecting retained keys so they cannot evict real attributes or inflate the drop count.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "feat(traces): cap the attributes on a sp..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
rn-flags-disable-and-update-flags belongs to separate RN work; it is preserved in the stash it came from. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AAPCyRnC5HbukEZV9hbai
…races-event-attribute-cap # Conflicts: # .changeset/node-span-limits.md
…races-event-attribute-cap # Conflicts: # .changeset/node-distributed-tracing.md
…races-event-attribute-cap
The cap is not one of the knobs the traces spec enumerates, so it stays a fixed 128 instead of a public `maxAttributesPerEvent` option. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HQy8eimnfV3jVQKbzELc65
Contributor
Replay incident risk checkThis diff touches code involved in past incidents. This is a heads-up, not a verdict: read the matched sections of INCIDENTS.md and answer their review questions before merging. For a judgment on whether this diff has the same failure mode, run the |
Contributor
posthog-node Compliance ReportDate: 2026-09-04 21:55:04 UTC ✅ All Tests Passed!111/111 tests passed Capture_V1 Tests✅ 94/94 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
|
Contributor
posthog-js Compliance ReportDate: 2026-09-04 22:01:40 UTC ✅ All Tests Passed!26/26 tests passed Capture Tests✅ 26/26 tests passed View Details
|
marandaneto
approved these changes
Sep 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
maxAttributesPerSpancounts a span's own attributes and does not reach inside its events, so #4584 bounded a span's width but not its events'.addEventcaps how many events a span carries and truncates each value tomaxAttributeValueLength, and nothing caps how many attributes one event holds. The resident worst case is 128 events × unbounded attributes × 8 KB, held on the span record until flush.This was raised on #4579 (thread) and answered with "I've filed that on #4584". No such comment exists — the id in that reply 404s, and #4584 has since merged with no cap. This PR is that missing follow-up.
Not a spec deviation:
openspec/specs/traces/spec.md"Span limits" requires 128 attributes and 128 events per span and is silent on per-event, so the merged code conforms. What is absent is OpenTelemetry'sAttributeCountPerEventCountLimit. A companionsdk-specsproposal covers the contract side — see Follow-ups.Changes
traces.maxAttributesPerEvent, default 128, resolved through the samepositiveIntegercoercion as its siblings (a fraction takes the default rather than being floored).addEvent, and re-applied inapplySpanLimitsafterbeforeSpanSend— a hook can widen an event as freely as it can add one, and neither goes throughaddEvent.Span.Event.dropped_attributes_count, coerced on the way to the wire like the span's own counters. Rolling it into the span-leveldroppedAttributesCountwould have misreported span-attribute drops.assignUserAttributesthentruncateAttributesover the same bag. The cap is now spent before a value is bounded, so keys past it are never read and a wide bag pays no getters on entries about to be dropped — the order_writeAttributealready uses.SpanRecorddeclares the richer event type, so the counter stays off the hook-visible shape like the span-level counts.No new changeset. The existing
node-span-limits.mdentry listed three options and would have shipped release notes that never name the fourth, so that line now names it.Stacking: based on
feat/traces-node-mvp(#4579), parallel to #4726. Traces is unreleased, so this ships at launch rather than as a post-release minor. Overlap with #4726 istraces/config.tsandtraces/types.ts; I'll rebase if it lands first.Release info Sub-libraries affected
Libraries affected
@posthog/coreand@posthog/typescarry the change;posthog-nodebumps as a dependent. Both are already declared in the existingnode-span-limits.mdchangeset on the base branch.Checklist
If releasing new changes
pnpm changesetto generate a changeset fileCovered by the existing base-branch entry, amended rather than duplicated — traces has never shipped, so a feature and its follow-ups are one changelog entry.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Claude Code (Opus 5), session. Found during a post-merge sanity review of #4579 after six PRs merged into it.
Decisions along the way:
Span.Event.dropped_attributes_countfor exactly this; folding event drops into the span's counter would have made span-attribute drops unreadable.exception.*attributes. feat(node): beforeSpanSend hook and per-span limits #4584 removed an analogous reserve for exception events on the grounds that it bought an unprovable case at the cost of the most intricate code in the change. Re-adding the same shape here would have repeated that. A test pins the current behaviour so the choice is visible if it turns out to matter.maxAttributesPerEventappeared nowhere, and fix(core): honor Retry-After on the OTLP export queues #4726 does not close it either (its own config comment documents the gap).Verification run locally:
packages/core1539 pass (63 suites),packages/node995 pass under both the default and edge vitest configs,turbo buildclean, oxlint and oxfmt clean, andpnpm generate-references && pnpm check:public-apigreen with the reference files unchanged. No manual/device testing was done.Follow-ups (not in this PR)
sdk-specsproposal. "Span limits" needs the per-event cap, and "Span data model" enumerates an event asname/timeUnixNano/attributes, which the new wire counter extends. Not yet written — that repo is on an unrelated branch with uncommitted work.1f1c33f0d(pre-send body-size guard) and the Retry-After work, neither of which is onfeat/traces-node-mvp. The stale2 MB body capcomments intraces/config.tsare in the same position. If feat(node): distributed tracing spans #4579 reaches main before fix(core): honor Retry-After on the OTLP export queues #4726, those resolutions are not true.🤖 Generated with Claude Code
https://claude.ai/code/session_018AAPCyRnC5HbukEZV9hbai