Skip to content

Commit 97ae3fc

Browse files
mydeamydea
committed
fix fetch
1 parent 0d1d26a commit 97ae3fc

File tree

18 files changed

+370
-39
lines changed

18 files changed

+370
-39
lines changed

‎dev-packages/node-integration-tests/suites/tracing/requests/fetch-noSampleRate/scenario.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
2+
import * as Sentry from '@sentry/node';
3+
4+
Sentry.init({
5+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
6+
release: '1.0',
7+
tracePropagationTargets: [/\/v0/, 'v1'],
8+
integrations: [],
9+
transport: loggingTransport,
10+
});
11+
12+
async function run(): Promise<void> {
13+
// Since fetch is lazy loaded, we need to wait a bit until it's fully instrumented
14+
await new Promise(resolve => setTimeout(resolve, 100));
15+
await fetch(`${process.env.SERVER_URL}/api/v0`);
16+
await fetch(`${process.env.SERVER_URL}/api/v1`);
17+
await fetch(`${process.env.SERVER_URL}/api/v2`);
18+
await fetch(`${process.env.SERVER_URL}/api/v3`);
19+
20+
Sentry.captureException(new Error('foo'));
21+
}
22+
23+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
24+
run();

‎dev-packages/node-integration-tests/suites/tracing/requests/fetch-noSampleRate/test.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import { conditionalTest } from '../../../../utils';
2+
import { createRunner } from '../../../../utils/runner';
3+
import { createTestServer } from '../../../../utils/server';
4+
5+
conditionalTest({ min: 18 })('outgoing fetch', () => {
6+
test('outgoing fetch requests are correctly instrumented without tracesSampleRate', done => {
7+
expect.assertions(15);
8+
9+
createTestServer(done)
10+
.get('/api/v0', headers => {
11+
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/));
12+
expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000');
13+
expect(headers['baggage']).toEqual(expect.any(String));
14+
expect(headers['__requestUrl']).toBeUndefined();
15+
})
16+
.get('/api/v1', headers => {
17+
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/));
18+
expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000');
19+
expect(headers['baggage']).toEqual(expect.any(String));
20+
expect(headers['__requestUrl']).toBeUndefined();
21+
})
22+
.get('/api/v2', headers => {
23+
expect(headers['baggage']).toBeUndefined();
24+
expect(headers['sentry-trace']).toBeUndefined();
25+
expect(headers['__requestUrl']).toBeUndefined();
26+
})
27+
.get('/api/v3', headers => {
28+
expect(headers['baggage']).toBeUndefined();
29+
expect(headers['sentry-trace']).toBeUndefined();
30+
expect(headers['__requestUrl']).toBeUndefined();
31+
})
32+
.start()
33+
.then(SERVER_URL => {
34+
createRunner(__dirname, 'scenario.ts')
35+
.withEnv({ SERVER_URL })
36+
.ensureNoErrorOutput()
37+
.ignore('session', 'sessions')
38+
.expect({
39+
event: {
40+
exception: {
41+
values: [
42+
{
43+
type: 'Error',
44+
value: 'foo',
45+
},
46+
],
47+
},
48+
},
49+
})
50+
.start(done);
51+
});
52+
});
53+
});

‎dev-packages/node-integration-tests/suites/tracing/requests/fetch-sampled/scenario.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
2+
import * as Sentry from '@sentry/node';
3+
4+
Sentry.init({
5+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
6+
release: '1.0',
7+
tracesSampleRate: 1.0,
8+
tracePropagationTargets: [/\/v0/, 'v1'],
9+
integrations: [],
10+
transport: loggingTransport,
11+
});
12+
13+
async function run(): Promise<void> {
14+
await Sentry.startSpan({ name: 'test_span' }, async () => {
15+
// Since fetch is lazy loaded, we need to wait a bit until it's fully instrumented
16+
await new Promise(resolve => setTimeout(resolve, 100));
17+
await fetch(`${process.env.SERVER_URL}/api/v0`);
18+
await fetch(`${process.env.SERVER_URL}/api/v1`);
19+
await fetch(`${process.env.SERVER_URL}/api/v2`);
20+
await fetch(`${process.env.SERVER_URL}/api/v3`);
21+
});
22+
}
23+
24+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
25+
run();

‎dev-packages/node-integration-tests/suites/tracing/requests/fetch-sampled/test.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import { conditionalTest } from '../../../../utils';
2+
import { createRunner } from '../../../../utils/runner';
3+
import { createTestServer } from '../../../../utils/server';
4+
5+
conditionalTest({ min: 18 })('outgoing fetch', () => {
6+
test('outgoing sampled fetch requests are correctly instrumented xxx', done => {
7+
expect.assertions(11);
8+
9+
createTestServer(done)
10+
.get('/api/v0', headers => {
11+
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/));
12+
expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000-1');
13+
expect(headers['baggage']).toEqual(expect.any(String));
14+
})
15+
.get('/api/v1', headers => {
16+
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/));
17+
expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000-1');
18+
expect(headers['baggage']).toEqual(expect.any(String));
19+
})
20+
.get('/api/v2', headers => {
21+
expect(headers['baggage']).toBeUndefined();
22+
expect(headers['sentry-trace']).toBeUndefined();
23+
})
24+
.get('/api/v3', headers => {
25+
expect(headers['baggage']).toBeUndefined();
26+
expect(headers['sentry-trace']).toBeUndefined();
27+
})
28+
.start()
29+
.then(SERVER_URL => {
30+
createRunner(__dirname, 'scenario.ts')
31+
.withEnv({ SERVER_URL })
32+
.expect({
33+
transaction: {
34+
// we're not too concerned with the actual transaction here since this is tested elsewhere
35+
},
36+
})
37+
.start(done);
38+
});
39+
});
40+
});

‎dev-packages/node-integration-tests/suites/tracing/requests/fetch-unsampled/scenario.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
2+
import * as Sentry from '@sentry/node';
3+
4+
Sentry.init({
5+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
6+
release: '1.0',
7+
tracePropagationTargets: [/\/v0/, 'v1'],
8+
tracesSampleRate: 0,
9+
integrations: [],
10+
transport: loggingTransport,
11+
});
12+
13+
async function run(): Promise<void> {
14+
// Wrap in span that is not sampled
15+
await Sentry.startSpan({ name: 'outer' }, async () => {
16+
// Since fetch is lazy loaded, we need to wait a bit until it's fully instrumented
17+
await new Promise(resolve => setTimeout(resolve, 100));
18+
await fetch(`${process.env.SERVER_URL}/api/v0`);
19+
await fetch(`${process.env.SERVER_URL}/api/v1`);
20+
await fetch(`${process.env.SERVER_URL}/api/v2`);
21+
await fetch(`${process.env.SERVER_URL}/api/v3`);
22+
});
23+
24+
Sentry.captureException(new Error('foo'));
25+
}
26+
27+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
28+
run();

‎dev-packages/node-integration-tests/suites/tracing/requests/fetch-unsampled/test.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import { conditionalTest } from '../../../../utils';
2+
import { createRunner } from '../../../../utils/runner';
3+
import { createTestServer } from '../../../../utils/server';
4+
5+
conditionalTest({ min: 18 })('outgoing fetch', () => {
6+
test('outgoing fetch requests are correctly instrumented when not sampled', done => {
7+
expect.assertions(11);
8+
9+
createTestServer(done)
10+
.get('/api/v0', headers => {
11+
expect(headers['baggage']).toEqual(expect.any(String));
12+
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-0$/));
13+
expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000-0');
14+
})
15+
.get('/api/v1', headers => {
16+
expect(headers['baggage']).toEqual(expect.any(String));
17+
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-0$/));
18+
expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000-0');
19+
})
20+
.get('/api/v2', headers => {
21+
expect(headers['baggage']).toBeUndefined();
22+
expect(headers['sentry-trace']).toBeUndefined();
23+
})
24+
.get('/api/v3', headers => {
25+
expect(headers['baggage']).toBeUndefined();
26+
expect(headers['sentry-trace']).toBeUndefined();
27+
})
28+
.start()
29+
.then(SERVER_URL => {
30+
createRunner(__dirname, 'scenario.ts')
31+
.withEnv({ SERVER_URL })
32+
.ignore('session', 'sessions')
33+
.expect({
34+
event: {
35+
exception: {
36+
values: [
37+
{
38+
type: 'Error',
39+
value: 'foo',
40+
},
41+
],
42+
},
43+
},
44+
})
45+
.start(done);
46+
});
47+
});
48+
});

‎dev-packages/node-integration-tests/suites/tracing/tracingNoSampleRate/scenario.ts renamed to ‎dev-packages/node-integration-tests/suites/tracing/requests/http-noSampleRate/scenario.ts

File renamed without changes.

‎dev-packages/node-integration-tests/suites/tracing/tracingNoSampleRate/test.ts renamed to ‎dev-packages/node-integration-tests/suites/tracing/requests/http-noSampleRate/test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
import { createRunner } from '../../../utils/runner';
2-
import { createTestServer } from '../../../utils/server';
1+
import { createRunner } from '../../../../utils/runner';
2+
import { createTestServer } from '../../../../utils/server';
33

4-
test('HttpIntegration should instrument correct requests even without tracesSampleRate', done => {
4+
test('outgoing http requests are correctly instrumented without tracesSampleRate', done => {
55
expect.assertions(15);
66

7-
createTestServer(() => {})
7+
createTestServer(done)
88
.get('/api/v0', headers => {
99
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})$/));
1010
expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000');

‎dev-packages/node-integration-tests/suites/tracing/requests/http-sampled/scenario.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
2+
import * as Sentry from '@sentry/node';
3+
4+
Sentry.init({
5+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
6+
release: '1.0',
7+
tracesSampleRate: 1.0,
8+
tracePropagationTargets: [/\/v0/, 'v1'],
9+
integrations: [],
10+
transport: loggingTransport,
11+
});
12+
13+
import * as http from 'http';
14+
15+
Sentry.startSpan({ name: 'test_span' }, () => {
16+
http.get(`${process.env.SERVER_URL}/api/v0`);
17+
http.get(`${process.env.SERVER_URL}/api/v1`);
18+
http.get(`${process.env.SERVER_URL}/api/v2`);
19+
http.get(`${process.env.SERVER_URL}/api/v3`);
20+
});

‎dev-packages/node-integration-tests/suites/tracing/requests/http-sampled/test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import { createRunner } from '../../../../utils/runner';
2+
import { createTestServer } from '../../../../utils/server';
3+
4+
test('outgoing sampled http requests are correctly instrumented', done => {
5+
expect.assertions(11);
6+
7+
createTestServer(done)
8+
.get('/api/v0', headers => {
9+
expect(headers['baggage']).toEqual(expect.any(String));
10+
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/));
11+
expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000-1');
12+
})
13+
.get('/api/v1', headers => {
14+
expect(headers['baggage']).toEqual(expect.any(String));
15+
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^([a-f0-9]{32})-([a-f0-9]{16})-1$/));
16+
expect(headers['sentry-trace']).not.toEqual('00000000000000000000000000000000-0000000000000000-1');
17+
})
18+
.get('/api/v2', headers => {
19+
expect(headers['baggage']).toBeUndefined();
20+
expect(headers['sentry-trace']).toBeUndefined();
21+
})
22+
.get('/api/v3', headers => {
23+
expect(headers['baggage']).toBeUndefined();
24+
expect(headers['sentry-trace']).toBeUndefined();
25+
})
26+
.start()
27+
.then(SERVER_URL => {
28+
createRunner(__dirname, 'scenario.ts')
29+
.withEnv({ SERVER_URL })
30+
.expect({
31+
transaction: {
32+
// we're not too concerned with the actual transaction here since this is tested elsewhere
33+
},
34+
})
35+
.start(done);
36+
});
37+
});

‎dev-packages/node-integration-tests/suites/tracing/tracingUnsampled/scenario.ts renamed to ‎dev-packages/node-integration-tests/suites/tracing/requests/http-unsampled/scenario.ts

File renamed without changes.

‎dev-packages/node-integration-tests/suites/tracing/tracingUnsampled/test.ts renamed to ‎dev-packages/node-integration-tests/suites/tracing/requests/http-unsampled/test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
import { createRunner } from '../../../utils/runner';
2-
import { createTestServer } from '../../../utils/server';
1+
import { createRunner } from '../../../../utils/runner';
2+
import { createTestServer } from '../../../../utils/server';
33

4-
test('HttpIntegration should instrument correct requests even when not sampled', done => {
4+
test('outgoing http requests are correctly instrumented when not sampled', done => {
55
expect.assertions(11);
66

77
createTestServer(done)

‎packages/node/src/integrations/http.ts

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { Span } from '@opentelemetry/api';
33
import { SpanKind } from '@opentelemetry/api';
44
import { registerInstrumentations } from '@opentelemetry/instrumentation';
55
import { HttpInstrumentation } from '@opentelemetry/instrumentation-http';
6+
import { storeRequestUrlOnPropagationCarrier } from '@sentry/opentelemetry';
67

78
import {
89
addBreadcrumb,
@@ -17,7 +18,7 @@ import {
1718
import { getClient, getRequestSpanData, getSpanKind } from '@sentry/opentelemetry';
1819
import type { IntegrationFn } from '@sentry/types';
1920

20-
import { addNonEnumerableProperty, stripUrlQueryAndFragment } from '@sentry/utils';
21+
import { stripUrlQueryAndFragment } from '@sentry/utils';
2122
import type { NodeClient } from '../sdk/client';
2223
import { setIsolationScope } from '../sdk/scope';
2324
import type { HTTPModuleRequestIncomingMessage } from '../transports/http-module';
@@ -65,13 +66,10 @@ const _httpIntegration = ((options: HttpOptions = {}) => {
6566
return true;
6667
}
6768

68-
// SUPER HACK:
69-
// We store the URL on the headers object, because this is passed to the propagator
70-
// Where we can pick the URL off to deterime if we should attach trace headers.
69+
// We keep the URL on the headers (which are the carrier for the propagator)
70+
// so we can access it in the propagator to check against tracePropagationTargets
7171
const headers = request.headers || {};
72-
// Can't use a non-enumerable property because http instrumentation clones this
73-
// We remove this in the propagator
74-
headers['__requestUrl'] = url;
72+
storeRequestUrlOnPropagationCarrier(headers, url);
7573
request.headers = headers;
7674

7775
if (_ignoreOutgoingRequests && _ignoreOutgoingRequests(url)) {

‎packages/opentelemetry/src/constants.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ export const SENTRY_BAGGAGE_HEADER = 'baggage';
66
export const SENTRY_TRACE_STATE_DSC = 'sentry.dsc';
77
export const SENTRY_TRACE_STATE_PARENT_SPAN_ID = 'sentry.parent_span_id';
88
export const SENTRY_TRACE_STATE_SAMPLED_NOT_RECORDING = 'sentry.sampled_not_recording';
9+
export const SENTRY_TRACE_STATE_URL = 'sentry.url';
910

1011
export const SENTRY_SCOPES_CONTEXT_KEY = createContextKey('sentry_scopes');
1112

‎packages/opentelemetry/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ export {
1717
} from './utils/spanTypes';
1818

1919
export { getDynamicSamplingContextFromSpan } from './utils/dynamicSamplingContext';
20+
export { storeRequestUrlOnPropagationCarrier } from './utils/storeRequestUrlForPropagation';
2021

2122
export { isSentryRequestSpan } from './utils/isSentryRequest';
2223

0 commit comments

Comments
 (0)