feat(traces): per-span limits and exception stacktraces - #955
Conversation
Prompt To Fix All With AI### Issue 1
posthog/tracing/_limits.py:98
**Nested keys bypass limits**
A nested mapping with a very long key bypasses `max_attribute_value_length` because this stores the original key. The OTLP encoder later emits that key without truncation, so it can make the batch exceed the transport limit and be dropped. Store the truncated `key_str` instead.
```suggestion
bounded[truncate_string(key_str, max_length)] = item
```
---
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): per-span limits and except..." | Re-trigger Greptile |
posthog-python Compliance ReportDate: 2026-09-17 03:43:00 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
|
384a4a8 to
8de0208
Compare
8de0208 to
8088f97
Compare
f02e6b4 to
3a514f7
Compare
3a514f7 to
21a1004
Compare
|
Reviews (2): Last reviewed commit: "feat(traces): per-span limits and except..." | Re-trigger Greptile |
dustinbyrne
left a comment
There was a problem hiding this comment.
Looks good overall. Two non-blocking inline notes on null-attribute drop counts and callable encoding; neither blocks this tracing slice.
Validation was static inspection and existing CI. The proposed regression cases were not executed.
Agent-led review, human-reviewed before posting.
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
jzhu13
left a comment
There was a problem hiding this comment.
Reviewed against traces/06-export. Tests pass at the head. Limits are applied at write time, overwrites and None are accounted correctly, auto join keys are exempt, and the truncation walk mirrors the encoder's budgets in the safe direction. No blocking issues. Approving with non-blocking notes.
Non-blocking
posthog/tracing/_span.py:281an empty-string key spends a span (or event) attribute slot that the encoder then discards. Reproduced withmax_attributes=1:set_attribute("", 1).set_attribute("real", 2)stores{"": 1}, counts one drop, and ships zero attributes._truncate_mappingalready skips falsy keys for nested values, so the top level is the odd one out. Treatnot key_strlikeNone.posthog/tracing/_span.py:190tail truncation of a chained traceback keeps the outer wrapper and discards the root cause, sinceformat_exceptionprints the cause first. Reproduced:raise RuntimeError("wrapper") from ValueError("ROOT CAUSE")atmax_length=120keepswrapper, notROOT CAUSE. The testtest_bounds_the_stack_keeping_the_crash_siteactually asserts the wrapper survives while the recursive crash site is cut off, contradicting its name. Either format withchain=Falseand prepend the innermost cause's tail, or rename the test and document that the tail is the outermost exception.posthog/tracing/_limits.py:34truncate_attribute_valueallocates a_WalkStatewith asetand enters atryfor every scalar. Measured:set_attribute("k", "value")spends 0.32 µs of 0.40 µs there versus 0.03 µs for a direct slice. A fast path forstr, numbers,bool, andNonebefore the walk removes it.posthog/tracing/_span.py:317add_eventwith non-mappingattributesno longer logs. The base went throughcopy_user_attributes, which debug-logs "expected a mapping";bound_attributesreturns({}, 0)silently. Route throughcopy_user_attributes(which also removes the duplicated key walk inbound_attributes).posthog/tracing/_span.py:281same-key check-then-write across threads can permanently consume a slot. Not reproduced in 300 trials, but the_Activatabledocstring invites multi-thread use. Guarding with the existing lock is cheap.- Nits: two tests (
test_the_per_event_attribute_cap_is_opentelemetrys_default, the== 8192literal intest_config.py) only fail on an intentional constant edit;test_a_self_referencing_attribute_ends_the_spanasserts only the record count and would pass if the value were stored unbounded.
Two notes for #956: apply_span_limits inherits item 1 verbatim and re-walks every already-bounded value whenever a hook is configured, and #956 adds SpanRecord.auto_attribute_keys that this PR's end() does not populate, so the tip's end() must.
Reviewed with Claude Code (Claude Fable 5.1). Behaviors above were reproduced against this branch head.
bbc0e2c to
27b9229
Compare
|
Thanks. Fixed in 27b9229:
|
27b9229 to
1975532
Compare
|
Follow-up, 1975532: item 5 is done. The cap check, the event count and the |
Bounds what one span can hold, per the traces spec. A span keeps at most max_attributes_per_span user attributes and max_events_per_span events (128 each, earliest-set wins), and each event at most 128 attributes; what the caps refuse is reported as droppedAttributesCount / droppedEventsCount, clamped to uint32. The posthogDistinctId and sessionId join keys are exempt. max_attribute_value_length (8192) bounds every string an attribute holds, nested ones included, along with span and event names, status messages and resource attributes, so one large value cannot get a span dropped as too large. Recorded exceptions now carry exception.stacktrace, keeping the tail of the traceback where Python puts the raising frame. Not reachable from the client. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TkZAsCciW4PV8ZdcCHmAbA
…g None event attributes past the cap A callable was stringified by the value-length walk, so the OTLP encoder emitted its repr instead of the stable [Function] marker. And an event attribute bag with None past the per-event cap counted that None as a drop even though the encoder never emits it.
The encoder drops an empty key, so charging it against the cap lost a real attribute. Scalars skip the truncation walk, event attributes go through the shared copier so a non-mapping logs, and the traceback test is named for what it asserts: the outermost exception survives the cut.
A same-key write from two threads could reserve two slots, and a write that lost the race to end() could land after the record was taken. The cap check, the event count and the end() snapshot now run under the handle's lock; the value walk still runs outside it.
1975532 to
3820d7d
Compare
💡 Motivation and Context
Bounds what one span can hold, per the traces spec.
max_attributes_per_spanuser attributes andmax_events_per_spanevents (128 each, earliest-set wins); each event at most 128 attributes. Overflow is reported asdroppedAttributesCount/droppedEventsCount, clamped to uint32. TheposthogDistinctIdandsessionIdjoin keys are exempt.max_attribute_value_length(8192) bounds every string an attribute holds, nested ones included, plus span and event names, status messages and resource attributes, so one large value cannot get a span dropped as too large.exception.stacktrace, keeping the tail of the traceback where Python puts the raising frame.Not reachable from the client yet.
Stack (PR 7 of 9, based on
traces/06-export):traces/01-ids-traceparenttraces/02-otlp-encodingtraces/03-span-handlestraces/04-transporttraces/05-pipelinetraces/06-exporttraces/07-span-limits← this PRtraces/08-before-span-sendtraces/09-client-wiring💚 How did you test it?
Unit tests in
posthog/test/tracing/test_limits.py, plus additions totest_span.py,test_otlp.py,test_pipeline.pyandtest_export.py.📝 Checklist
If releasing new changes
sampo addto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Implemented with Claude Code (Claude Opus 5) against the traces spec, one commit per slice so each PR reviews on its own. Rebased onto main and opened as a stacked draft in a later Claude Code session (Claude Fable 5.1).
🤖 Generated with Claude Code
https://claude.ai/code/session_012o7CtHLfcypjmXL7g9ZGRC