Skip to content

Commit 9da8e7f

Browse files
mydeamydea
authored
test(opentelemetry): Remove deprecated new Span() usage in tests (#15702)
Fixes part of #15518 We have been using the deprecated `new Span()` syntax for tests in OTEL. This PR changes this to instead use a proper tracer, which is also supported in v2 of otel.
1 parent 2b55265 commit 9da8e7f

File tree

6 files changed

+170
-110
lines changed

6 files changed

+170
-110
lines changed

‎packages/node/src/integrations/tracing/graphql.ts

-3
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,6 @@ export const instrumentGraphql = generateInstrumentOnce<GraphqlOptions>(
5555

5656
if (options.useOperationNameForRootSpan && operationType) {
5757
const rootSpan = getRootSpan(span);
58-
59-
// We guard to only do this on http.server spans
60-
6158
const rootSpanAttributes = spanToJSON(rootSpan).data;
6259

6360
const existingOperations = rootSpanAttributes[SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION] || [];

‎packages/opentelemetry/test/helpers/createSpan.ts

-40
This file was deleted.

‎packages/opentelemetry/test/utils/getRequestSpanData.test.ts

+26-7
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,39 @@
11
/* eslint-disable deprecation/deprecation */
2+
import type { Span } from '@opentelemetry/api';
3+
import { trace } from '@opentelemetry/api';
4+
import type { BasicTracerProvider } from '@opentelemetry/sdk-trace-base';
25
import { SEMATTRS_HTTP_METHOD, SEMATTRS_HTTP_URL } from '@opentelemetry/semantic-conventions';
3-
import { describe, expect, it } from 'vitest';
4-
6+
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
57
import { getRequestSpanData } from '../../src/utils/getRequestSpanData';
6-
import { createSpan } from '../helpers/createSpan';
8+
import { TestClient, getDefaultTestClientOptions } from '../helpers/TestClient';
9+
import { setupOtel } from '../helpers/initOtel';
10+
import { cleanupOtel } from '../helpers/mockSdkInit';
711

812
describe('getRequestSpanData', () => {
13+
let provider: BasicTracerProvider | undefined;
14+
15+
beforeEach(() => {
16+
const client = new TestClient(getDefaultTestClientOptions({ tracesSampleRate: 1 }));
17+
provider = setupOtel(client);
18+
});
19+
20+
afterEach(() => {
21+
cleanupOtel(provider);
22+
});
23+
24+
function createSpan(name: string): Span {
25+
return trace.getTracer('test').startSpan(name);
26+
}
27+
928
it('works with basic span', () => {
10-
const span = createSpan();
29+
const span = createSpan('test-span');
1130
const data = getRequestSpanData(span);
1231

1332
expect(data).toEqual({});
1433
});
1534

1635
it('works with http span', () => {
17-
const span = createSpan();
36+
const span = createSpan('test-span');
1837
span.setAttributes({
1938
[SEMATTRS_HTTP_URL]: 'http://example.com?foo=bar#baz',
2039
[SEMATTRS_HTTP_METHOD]: 'GET',
@@ -31,7 +50,7 @@ describe('getRequestSpanData', () => {
3150
});
3251

3352
it('works without method', () => {
34-
const span = createSpan();
53+
const span = createSpan('test-span');
3554
span.setAttributes({
3655
[SEMATTRS_HTTP_URL]: 'http://example.com',
3756
});
@@ -45,7 +64,7 @@ describe('getRequestSpanData', () => {
4564
});
4665

4766
it('works with incorrect URL', () => {
48-
const span = createSpan();
67+
const span = createSpan('test-span');
4968
span.setAttributes({
5069
[SEMATTRS_HTTP_URL]: 'malformed-url-here',
5170
[SEMATTRS_HTTP_METHOD]: 'GET',

‎packages/opentelemetry/test/utils/groupSpansWithParents.test.ts

+65-16
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,45 @@
1-
import { describe, expect, it } from 'vitest';
2-
1+
import { trace } from '@opentelemetry/api';
2+
import type { BasicTracerProvider, ReadableSpan } from '@opentelemetry/sdk-trace-base';
3+
import type { Span } from '@sentry/core';
4+
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
5+
import { withActiveSpan } from '../../src/trace';
36
import { groupSpansWithParents } from '../../src/utils/groupSpansWithParents';
4-
import { createSpan } from '../helpers/createSpan';
7+
import { TestClient, getDefaultTestClientOptions } from '../helpers/TestClient';
8+
import { setupOtel } from '../helpers/initOtel';
9+
import { cleanupOtel } from '../helpers/mockSdkInit';
510

611
describe('groupSpansWithParents', () => {
12+
let provider: BasicTracerProvider | undefined;
13+
14+
beforeEach(() => {
15+
const client = new TestClient(getDefaultTestClientOptions({ tracesSampleRate: 1 }));
16+
provider = setupOtel(client);
17+
});
18+
19+
afterEach(() => {
20+
cleanupOtel(provider);
21+
});
22+
723
it('works with no spans', () => {
824
const actual = groupSpansWithParents([]);
925
expect(actual).toEqual([]);
1026
});
1127

1228
it('works with a single root span & in-order spans', () => {
13-
const rootSpan = createSpan('root', { spanId: 'rootId' });
14-
const parentSpan1 = createSpan('parent1', { spanId: 'parent1Id', parentSpanId: 'rootId' });
15-
const parentSpan2 = createSpan('parent2', { spanId: 'parent2Id', parentSpanId: 'rootId' });
16-
const child1 = createSpan('child1', { spanId: 'child1', parentSpanId: 'parent1Id' });
29+
const tracer = trace.getTracer('test');
30+
const rootSpan = tracer.startSpan('root') as unknown as ReadableSpan;
31+
const parentSpan1 = withActiveSpan(
32+
rootSpan as unknown as Span,
33+
() => tracer.startSpan('parent1') as unknown as ReadableSpan,
34+
);
35+
const parentSpan2 = withActiveSpan(
36+
rootSpan as unknown as Span,
37+
() => tracer.startSpan('parent2') as unknown as ReadableSpan,
38+
);
39+
const child1 = withActiveSpan(
40+
parentSpan1 as unknown as Span,
41+
() => tracer.startSpan('child1') as unknown as ReadableSpan,
42+
);
1743

1844
const actual = groupSpansWithParents([rootSpan, parentSpan1, parentSpan2, child1]);
1945
expect(actual).toHaveLength(4);
@@ -46,15 +72,28 @@ describe('groupSpansWithParents', () => {
4672
});
4773

4874
it('works with a spans with missing root span', () => {
49-
const parentSpan1 = createSpan('parent1', { spanId: 'parent1Id', parentSpanId: 'rootId' });
50-
const parentSpan2 = createSpan('parent2', { spanId: 'parent2Id', parentSpanId: 'rootId' });
51-
const child1 = createSpan('child1', { spanId: 'child1', parentSpanId: 'parent1Id' });
75+
const tracer = trace.getTracer('test');
76+
77+
// We create this root span here, but we do not pass it to `groupSpansWithParents` below
78+
const rootSpan = tracer.startSpan('root') as unknown as ReadableSpan;
79+
const parentSpan1 = withActiveSpan(
80+
rootSpan as unknown as Span,
81+
() => tracer.startSpan('parent1') as unknown as ReadableSpan,
82+
);
83+
const parentSpan2 = withActiveSpan(
84+
rootSpan as unknown as Span,
85+
() => tracer.startSpan('parent2') as unknown as ReadableSpan,
86+
);
87+
const child1 = withActiveSpan(
88+
parentSpan1 as unknown as Span,
89+
() => tracer.startSpan('child1') as unknown as ReadableSpan,
90+
);
5291

5392
const actual = groupSpansWithParents([parentSpan1, parentSpan2, child1]);
5493
expect(actual).toHaveLength(4);
5594

5695
// Ensure parent & span is correctly set
57-
const rootRef = actual.find(ref => ref.id === 'rootId');
96+
const rootRef = actual.find(ref => ref.id === rootSpan.spanContext().spanId);
5897
const parent1Ref = actual.find(ref => ref.span === parentSpan1);
5998
const parent2Ref = actual.find(ref => ref.span === parentSpan2);
6099
const child1Ref = actual.find(ref => ref.span === child1);
@@ -82,11 +121,21 @@ describe('groupSpansWithParents', () => {
82121
});
83122

84123
it('works with multiple root spans & out-of-order spans', () => {
85-
const rootSpan1 = createSpan('root1', { spanId: 'root1Id' });
86-
const rootSpan2 = createSpan('root2', { spanId: 'root2Id' });
87-
const parentSpan1 = createSpan('parent1', { spanId: 'parent1Id', parentSpanId: 'root1Id' });
88-
const parentSpan2 = createSpan('parent2', { spanId: 'parent2Id', parentSpanId: 'root2Id' });
89-
const childSpan1 = createSpan('child1', { spanId: 'child1Id', parentSpanId: 'parent1Id' });
124+
const tracer = trace.getTracer('test');
125+
const rootSpan1 = tracer.startSpan('root1') as unknown as ReadableSpan;
126+
const rootSpan2 = tracer.startSpan('root2') as unknown as ReadableSpan;
127+
const parentSpan1 = withActiveSpan(
128+
rootSpan1 as unknown as Span,
129+
() => tracer.startSpan('parent1') as unknown as ReadableSpan,
130+
);
131+
const parentSpan2 = withActiveSpan(
132+
rootSpan2 as unknown as Span,
133+
() => tracer.startSpan('parent2') as unknown as ReadableSpan,
134+
);
135+
const childSpan1 = withActiveSpan(
136+
parentSpan1 as unknown as Span,
137+
() => tracer.startSpan('child1') as unknown as ReadableSpan,
138+
);
90139

91140
const actual = groupSpansWithParents([childSpan1, parentSpan1, parentSpan2, rootSpan2, rootSpan1]);
92141
expect(actual).toHaveLength(5);

‎packages/opentelemetry/test/utils/mapStatus.test.ts

+41-25
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,32 @@
11
/* eslint-disable deprecation/deprecation */
2+
import type { Span } from '@opentelemetry/api';
3+
import { trace } from '@opentelemetry/api';
4+
import type { BasicTracerProvider } from '@opentelemetry/sdk-trace-base';
25
import { SEMATTRS_HTTP_STATUS_CODE, SEMATTRS_RPC_GRPC_STATUS_CODE } from '@opentelemetry/semantic-conventions';
3-
import { SPAN_STATUS_ERROR, SPAN_STATUS_OK } from '@sentry/core';
46
import type { SpanStatus } from '@sentry/core';
5-
import { describe, expect, it } from 'vitest';
6-
7+
import { SPAN_STATUS_ERROR, SPAN_STATUS_OK } from '@sentry/core';
8+
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
79
import { mapStatus } from '../../src/utils/mapStatus';
8-
import { createSpan } from '../helpers/createSpan';
10+
import { TestClient, getDefaultTestClientOptions } from '../helpers/TestClient';
11+
import { setupOtel } from '../helpers/initOtel';
12+
import { cleanupOtel } from '../helpers/mockSdkInit';
913

1014
describe('mapStatus', () => {
15+
let provider: BasicTracerProvider | undefined;
16+
17+
beforeEach(() => {
18+
const client = new TestClient(getDefaultTestClientOptions({ tracesSampleRate: 1 }));
19+
provider = setupOtel(client);
20+
});
21+
22+
afterEach(() => {
23+
cleanupOtel(provider);
24+
});
25+
26+
function createSpan(name: string): Span {
27+
return trace.getTracer('test').startSpan(name);
28+
}
29+
1130
const statusTestTable: [undefined | number | string, undefined | string, SpanStatus][] = [
1231
// http codes
1332
[400, undefined, { code: SPAN_STATUS_ERROR, message: 'invalid_argument' }],
@@ -23,19 +42,6 @@ describe('mapStatus', () => {
2342
[504, undefined, { code: SPAN_STATUS_ERROR, message: 'deadline_exceeded' }],
2443
[999, undefined, { code: SPAN_STATUS_ERROR, message: 'unknown_error' }],
2544

26-
['400', undefined, { code: SPAN_STATUS_ERROR, message: 'invalid_argument' }],
27-
['401', undefined, { code: SPAN_STATUS_ERROR, message: 'unauthenticated' }],
28-
['403', undefined, { code: SPAN_STATUS_ERROR, message: 'permission_denied' }],
29-
['404', undefined, { code: SPAN_STATUS_ERROR, message: 'not_found' }],
30-
['409', undefined, { code: SPAN_STATUS_ERROR, message: 'already_exists' }],
31-
['429', undefined, { code: SPAN_STATUS_ERROR, message: 'resource_exhausted' }],
32-
['499', undefined, { code: SPAN_STATUS_ERROR, message: 'cancelled' }],
33-
['500', undefined, { code: SPAN_STATUS_ERROR, message: 'internal_error' }],
34-
['501', undefined, { code: SPAN_STATUS_ERROR, message: 'unimplemented' }],
35-
['503', undefined, { code: SPAN_STATUS_ERROR, message: 'unavailable' }],
36-
['504', undefined, { code: SPAN_STATUS_ERROR, message: 'deadline_exceeded' }],
37-
['999', undefined, { code: SPAN_STATUS_ERROR, message: 'unknown_error' }],
38-
3945
// grpc codes
4046
[undefined, '1', { code: SPAN_STATUS_ERROR, message: 'cancelled' }],
4147
[undefined, '2', { code: SPAN_STATUS_ERROR, message: 'unknown_error' }],
@@ -56,11 +62,11 @@ describe('mapStatus', () => {
5662
[undefined, '999', { code: SPAN_STATUS_ERROR, message: 'unknown_error' }],
5763

5864
// http takes precedence over grpc
59-
['400', '2', { code: SPAN_STATUS_ERROR, message: 'invalid_argument' }],
65+
[400, '2', { code: SPAN_STATUS_ERROR, message: 'invalid_argument' }],
6066
];
6167

6268
it.each(statusTestTable)('works with httpCode=%s, grpcCode=%s', (httpCode, grpcCode, expected) => {
63-
const span = createSpan();
69+
const span = createSpan('test-span');
6470
span.setStatus({ code: 0 }); // UNSET
6571

6672
if (httpCode) {
@@ -75,39 +81,49 @@ describe('mapStatus', () => {
7581
expect(actual).toEqual(expected);
7682
});
7783

84+
it('works with string SEMATTRS_HTTP_STATUS_CODE xxx', () => {
85+
const span = createSpan('test-span');
86+
87+
span.setStatus({ code: 0 }); // UNSET
88+
span.setAttribute(SEMATTRS_HTTP_STATUS_CODE, '400');
89+
90+
const actual = mapStatus(span);
91+
expect(actual).toEqual({ code: SPAN_STATUS_ERROR, message: 'invalid_argument' });
92+
});
93+
7894
it('returns ok span status when is UNSET present on span', () => {
79-
const span = createSpan();
95+
const span = createSpan('test-span');
8096
span.setStatus({ code: 0 }); // UNSET
8197
expect(mapStatus(span)).toEqual({ code: SPAN_STATUS_OK });
8298
});
8399

84100
it('returns ok span status when already present on span', () => {
85-
const span = createSpan();
101+
const span = createSpan('test-span');
86102
span.setStatus({ code: 1 }); // OK
87103
expect(mapStatus(span)).toEqual({ code: SPAN_STATUS_OK });
88104
});
89105

90106
it('returns error status when span already has error status', () => {
91-
const span = createSpan();
107+
const span = createSpan('test-span');
92108
span.setStatus({ code: 2, message: 'invalid_argument' }); // ERROR
93109
expect(mapStatus(span)).toEqual({ code: SPAN_STATUS_ERROR, message: 'invalid_argument' });
94110
});
95111

96112
it('returns error status when span already has error status without message', () => {
97-
const span = createSpan();
113+
const span = createSpan('test-span');
98114
span.setStatus({ code: 2 }); // ERROR
99115
expect(mapStatus(span)).toEqual({ code: SPAN_STATUS_ERROR, message: 'unknown_error' });
100116
});
101117

102118
it('infers error status form attributes when span already has error status without message', () => {
103-
const span = createSpan();
119+
const span = createSpan('test-span');
104120
span.setAttribute(SEMATTRS_HTTP_STATUS_CODE, 500);
105121
span.setStatus({ code: 2 }); // ERROR
106122
expect(mapStatus(span)).toEqual({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
107123
});
108124

109125
it('returns unknown error status when code is unknown', () => {
110-
const span = createSpan();
126+
const span = createSpan('test-span');
111127
span.setStatus({ code: -1 as 0 });
112128
expect(mapStatus(span)).toEqual({ code: SPAN_STATUS_ERROR, message: 'unknown_error' });
113129
});

0 commit comments

Comments
 (0)