diff --git a/packages/core/src/traces/config.spec.ts b/packages/core/src/traces/config.spec.ts index 5237a0d52d..eaef6082bc 100644 --- a/packages/core/src/traces/config.spec.ts +++ b/packages/core/src/traces/config.spec.ts @@ -31,6 +31,11 @@ describe('resolveTracesConfig', () => { }) }) + it('keeps the per-event attribute cap fixed', () => { + // The cap is internal, so an untyped caller naming it gets the default. + expect(resolveTracesConfig({ maxAttributesPerEvent: 9 } as any).maxAttributesPerEvent).toBe(128) + }) + it('applies the documented defaults', () => { expect(resolveTracesConfig(undefined)).toMatchObject({ flushIntervalMs: 5000, @@ -38,6 +43,7 @@ describe('resolveTracesConfig', () => { maxQueueSize: 2048, maxAttributesPerSpan: 128, maxEventsPerSpan: 128, + maxAttributesPerEvent: 128, maxAttributeValueLength: 8192, maxLiveSpans: 10_000, maxSpanAgeMs: 3_600_000, diff --git a/packages/core/src/traces/config.ts b/packages/core/src/traces/config.ts index d0c6143fde..cf7faca372 100644 --- a/packages/core/src/traces/config.ts +++ b/packages/core/src/traces/config.ts @@ -11,6 +11,11 @@ const DEFAULT_MAX_QUEUE_SIZE = 2048 // OpenTelemetry's per-span defaults. const DEFAULT_MAX_ATTRIBUTES_PER_SPAN = 128 const DEFAULT_MAX_EVENTS_PER_SPAN = 128 +// OpenTelemetry's per-event limit, which is the same number. Fixed rather than +// configurable: `maxAttributesPerSpan` and `maxEventsPerSpan` already give a +// caller room to shape a span, and this one only has to stop an event holding +// an unbounded bag. +const DEFAULT_MAX_ATTRIBUTES_PER_EVENT = 128 // OpenTelemetry leaves the value length unlimited, which is what lets one // multi-MB attribute make a span too large for the endpoint to accept — and an // oversized span is dropped whole. 8 KB holds a deep stack trace and any @@ -121,6 +126,7 @@ export function resolveTracesConfig( beforeSpanSend: resolveBeforeSpanSend(config?.beforeSpanSend, logger), maxAttributesPerSpan: positiveInteger(config?.maxAttributesPerSpan, DEFAULT_MAX_ATTRIBUTES_PER_SPAN), maxEventsPerSpan: positiveInteger(config?.maxEventsPerSpan, DEFAULT_MAX_EVENTS_PER_SPAN), + maxAttributesPerEvent: DEFAULT_MAX_ATTRIBUTES_PER_EVENT, maxAttributeValueLength: positiveInteger(config?.maxAttributeValueLength, DEFAULT_MAX_ATTRIBUTE_VALUE_LENGTH), flushIntervalMs: positiveInteger(config?.flushIntervalMs, DEFAULT_FLUSH_INTERVAL_MS), maxExportBatchSize, diff --git a/packages/core/src/traces/index.spec.ts b/packages/core/src/traces/index.spec.ts index 0712ea99f8..696620e79f 100644 --- a/packages/core/src/traces/index.spec.ts +++ b/packages/core/src/traces/index.spec.ts @@ -22,6 +22,7 @@ const resolveForTest = (partial?: Partial): ResolvedTraces beforeSpanSend: [], maxAttributesPerSpan: 128, maxEventsPerSpan: 128, + maxAttributesPerEvent: 128, maxAttributeValueLength: 8192, maxLiveSpans: 10000, maxSpanAgeMs: 3600000, @@ -1509,6 +1510,26 @@ describe('PostHogTraces', () => { expect(sent.droppedEventsCount).toBe(1) }) + it('re-applies the event attribute cap to what beforeSpanSend widened', async () => { + const traces = createTraces({ + maxAttributesPerEvent: 2, + beforeSpanSend: [ + (span) => { + span.events[0].attributes = { a: 1, b: 2, c: 3, d: 4 } + return span + }, + ], + }) + const span = traces.startSpan('checkout') + span.addEvent('query', { a: 1 }) + span.end() + await traces.flush() + + const event = sentSpans()[0].events![0] + expect(event.attributes!.map((attribute) => attribute.key)).toEqual(['a', 'b']) + expect(event.droppedAttributesCount).toBe(2) + }) + it('keeps the auto-context keys when beforeSpanSend pushes past the cap', async () => { context = { distinctId: 'alice', sessionId: 'session-1' } const traces = createTraces({ diff --git a/packages/core/src/traces/index.ts b/packages/core/src/traces/index.ts index fca29ebaee..95547c32ee 100644 --- a/packages/core/src/traces/index.ts +++ b/packages/core/src/traces/index.ts @@ -246,6 +246,7 @@ export class PostHogTraces { autoAttributeKeys: Object.keys(autoAttributes), maxAttributes: this._config.maxAttributesPerSpan, maxEvents: this._config.maxEventsPerSpan, + maxAttributesPerEvent: this._config.maxAttributesPerEvent, maxAttributeValueLength: this._config.maxAttributeValueLength, startTime, backdated: startTime !== now, @@ -654,6 +655,7 @@ export class PostHogTraces { autoKeys, this._config.maxAttributesPerSpan, this._config.maxEventsPerSpan, + this._config.maxAttributesPerEvent, this._config.maxAttributeValueLength, keysBeforeHook ) diff --git a/packages/core/src/traces/live-spans.spec.ts b/packages/core/src/traces/live-spans.spec.ts index 47ab14c5d9..3e9815bafe 100644 --- a/packages/core/src/traces/live-spans.spec.ts +++ b/packages/core/src/traces/live-spans.spec.ts @@ -23,6 +23,7 @@ describe('live spans', () => { beforeSpanSend: [], maxAttributesPerSpan: 128, maxEventsPerSpan: 128, + maxAttributesPerEvent: 128, maxAttributeValueLength: 8192, } diff --git a/packages/core/src/traces/otlp.spec.ts b/packages/core/src/traces/otlp.spec.ts index 641171df2d..4c926668f3 100644 --- a/packages/core/src/traces/otlp.spec.ts +++ b/packages/core/src/traces/otlp.spec.ts @@ -110,6 +110,30 @@ describe('OTLP span encoding', () => { expect(span.events).toEqual([{ name: 'cache miss', timeUnixNano: '1700000000040000000' }]) }) + it('carries an event attribute drop as the proto counter', () => { + const span = buildOtlpSpan( + record({ + events: [{ name: 'query', timestamp: 1_700_000_000_040, droppedAttributesCount: 3 }], + }) + ) + expect(span.events?.[0]).toMatchObject({ name: 'query', droppedAttributesCount: 3 }) + }) + + it.each([ + ['none were dropped', 0], + ['a hook wrote a negative', -2], + ['a hook wrote a non-number', 'lots' as unknown as number], + ])('omits the event drop counter when %s', (_label, dropped) => { + // Coerced like the span's own counters: a non-integer here is refused for + // the whole request, taking unrelated spans with it. + const span = buildOtlpSpan( + record({ + events: [{ name: 'query', timestamp: 1_700_000_000_040, droppedAttributesCount: dropped }], + }) + ) + expect(span.events?.[0].droppedAttributesCount).toBeUndefined() + }) + it('sets the sampled bit and marks a root span as known-not-remote', () => { // A root span has no parent context to be remote, which the OTel Go and // Java exporters also report as known-not-remote rather than unknown. @@ -143,6 +167,7 @@ describe('OTLP span encoding', () => { beforeSpanSend: [], maxAttributesPerSpan: 128, maxEventsPerSpan: 128, + maxAttributesPerEvent: 128, maxAttributeValueLength: 8192, maxLiveSpans: 10000, maxSpanAgeMs: 3600000, diff --git a/packages/core/src/traces/otlp.ts b/packages/core/src/traces/otlp.ts index 4d93be4a14..0ca8203293 100644 --- a/packages/core/src/traces/otlp.ts +++ b/packages/core/src/traces/otlp.ts @@ -101,6 +101,10 @@ function toOtlpEvent(event: SpanRecord['events'][number], logger?: Logger): Otlp encoded.attributes = attributes } } + const dropped = nonNegativeCount(event.droppedAttributesCount) + if (dropped) { + encoded.droppedAttributesCount = dropped + } return encoded } diff --git a/packages/core/src/traces/span.spec.ts b/packages/core/src/traces/span.spec.ts index 2039adc43a..2733952119 100644 --- a/packages/core/src/traces/span.spec.ts +++ b/packages/core/src/traces/span.spec.ts @@ -26,6 +26,7 @@ describe('PostHogSpan', () => { autoAttributeKeys: [], maxAttributes: 128, maxEvents: 128, + maxAttributesPerEvent: 128, maxAttributeValueLength: 8192, ...init, }, @@ -227,6 +228,101 @@ describe('PostHogSpan', () => { }) }) + describe('event attribute cap', () => { + it('keeps the first attributes and reports the rest as dropped', () => { + const span = createSpan({ maxAttributesPerEvent: 2 }) + span.addEvent('query', { a: 1, b: 2, c: 3, d: 4 }) + span.end() + + expect(ended[0].events[0].attributes).toEqual({ a: 1, b: 2 }) + expect(ended[0].events[0].droppedAttributesCount).toBe(2) + }) + + it('leaves the count off an event that lost nothing', () => { + const span = createSpan({ maxAttributesPerEvent: 2 }) + span.addEvent('query', { a: 1, b: 2 }) + span.end() + + expect(ended[0].events[0].droppedAttributesCount).toBeUndefined() + }) + + it('bounds an exception event like any other', () => { + // The SDK's own `exception.*` attributes are width the caller sees too, so + // they spend the cap rather than being exempt from it. + const span = createSpan({ maxAttributesPerEvent: 1 }) + span.recordException(new Error('boom')) + span.end() + + expect(Object.keys(ended[0].events[0].attributes ?? {})).toEqual(['exception.type']) + expect(ended[0].events[0].droppedAttributesCount).toBe(2) + }) + + it('does not read a value past the cap', () => { + // The cap is spent before the value is bounded, so a wide bag does not pay + // for getters on entries that are about to be dropped. + const read: string[] = [] + const watched: any = {} + for (const key of ['a', 'b', 'c']) { + Object.defineProperty(watched, key, { + enumerable: true, + get() { + read.push(key) + return key + }, + }) + } + + const span = createSpan({ maxAttributesPerEvent: 2 }) + span.addEvent('query', watched) + span.end() + + expect(read).toEqual(['a', 'b']) + expect(ended[0].events[0].droppedAttributesCount).toBe(1) + }) + + it('does not let a nullish value spend a slot', () => { + // The encoder drops these, so a caller who blanked a value rather than + // omitting the key must not cost the event a real attribute. Same rule the + // span half of the cap already follows. + const span = createSpan({ maxAttributesPerEvent: 2 }) + span.addEvent('query', { blanked: undefined, cleared: null, real: 1, second: 2 }) + span.end() + + expect(ended[0].events[0].attributes).toEqual({ real: 1, second: 2 }) + expect(ended[0].events[0].droppedAttributesCount).toBeUndefined() + }) + + it('survives an attribute bag whose own keys cannot be read', () => { + const hostile = new Proxy( + {}, + { + ownKeys() { + throw new Error('ownKeys exploded') + }, + } + ) + const span = createSpan() + expect(() => span.addEvent('query', hostile)).not.toThrow() + expect(() => span.end()).not.toThrow() + + expect(ended[0].events[0].attributes).toEqual({}) + }) + + it('counts the span attribute cap separately from an event cap', () => { + // maxAttributesPerSpan does not reach inside events, which is the gap this + // cap closes: a span at its own cap can still carry full-width events. + const span = createSpan({ maxAttributes: 1, maxAttributesPerEvent: 3 }) + span.setAttributes({ kept: 1, dropped: 2 }) + span.addEvent('query', { a: 1, b: 2, c: 3 }) + span.end() + + expect(ended[0].attributes).toEqual({ kept: 1 }) + expect(ended[0].droppedAttributesCount).toBe(1) + expect(ended[0].events[0].attributes).toEqual({ a: 1, b: 2, c: 3 }) + expect(ended[0].events[0].droppedAttributesCount).toBeUndefined() + }) + }) + describe('attribute store hygiene', () => { it('does not copy a polluted Object.prototype key into the span', () => { ;(Object.prototype as any).polluted = 'yes' @@ -900,6 +996,7 @@ describe('attribute store', () => { autoAttributeKeys: [], maxAttributes: 128, maxEvents: 128, + maxAttributesPerEvent: 128, maxAttributeValueLength: 8192, }, (record) => ended.push(record) @@ -926,6 +1023,7 @@ describe('attribute store', () => { autoAttributeKeys: [], maxAttributes: 128, maxEvents: 128, + maxAttributesPerEvent: 128, maxAttributeValueLength: 8192, }, (record) => ended.push(record) diff --git a/packages/core/src/traces/span.ts b/packages/core/src/traces/span.ts index 9d657c0aee..caf4f42535 100644 --- a/packages/core/src/traces/span.ts +++ b/packages/core/src/traces/span.ts @@ -42,6 +42,7 @@ export interface SpanInit { autoAttributeKeys: string[] maxAttributes: number maxEvents: number + maxAttributesPerEvent: number maxAttributeValueLength: number } @@ -65,6 +66,7 @@ export class PostHogSpan implements Span { private readonly _autoKeys: Set private readonly _maxAttributes: number private readonly _maxEvents: number + private readonly _maxAttributesPerEvent: number private readonly _maxAttributeValueLength: number private _userAttributeCount = 0 private _userEventCount = 0 @@ -87,6 +89,7 @@ export class PostHogSpan implements Span { this._autoKeys = new Set(init.autoAttributeKeys) this._maxAttributes = init.maxAttributes this._maxEvents = init.maxEvents + this._maxAttributesPerEvent = init.maxAttributesPerEvent this._maxAttributeValueLength = init.maxAttributeValueLength // Null-prototype: a `__proto__` key would otherwise swap this object's prototype // instead of becoming an entry, and `toString` and friends would read as @@ -183,12 +186,15 @@ export class PostHogSpan implements Span { return this } this._userEventCount++ + // Copied so a caller reusing one object across events can't mutate a recorded one. + const bounded = + attributes && boundAttributes(attributes, this._maxAttributesPerEvent, this._maxAttributeValueLength) this._events.push({ name: sanitizeName(name, 'Span event name', this._maxAttributeValueLength, this._logger), timestamp: resolveSuppliedTime(timestamp, this._now(), 'event timestamp', this._logger), - // Copied so a caller reusing one object across events can't mutate a recorded one. - ...(attributes && { - attributes: truncateAttributes(assignUserAttributes({}, attributes), this._maxAttributeValueLength), + ...(bounded && { + attributes: bounded.attributes, + ...(bounded.dropped && { droppedAttributesCount: bounded.dropped }), }), }) } @@ -342,6 +348,7 @@ export function applySpanLimits( autoKeys: ReadonlySet, maxAttributes: number, maxEvents: number, + maxAttributesPerEvent: number, maxAttributeValueLength: number, keysBeforeHook: readonly string[] = [] ): void { @@ -391,7 +398,13 @@ export function applySpanLimits( } keptEvents++ if (event.attributes) { - event.attributes = truncateAttributes({ ...event.attributes }, maxAttributeValueLength) + // A hook can widen an event as freely as it can add one, and neither goes + // through `addEvent`. + const bounded = boundAttributes(event.attributes, maxAttributesPerEvent, maxAttributeValueLength) + event.attributes = bounded.attributes + if (bounded.dropped) { + event.droppedAttributesCount = nonNegativeCount(event.droppedAttributesCount) + bounded.dropped + } } events.push(event) } @@ -691,6 +704,54 @@ function resolveToJson(value: object): { selfDescribed: boolean; value?: SpanAtt return { selfDescribed: false } } +/** + * A copy of a caller-supplied attribute bag holding at most `max` entries, each + * value bounded to `maxLength`, plus how many entries the cap refused. + * + * Once the cap is spent the remaining keys are counted without being read, so a + * wide object does not pay for the getters on values it is about to drop. + */ +function boundAttributes( + source: SpanAttributes, + max: number, + maxLength: number +): { attributes: SpanAttributes; dropped: number } { + let keys: string[] + try { + keys = Object.keys(source) + } catch { + // A hostile own-keys trap costs the bag, not the event carrying it. + return { attributes: {}, dropped: 0 } + } + const attributes: SpanAttributes = {} + let kept = 0 + let dropped = 0 + for (const key of keys) { + if (kept >= max) { + dropped++ + continue + } + let value: SpanAttributeValue + try { + value = truncateAttributeValue(source[key], maxLength) + } catch { + // A throwing getter costs its own key, as it does in `assignUserAttributes`. + value = UNSERIALIZABLE_VALUE + } + // Nullish spends no slot, matching `_writeAttribute` and the span half of + // `applySpanLimits`: the encoder drops these, so a caller who blanked a value + // rather than omitting the key must not lose a real attribute to it. + if (isNullish(value)) { + continue + } + kept++ + // defineProperty, not assignment: `attributes['__proto__'] = v` hits the + // prototype setter and the attribute vanishes. + Object.defineProperty(attributes, key, { value, enumerable: true, writable: true, configurable: true }) + } + return { attributes, dropped } +} + /** `truncateAttributeValue` across an attribute bag, in place. */ export function truncateAttributes(attributes: SpanAttributes, maxLength: number): SpanAttributes { for (const key of Object.keys(attributes)) { diff --git a/packages/core/src/traces/types.ts b/packages/core/src/traces/types.ts index 7360c70a26..83e995f3fe 100644 --- a/packages/core/src/traces/types.ts +++ b/packages/core/src/traces/types.ts @@ -63,6 +63,7 @@ export interface SpanEventRecord { /** ms epoch. */ timestamp: number attributes?: SpanAttributes + droppedAttributesCount?: number } /** @@ -72,6 +73,8 @@ export interface SpanEventRecord { * field `beforeSpanSend` cannot see, and so cannot corrupt. */ export interface SpanRecord extends HookSpanRecord { + /** The hook-visible event plus the SDK's own per-event drop count. */ + events: SpanEventRecord[] traceState?: string /** The W3C trace-flags byte this span propagates, e.g. `01` sampled. */ traceFlags: string @@ -108,6 +111,7 @@ export interface ResolvedTracesConfig extends TracesConfig { beforeSpanSend: BeforeSpanSendFn[] maxAttributesPerSpan: number maxEventsPerSpan: number + maxAttributesPerEvent: number maxAttributeValueLength: number /** Bound on spans started but not yet ended. At the bound `startSpan` returns a no-op handle. */ maxLiveSpans: number diff --git a/packages/types/src/traces.ts b/packages/types/src/traces.ts index eaf84e2309..1881574ef1 100644 --- a/packages/types/src/traces.ts +++ b/packages/types/src/traces.ts @@ -120,6 +120,10 @@ export interface Span { /** * Record a timestamped event within the span, e.g. a cache miss or a retry. * Defaults to the current time. + * + * One event carries at most 128 attributes; further keys are dropped and + * counted on the exported event. Use `maxEventsPerSpan` to bound how many + * events a span carries. */ addEvent(name: string, attributes?: SpanAttributes, timestamp?: SpanTimeInput): this @@ -378,6 +382,8 @@ export interface OtlpSpanEvent { name: string timeUnixNano: string attributes?: OtlpSpanKeyValue[] + /** Attributes dropped by the SDK's per-event attribute cap. Omitted when none were. */ + droppedAttributesCount?: number } export interface OtlpSpanStatus {