diff --git a/.changeset/node-span-limits.md b/.changeset/node-span-limits.md new file mode 100644 index 0000000000..c1775462e7 --- /dev/null +++ b/.changeset/node-span-limits.md @@ -0,0 +1,7 @@ +--- +'posthog-node': minor +'@posthog/core': minor +'@posthog/types': minor +--- + +Cap spans at 128 user attributes and 128 events, configurable with `traces.maxAttributesPerSpan` and `traces.maxEventsPerSpan`. diff --git a/packages/core/src/traces/index.spec.ts b/packages/core/src/traces/index.spec.ts index c8405090c7..e9a038f6c5 100644 --- a/packages/core/src/traces/index.spec.ts +++ b/packages/core/src/traces/index.spec.ts @@ -18,6 +18,8 @@ const resolveForTest = (partial?: Partial): ResolvedTraces flushIntervalMs: 5000, maxExportBatchSize: 512, maxQueueSize: 2048, + maxAttributesPerSpan: 128, + maxEventsPerSpan: 128, ...partial, }) @@ -498,6 +500,157 @@ describe('PostHogTraces', () => { }) }) + describe('span limits', () => { + it('keeps the earliest attributes and counts the rest', async () => { + const traces = createTraces({ maxAttributesPerSpan: 3 }) + const span = traces.startSpan('checkout') + for (let i = 0; i < 5; i++) { + span.setAttribute(`key-${i}`, i) + } + span.end() + await traces.flush() + + const [sent] = sentSpans() + expect(sent.attributes!.map((attribute) => attribute.key)).toEqual(['key-0', 'key-1', 'key-2']) + expect(sent.droppedAttributesCount).toBe(2) + }) + + it('never evicts the auto-context keys', async () => { + context = { distinctId: 'alice', sessionId: 'session-1' } + const traces = createTraces({ maxAttributesPerSpan: 1 }) + const span = traces.startSpan('checkout') + for (let i = 0; i < 5; i++) { + span.setAttribute(`key-${i}`, i) + } + span.end() + await traces.flush() + + const keys = sentSpans()[0].attributes!.map((attribute) => attribute.key) + expect(keys).toEqual(expect.arrayContaining(['posthogDistinctId', 'sessionId', 'key-0'])) + expect(keys).not.toContain('key-1') + }) + + it('caps events and counts the rest', async () => { + const traces = createTraces({ maxEventsPerSpan: 2 }) + const span = traces.startSpan('checkout') + for (let i = 0; i < 4; i++) { + span.addEvent(`event-${i}`) + } + span.end() + await traces.flush() + + const [sent] = sentSpans() + expect(sent.events!.map((event) => event.name)).toEqual(['event-0', 'event-1']) + expect(sent.droppedEventsCount).toBe(2) + }) + + it('lets a caller overwrite an attribute it already set while at the cap', async () => { + const traces = createTraces({ maxAttributesPerSpan: 1 }) + const span = traces.startSpan('checkout') + span.setAttribute('plan', 'free') + span.setAttribute('plan', 'pro') + span.end() + await traces.flush() + + const [sent] = sentSpans() + expect(sent.attributes).toEqual([{ key: 'plan', value: { stringValue: 'pro' } }]) + expect(sent.droppedAttributesCount).toBeUndefined() + }) + + it('omits the counters when nothing was dropped', async () => { + const traces = createTraces() + traces.startSpan('checkout', { attributes: { plan: 'pro' } }).end() + await traces.flush() + + const [sent] = sentSpans() + expect(sent).not.toHaveProperty('droppedAttributesCount') + expect(sent).not.toHaveProperty('droppedEventsCount') + }) + + it('counts a parsed __proto__ key against the cap instead of smuggling it through', async () => { + // JSON.parse produces an own `__proto__` key; a plain object store would + // swap its prototype and leak every nested key past the cap. + const traces = createTraces({ maxAttributesPerSpan: 2 }) + const parsed = JSON.parse('{"__proto__": {"leaked": 1}, "orderId": "abc"}') + traces.startSpan('checkout', { attributes: parsed }).end() + await traces.flush() + + const keys = sentSpans()[0].attributes!.map((attribute) => attribute.key) + expect(keys).not.toContain('leaked') + expect(keys).toContain('orderId') + }) + + it('does not let reserved property names bypass the cap', async () => { + const traces = createTraces({ maxAttributesPerSpan: 1 }) + const span = traces.startSpan('checkout') + span.setAttribute('kept', 1) + span.setAttribute('toString', 'nope') + span.setAttribute('constructor', 'nope') + span.end() + await traces.flush() + + const [sent] = sentSpans() + expect(sent.attributes!.map((attribute) => attribute.key)).toEqual(['kept']) + expect(sent.droppedAttributesCount).toBe(2) + }) + + it('does not spend cap budget on values that are dropped at encode time', async () => { + const traces = createTraces({ maxAttributesPerSpan: 2 }) + const span = traces.startSpan('checkout') + span.setAttribute('skipped-a', undefined) + span.setAttribute('skipped-b', null) + span.setAttribute('orderId', 'abc-123') + span.end() + await traces.flush() + + const [sent] = sentSpans() + expect(sent.attributes!.map((attribute) => attribute.key)).toEqual(['orderId']) + expect(sent.droppedAttributesCount).toBeUndefined() + }) + + it('still caps a key first seen with an optional value', async () => { + const traces = createTraces({ maxAttributesPerSpan: 2 }) + const span = traces.startSpan('checkout') + for (let i = 0; i < 5; i++) { + span.setAttribute(`field-${i}`, undefined) + } + for (let i = 0; i < 5; i++) { + span.setAttribute(`field-${i}`, i) + } + span.end() + await traces.flush() + + const [sent] = sentSpans() + expect(sent.attributes).toHaveLength(2) + expect(sent.droppedAttributesCount).toBe(3) + }) + + it('clears a key that is set back to null', async () => { + const traces = createTraces({ maxAttributesPerSpan: 2 }) + const span = traces.startSpan('checkout') + span.setAttribute('orderId', 'abc-123') + span.setAttribute('orderId', null) + span.setAttribute('a', 1) + span.setAttribute('b', 2) + span.end() + await traces.flush() + + const [sent] = sentSpans() + expect(sent.attributes!.map((attribute) => attribute.key)).toEqual(['a', 'b']) + expect(sent.droppedAttributesCount).toBeUndefined() + }) + + it('caps attributes supplied at start', async () => { + const traces = createTraces({ maxAttributesPerSpan: 2 }) + traces.startSpan('checkout', { attributes: { a: 1, b: 2, c: 3 } }).end() + await traces.flush() + + const [sent] = sentSpans() + expect(sent.attributes!.map((attribute) => attribute.key)).toEqual(['a', 'b']) + expect(sent.droppedAttributesCount).toBe(1) + }) + }) + describe('export', () => { it('flushes when the queue reaches the batch size', async () => { const traces = createTraces({ maxExportBatchSize: 2 }) diff --git a/packages/core/src/traces/index.ts b/packages/core/src/traces/index.ts index 931b8356f0..a8094f5612 100644 --- a/packages/core/src/traces/index.ts +++ b/packages/core/src/traces/index.ts @@ -78,6 +78,8 @@ export class PostHogTraces { const now = Date.now() const startTime = resolveStartTime(options?.startTime, now, this._logger) + const autoAttributes = this._autoContextAttributes() + return new PostHogSpan( { traceId: parent?.traceId ?? newTraceId(), @@ -87,7 +89,10 @@ export class PostHogTraces { name: sanitizeName(name, 'Span name', this._logger), kind: options?.kind ?? 'internal', // Auto-context first so user-supplied attributes win on collision. - attributes: assignUserAttributes(this._autoContextAttributes(), options?.attributes), + attributes: assignUserAttributes({ ...autoAttributes }, options?.attributes), + autoAttributeKeys: Object.keys(autoAttributes), + maxAttributes: this._config.maxAttributesPerSpan, + maxEvents: this._config.maxEventsPerSpan, startTime, backdated: startTime !== now, }, diff --git a/packages/core/src/traces/otlp.spec.ts b/packages/core/src/traces/otlp.spec.ts index 5e4703cb31..497f203610 100644 --- a/packages/core/src/traces/otlp.spec.ts +++ b/packages/core/src/traces/otlp.spec.ts @@ -118,6 +118,8 @@ describe('OTLP span encoding', () => { flushIntervalMs: 5000, maxExportBatchSize: 512, maxQueueSize: 2048, + maxAttributesPerSpan: 128, + maxEventsPerSpan: 128, ...partial, }) diff --git a/packages/core/src/traces/otlp.ts b/packages/core/src/traces/otlp.ts index ec78677437..0a43ce4745 100644 --- a/packages/core/src/traces/otlp.ts +++ b/packages/core/src/traces/otlp.ts @@ -87,6 +87,12 @@ export function buildOtlpSpan(record: SpanRecord, logger?: Logger): OtlpSpan { if (record.events.length) { span.events = record.events.map((event) => toOtlpEvent(event, logger)) } + if (record.droppedAttributesCount) { + span.droppedAttributesCount = record.droppedAttributesCount + } + if (record.droppedEventsCount) { + span.droppedEventsCount = record.droppedEventsCount + } // An unset status is omitted rather than sent as code 0. if (record.status) { span.status = { diff --git a/packages/core/src/traces/span.spec.ts b/packages/core/src/traces/span.spec.ts index f160c555fe..5195cd62af 100644 --- a/packages/core/src/traces/span.spec.ts +++ b/packages/core/src/traces/span.spec.ts @@ -21,6 +21,9 @@ describe('PostHogSpan', () => { attributes: {}, startTime: Date.now(), backdated: false, + autoAttributeKeys: [], + maxAttributes: 128, + maxEvents: 128, ...init, }, (record) => ended.push(record), @@ -284,6 +287,59 @@ describe('NoopSpan', () => { }) }) +describe('attribute store', () => { + it('hands out a record whose attributes behave like an ordinary object', () => { + const ended: SpanRecord[] = [] + const span = new PostHogSpan( + { + traceId: TRACE_ID, + spanId: SPAN_ID, + name: 'checkout', + kind: 'internal', + attributes: { plan: 'pro' }, + startTime: Date.now(), + backdated: false, + autoAttributeKeys: [], + maxAttributes: 128, + maxEvents: 128, + }, + (record) => ended.push(record) + ) + span.end() + + const { attributes } = ended[0] + expect(Object.getPrototypeOf(attributes)).toBe(Object.prototype) + expect(Object.prototype.hasOwnProperty.call(attributes, 'plan')).toBe(true) + expect(() => JSON.stringify(attributes)).not.toThrow() + }) + + it('keeps a parsed __proto__ key as an ordinary attribute', () => { + const ended: SpanRecord[] = [] + const span = new PostHogSpan( + { + traceId: TRACE_ID, + spanId: SPAN_ID, + name: 'checkout', + kind: 'internal', + attributes: JSON.parse('{"__proto__": {"leaked": 1}, "orderId": "abc"}'), + startTime: Date.now(), + backdated: false, + autoAttributeKeys: [], + maxAttributes: 128, + maxEvents: 128, + }, + (record) => ended.push(record) + ) + span.end() + + const keys: string[] = [] + for (const key in ended[0].attributes) { + keys.push(key) + } + expect(keys).not.toContain('leaked') + }) +}) + describe('describeError', () => { it.each([ ['an Error', new Error('boom'), { type: 'Error', message: 'boom' }], diff --git a/packages/core/src/traces/span.ts b/packages/core/src/traces/span.ts index dc2c336d61..63aeeacd41 100644 --- a/packages/core/src/traces/span.ts +++ b/packages/core/src/traces/span.ts @@ -3,7 +3,7 @@ import type { Logger } from '../types' import type { SpanEventRecord, SpanRecord } from './types' import { formatTraceparent } from './traceparent' import { assignUserAttributes, clampEndTime, resolveSuppliedTime, sanitizeName } from './sanitize' -import { isError } from '../utils' +import { isError, isNullish } from '../utils' /** * A monotonic millisecond reading where the platform has one, so an NTP @@ -26,6 +26,10 @@ export interface SpanInit { startTime: number /** True when the caller supplied an explicit `startTime`. */ backdated: boolean + /** Keys the SDK attached itself. Exempt from the attribute cap and never evicted. */ + autoAttributeKeys: string[] + maxAttributes: number + maxEvents: number } export class PostHogSpan implements Span { @@ -43,6 +47,12 @@ export class PostHogSpan implements Span { private _events: SpanEventRecord[] = [] private _status?: { code: SpanStatusCode; message?: string } private _ended = false + private readonly _autoKeys: Set + private readonly _maxAttributes: number + private readonly _maxEvents: number + private _userAttributeCount = 0 + private _droppedAttributes = 0 + private _droppedEvents = 0 constructor( init: SpanInit, @@ -55,7 +65,16 @@ export class PostHogSpan implements Span { this._traceState = init.traceState this._name = init.name this._kind = init.kind - this._attributes = init.attributes + this._autoKeys = new Set(init.autoAttributeKeys) + this._maxAttributes = init.maxAttributes + this._maxEvents = init.maxEvents + // Null-prototype: a `__proto__` key would otherwise swap this object's prototype + // instead of becoming an entry, and `toString` and friends would read as + // already-present. + this._attributes = Object.create(null) as SpanAttributes + for (const key in init.attributes) { + this._writeAttribute(key, init.attributes[key]) + } this._startTime = init.startTime this._startMono = init.backdated ? undefined : monotonicNow() } @@ -83,22 +102,60 @@ export class PostHogSpan implements Span { return true } + /** + * Writes an attribute unless the span is already at its user-attribute cap. + * + * Overwriting a key already on the span always succeeds — the cap counts + * distinct user keys, not writes — and SDK-attached keys never count toward + * it, so a span at the cap still carries its person and session ids. + */ + private _writeAttribute(key: string, value: SpanAttributeValue): void { + // Nullish removes the key rather than occupying it: storing one would spend no + // budget and make every later write to that key free, exceeding the cap. + if (isNullish(value)) { + if (key in this._attributes && !this._autoKeys.has(key)) { + this._userAttributeCount-- + } + delete this._attributes[key] + return + } + if (this._autoKeys.has(key) || key in this._attributes) { + this._attributes[key] = value + return + } + if (this._userAttributeCount >= this._maxAttributes) { + this._droppedAttributes++ + return + } + this._userAttributeCount++ + this._attributes[key] = value + } + setAttribute(key: string, value: SpanAttributeValue): this { if (this._mutable('setAttribute')) { - Object.defineProperty(this._attributes, key, { value, enumerable: true, writable: true, configurable: true }) + this._writeAttribute(key, value) } return this } setAttributes(attributes: SpanAttributes): this { if (this._mutable('setAttributes')) { - assignUserAttributes(this._attributes, attributes) + // Read through the shared guard first — own enumerable keys only, and a + // throwing getter costs its own key — then write each through the cap. + const safe: SpanAttributes = assignUserAttributes({}, attributes) + for (const key of Object.keys(safe)) { + this._writeAttribute(key, safe[key]) + } } return this } addEvent(name: string, attributes?: SpanAttributes, timestamp?: SpanTimeInput): this { if (this._mutable('addEvent')) { + if (this._events.length >= this._maxEvents) { + this._droppedEvents++ + return this + } this._events.push({ name: sanitizeName(name, 'Span event name', this._logger), timestamp: resolveSuppliedTime(timestamp, this._now(), 'event timestamp', this._logger), @@ -177,10 +234,14 @@ export class PostHogSpan implements Span { name: this._name, kind: this._kind, ...(this._status && { status: this._status }), - attributes: this._attributes, + // Copied out with an ordinary prototype: the store is null-prototype, but a + // record handed to user code should behave like a normal object. + attributes: { ...this._attributes }, events: this._events, startTime: this._startTime, endTime: clampEndTime(resolved, this._startTime), + ...(this._droppedAttributes && { droppedAttributesCount: this._droppedAttributes }), + ...(this._droppedEvents && { droppedEventsCount: this._droppedEvents }), }) } } diff --git a/packages/core/src/traces/types.ts b/packages/core/src/traces/types.ts index 8ec3556471..5a54a0c17f 100644 --- a/packages/core/src/traces/types.ts +++ b/packages/core/src/traces/types.ts @@ -76,6 +76,8 @@ export interface SpanRecord { /** ms epoch. */ startTime: number endTime: number + droppedAttributesCount?: number + droppedEventsCount?: number } /** @@ -102,4 +104,6 @@ export interface ResolvedTracesConfig extends TracesConfig { * dropped rather than queued ones, whose children may already have shipped. */ maxQueueSize: number + maxAttributesPerSpan: number + maxEventsPerSpan: number } diff --git a/packages/node/src/__tests__/traces-defaults.spec.ts b/packages/node/src/__tests__/traces-defaults.spec.ts index bf5b150758..de9afde94f 100644 --- a/packages/node/src/__tests__/traces-defaults.spec.ts +++ b/packages/node/src/__tests__/traces-defaults.spec.ts @@ -1,11 +1,23 @@ import { resolveTracesConfig } from '../traces-defaults' describe('resolveTracesConfig', () => { + it.each([ + ['zero', 0], + ['negative', -1], + ['not a number', NaN], + ])('falls back to the default per-span caps when given %s', (_label, value) => { + const resolved = resolveTracesConfig({ maxAttributesPerSpan: value, maxEventsPerSpan: value }) + expect(resolved.maxAttributesPerSpan).toBe(128) + expect(resolved.maxEventsPerSpan).toBe(128) + }) + it('applies the documented defaults', () => { expect(resolveTracesConfig(undefined)).toMatchObject({ flushIntervalMs: 5000, maxExportBatchSize: 512, maxQueueSize: 2048, + maxAttributesPerSpan: 128, + maxEventsPerSpan: 128, }) }) diff --git a/packages/node/src/__tests__/traces.spec.ts b/packages/node/src/__tests__/traces.spec.ts index 2a6c37483f..d879662e55 100644 --- a/packages/node/src/__tests__/traces.spec.ts +++ b/packages/node/src/__tests__/traces.spec.ts @@ -319,6 +319,36 @@ describe('PostHog traces', () => { }) }) + describe('span limits', () => { + it('caps attributes and reports how many were dropped', async () => { + const client = createClient({ traces: { serviceName: 'svc', maxAttributesPerSpan: 2 } }) + const span = client.startSpan('checkout') + span.setAttribute('a', 1) + span.setAttribute('b', 2) + span.setAttribute('c', 3) + span.end() + await client.shutdown() + + const [sent] = sentSpans() + expect(sent.attributes?.map((a) => a.key)).toEqual(['a', 'b']) + expect(sent.droppedAttributesCount).toBe(1) + }) + + it('defaults to the OpenTelemetry cap of 128', async () => { + const client = createClient({ traces: { serviceName: 'svc' } }) + const span = client.startSpan('checkout') + for (let i = 0; i < 130; i++) { + span.setAttribute(`key-${i}`, i) + } + span.end() + await client.shutdown() + + const [sent] = sentSpans() + expect(sent.attributes).toHaveLength(128) + expect(sent.droppedAttributesCount).toBe(2) + }) + }) + describe('shutdown', () => { it('drains queued spans', async () => { posthog.startSpan('a').end() diff --git a/packages/node/src/traces-defaults.ts b/packages/node/src/traces-defaults.ts index 791f42d8bb..c30eec7d43 100644 --- a/packages/node/src/traces-defaults.ts +++ b/packages/node/src/traces-defaults.ts @@ -5,6 +5,9 @@ import type { ResolvedTracesConfig, TracesConfig } from '@posthog/core' const DEFAULT_FLUSH_INTERVAL_MS = 5000 const DEFAULT_MAX_EXPORT_BATCH_SIZE = 512 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 /** * Coerces a caller-supplied positive-integer option. `0`, a negative, or `NaN` @@ -27,6 +30,8 @@ export function resolveTracesConfig(config: TracesConfig | undefined): ResolvedT serviceVersion: (resourceAttributes?.['service.version'] as string | undefined) ?? config?.serviceVersion, environment: (resourceAttributes?.['deployment.environment'] as string | undefined) ?? config?.environment, resourceAttributes, + maxAttributesPerSpan: positiveInteger(config?.maxAttributesPerSpan, DEFAULT_MAX_ATTRIBUTES_PER_SPAN), + maxEventsPerSpan: positiveInteger(config?.maxEventsPerSpan, DEFAULT_MAX_EVENTS_PER_SPAN), flushIntervalMs: positiveInteger(config?.flushIntervalMs, DEFAULT_FLUSH_INTERVAL_MS), maxExportBatchSize, // Never below the flush trigger, or the depth-based flush could never fire. diff --git a/packages/types/src/traces.ts b/packages/types/src/traces.ts index 678889edfa..9fbf591bad 100644 --- a/packages/types/src/traces.ts +++ b/packages/types/src/traces.ts @@ -230,6 +230,28 @@ export interface TracesConfig { * @default 2048 */ maxQueueSize?: number + + /** + * Maximum user-supplied attributes on a single span. Attributes the SDK + * attaches itself — `posthogDistinctId`, `sessionId` and friends — are + * exempt and are never evicted, because they are what links a span to a + * person and a session. + * + * On overflow the earliest-set attributes are kept and later ones are + * dropped, with the number dropped reported on the exported span. + * + * @default 128 + */ + maxAttributesPerSpan?: number + + /** + * Maximum events on a single span. On overflow the earliest events are kept + * and later ones are dropped, with the number dropped reported on the + * exported span. + * + * @default 128 + */ + maxEventsPerSpan?: number } // ============================================================================ @@ -273,6 +295,10 @@ export interface OtlpSpan { status?: OtlpSpanStatus /** W3C trace flags in the low byte; the sampled bit is always set. */ flags?: number + /** User attributes dropped by `maxAttributesPerSpan`. Omitted when none were. */ + droppedAttributesCount?: number + /** Events dropped by `maxEventsPerSpan`. Omitted when none were. */ + droppedEventsCount?: number } export interface OtlpTracesPayload {