Skip to content

Commit d6cbd39

Browse files
mydeamydea
committed
feat(core): Add attributes to Span
Together with `setAttribute()` and `setAttributes()` APIs, mirroring the OpenTelemetry API for their spans. For now, these are stored as `data` on spans/transactions, until we "directly" support them.
1 parent 9866412 commit d6cbd39

File tree

8 files changed

+259
-11
lines changed

8 files changed

+259
-11
lines changed

‎packages/core/src/tracing/span.ts‎

Lines changed: 57 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import type {
33
Instrumenter,
44
Primitive,
55
Span as SpanInterface,
6+
SpanAttributeValue,
7+
SpanAttributes,
68
SpanContext,
79
SpanOrigin,
810
TraceContext,
@@ -104,6 +106,11 @@ export class Span implements SpanInterface {
104106
// eslint-disable-next-line @typescript-eslint/no-explicit-any
105107
public data: { [key: string]: any };
106108

109+
/**
110+
* @inheritDoc
111+
*/
112+
public attributes: SpanAttributes;
113+
107114
/**
108115
* List of spans that were finalized
109116
*/
@@ -137,6 +144,7 @@ export class Span implements SpanInterface {
137144
this.startTimestamp = spanContext.startTimestamp || timestampInSeconds();
138145
this.tags = spanContext.tags || {};
139146
this.data = spanContext.data || {};
147+
this.attributes = spanContext.attributes || {};
140148
this.instrumenter = spanContext.instrumenter || 'sentry';
141149
this.origin = spanContext.origin || 'manual';
142150

@@ -217,12 +225,27 @@ export class Span implements SpanInterface {
217225
/**
218226
* @inheritDoc
219227
*/
220-
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types
228+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
221229
public setData(key: string, value: any): this {
222230
this.data = { ...this.data, [key]: value };
223231
return this;
224232
}
225233

234+
/** @inheritdoc */
235+
public setAttribute(key: string, value: SpanAttributeValue | undefined): void {
236+
if (value === undefined) {
237+
// eslint-disable-next-line @typescript-eslint/no-dynamic-delete
238+
delete this.attributes[key];
239+
} else {
240+
this.attributes[key] = value;
241+
}
242+
}
243+
244+
/** @inheritdoc */
245+
public setAttributes(attributes: SpanAttributes): void {
246+
Object.keys(attributes).forEach(key => this.setAttribute(key, attributes[key]));
247+
}
248+
226249
/**
227250
* @inheritDoc
228251
*/
@@ -297,7 +320,7 @@ export class Span implements SpanInterface {
297320
*/
298321
public toContext(): SpanContext {
299322
return dropUndefinedKeys({
300-
data: this.data,
323+
data: this._getData(),
301324
description: this.description,
302325
endTimestamp: this.endTimestamp,
303326
op: this.op,
@@ -335,7 +358,7 @@ export class Span implements SpanInterface {
335358
*/
336359
public getTraceContext(): TraceContext {
337360
return dropUndefinedKeys({
338-
data: Object.keys(this.data).length > 0 ? this.data : undefined,
361+
data: this._getData(),
339362
description: this.description,
340363
op: this.op,
341364
parent_span_id: this.parentSpanId,
@@ -365,7 +388,7 @@ export class Span implements SpanInterface {
365388
origin?: SpanOrigin;
366389
} {
367390
return dropUndefinedKeys({
368-
data: Object.keys(this.data).length > 0 ? this.data : undefined,
391+
data: this._getData(),
369392
description: this.description,
370393
op: this.op,
371394
parent_span_id: this.parentSpanId,
@@ -378,6 +401,36 @@ export class Span implements SpanInterface {
378401
origin: this.origin,
379402
});
380403
}
404+
405+
/**
406+
* Get the merged data for this span.
407+
* For now, this combines `data` and `attributes` together,
408+
* until eventually we can ingest `attributes` directly.
409+
*/
410+
private _getData():
411+
| {
412+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
413+
[key: string]: any;
414+
}
415+
| undefined {
416+
const { data, attributes } = this;
417+
418+
const hasData = Object.keys(data).length > 0;
419+
const hasAttributes = Object.keys(attributes).length > 0;
420+
421+
if (!hasData && !hasAttributes) {
422+
return undefined;
423+
}
424+
425+
if (hasData && hasAttributes) {
426+
return {
427+
...data,
428+
...attributes,
429+
};
430+
}
431+
432+
return hasData ? data : attributes;
433+
}
381434
}
382435

383436
export type SpanStatusType =

‎packages/core/src/tracing/trace.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
1-
import type { TransactionContext } from '@sentry/types';
1+
import type { Span, TransactionContext } from '@sentry/types';
22
import { dropUndefinedKeys, isThenable, logger, tracingContextFromHeaders } from '@sentry/utils';
33

44
import { DEBUG_BUILD } from '../debug-build';
55
import { getCurrentScope, withScope } from '../exports';
66
import type { Hub } from '../hub';
77
import { getCurrentHub } from '../hub';
88
import { hasTracingEnabled } from '../utils/hasTracingEnabled';
9-
import type { Span } from './span';
109

1110
/**
1211
* Wraps a function with a transaction/span and finishes the span after the function is done.

‎packages/core/test/lib/tracing/span.test.ts‎

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,162 @@ describe('span', () => {
2929
expect(span.name).toEqual('new name');
3030
expect(span.description).toEqual('new name');
3131
});
32+
33+
describe('setAttribute', () => {
34+
it('allows to set attributes', () => {
35+
const span = new Span();
36+
37+
span.setAttribute('str', 'bar');
38+
span.setAttribute('num', 1);
39+
span.setAttribute('zero', 0);
40+
span.setAttribute('bool', true);
41+
span.setAttribute('false', false);
42+
span.setAttribute('undefined', undefined);
43+
span.setAttribute('numArray', [1, 2]);
44+
span.setAttribute('strArray', ['aa', 'bb']);
45+
span.setAttribute('boolArray', [true, false]);
46+
span.setAttribute('arrayWithUndefined', [1, undefined, 2]);
47+
48+
expect(span.attributes).toEqual({
49+
str: 'bar',
50+
num: 1,
51+
zero: 0,
52+
bool: true,
53+
false: false,
54+
numArray: [1, 2],
55+
strArray: ['aa', 'bb'],
56+
boolArray: [true, false],
57+
arrayWithUndefined: [1, undefined, 2],
58+
});
59+
});
60+
61+
it('deletes attributes when setting to `undefined`', () => {
62+
const span = new Span();
63+
64+
span.setAttribute('str', 'bar');
65+
66+
expect(Object.keys(span.attributes).length).toEqual(1);
67+
68+
span.setAttribute('str', undefined);
69+
70+
expect(Object.keys(span.attributes).length).toEqual(0);
71+
});
72+
73+
it('disallows invalid attribute types', () => {
74+
const span = new Span();
75+
76+
/** @ts-expect-error this is invalid */
77+
span.setAttribute('str', {});
78+
79+
/** @ts-expect-error this is invalid */
80+
span.setAttribute('str', null);
81+
82+
/** @ts-expect-error this is invalid */
83+
span.setAttribute('str', [1, 'a']);
84+
});
85+
});
86+
87+
describe('setAttributes', () => {
88+
it('allows to set attributes', () => {
89+
const span = new Span();
90+
91+
const initialAttributes = span.attributes;
92+
93+
expect(initialAttributes).toEqual({});
94+
95+
const newAttributes = {
96+
str: 'bar',
97+
num: 1,
98+
zero: 0,
99+
bool: true,
100+
false: false,
101+
undefined: undefined,
102+
numArray: [1, 2],
103+
strArray: ['aa', 'bb'],
104+
boolArray: [true, false],
105+
arrayWithUndefined: [1, undefined, 2],
106+
};
107+
span.setAttributes(newAttributes);
108+
109+
expect(span.attributes).toEqual({
110+
str: 'bar',
111+
num: 1,
112+
zero: 0,
113+
bool: true,
114+
false: false,
115+
numArray: [1, 2],
116+
strArray: ['aa', 'bb'],
117+
boolArray: [true, false],
118+
arrayWithUndefined: [1, undefined, 2],
119+
});
120+
121+
expect(span.attributes).not.toBe(newAttributes);
122+
123+
span.setAttributes({
124+
num: 2,
125+
numArray: [3, 4],
126+
});
127+
128+
expect(span.attributes).toEqual({
129+
str: 'bar',
130+
num: 2,
131+
zero: 0,
132+
bool: true,
133+
false: false,
134+
numArray: [3, 4],
135+
strArray: ['aa', 'bb'],
136+
boolArray: [true, false],
137+
arrayWithUndefined: [1, undefined, 2],
138+
});
139+
});
140+
141+
it('deletes attributes when setting to `undefined`', () => {
142+
const span = new Span();
143+
144+
span.setAttribute('str', 'bar');
145+
146+
expect(Object.keys(span.attributes).length).toEqual(1);
147+
148+
span.setAttributes({ str: undefined });
149+
150+
expect(Object.keys(span.attributes).length).toEqual(0);
151+
});
152+
});
153+
154+
// Ensure that attributes & data are merged together
155+
describe('_getData', () => {
156+
it('works without data & attributes', () => {
157+
const span = new Span();
158+
159+
expect(span['_getData']()).toEqual(undefined);
160+
});
161+
162+
it('works with data only', () => {
163+
const span = new Span();
164+
span.setData('foo', 'bar');
165+
166+
expect(span['_getData']()).toEqual({ foo: 'bar' });
167+
expect(span['_getData']()).toBe(span.data);
168+
});
169+
170+
it('works with attributes only', () => {
171+
const span = new Span();
172+
span.setAttribute('foo', 'bar');
173+
174+
expect(span['_getData']()).toEqual({ foo: 'bar' });
175+
expect(span['_getData']()).toBe(span.attributes);
176+
});
177+
178+
it('merges data & attributes', () => {
179+
const span = new Span();
180+
span.setAttribute('foo', 'foo');
181+
span.setAttribute('bar', 'bar');
182+
span.setData('foo', 'foo2');
183+
span.setData('baz', 'baz');
184+
185+
expect(span['_getData']()).toEqual({ foo: 'foo', bar: 'bar', baz: 'baz' });
186+
expect(span['_getData']()).not.toBe(span.attributes);
187+
expect(span['_getData']()).not.toBe(span.data);
188+
});
189+
});
32190
});

‎packages/node/src/integrations/undici/types.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Vendored from https://github.com/DefinitelyTyped/DefinitelyTyped/blob/5a94716c6788f654aea7999a5fc28f4f1e7c48ad/types/node/diagnostics_channel.d.ts
22

33
import type { URL } from 'url';
4-
import type { Span } from '@sentry/core';
4+
import type { Span } from '@sentry/types';
55

66
// License:
77
// This project is licensed under the MIT license.

‎packages/sveltekit/src/server/handle.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
/* eslint-disable @sentry-internal/sdk/no-optional-chaining */
2-
import type { Span } from '@sentry/core';
31
import { getCurrentScope } from '@sentry/core';
42
import { getActiveTransaction, runWithAsyncContext, startSpan } from '@sentry/core';
53
import { captureException } from '@sentry/node';
4+
/* eslint-disable @sentry-internal/sdk/no-optional-chaining */
5+
import type { Span } from '@sentry/types';
66
import { dynamicSamplingContextToSentryBaggageHeader, objectify } from '@sentry/utils';
77
import type { Handle, ResolveOptions } from '@sveltejs/kit';
88

‎packages/types/src/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ export type {
8989

9090
// eslint-disable-next-line deprecation/deprecation
9191
export type { Severity, SeverityLevel } from './severity';
92-
export type { Span, SpanContext, SpanOrigin } from './span';
92+
export type { Span, SpanContext, SpanOrigin, SpanAttributeValue, SpanAttributes } from './span';
9393
export type { StackFrame } from './stackframe';
9494
export type { Stacktrace, StackParser, StackLineParser, StackLineParserFn } from './stacktrace';
9595
export type { TextEncoderInternal } from './textencoder';

‎packages/types/src/span.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,17 @@ export type SpanOrigin =
1212
| `${SpanOriginType}.${SpanOriginCategory}.${SpanOriginIntegrationName}`
1313
| `${SpanOriginType}.${SpanOriginCategory}.${SpanOriginIntegrationName}.${SpanOriginIntegrationPart}`;
1414

15+
// These types are aligned with OpenTelemetry Span Attributes
16+
export type SpanAttributeValue =
17+
| string
18+
| number
19+
| boolean
20+
| Array<null | undefined | string>
21+
| Array<null | undefined | number>
22+
| Array<null | undefined | boolean>;
23+
24+
export type SpanAttributes = Record<string, SpanAttributeValue | undefined>;
25+
1526
/** Interface holding all properties that can be set on a Span on creation. */
1627
export interface SpanContext {
1728
/**
@@ -65,6 +76,11 @@ export interface SpanContext {
6576
*/
6677
data?: { [key: string]: any };
6778

79+
/**
80+
* Attributes of the Span.
81+
*/
82+
attributes?: SpanAttributes;
83+
6884
/**
6985
* Timestamp in seconds (epoch time) indicating when the span started.
7086
*/
@@ -118,6 +134,11 @@ export interface Span extends SpanContext {
118134
*/
119135
data: { [key: string]: any };
120136

137+
/**
138+
* @inheritDoc
139+
*/
140+
attributes: SpanAttributes;
141+
121142
/**
122143
* The transaction containing this span
123144
*/
@@ -156,6 +177,18 @@ export interface Span extends SpanContext {
156177
*/
157178
setData(key: string, value: any): this;
158179

180+
/**
181+
* Set a single attribute on the span.
182+
* Set it to `undefined` to remove the attribute.
183+
*/
184+
setAttribute(key: string, value: SpanAttributeValue | undefined): void;
185+
186+
/**
187+
* Set multiple attributes on the span.
188+
* Any attribute set to `undefined` will be removed.
189+
*/
190+
setAttributes(attributes: SpanAttributes): void;
191+
159192
/**
160193
* Sets the status attribute on the current span
161194
* See: {@sentry/tracing SpanStatus} for possible values

0 commit comments

Comments
 (0)