Skip to content

Commit 75b0305

Browse files
mydeamydea
committed
fix(core): Ensure tunnel is considered for isSentryUrl checks
Also streamline these and use the same util in all places.
1 parent a7cc51e commit 75b0305

File tree

13 files changed

+134
-79
lines changed

13 files changed

+134
-79
lines changed

‎packages/core/src/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ export { FunctionToString, InboundFilters } from './integrations';
5151
export { prepareEvent } from './utils/prepareEvent';
5252
export { createCheckInEnvelope } from './checkin';
5353
export { hasTracingEnabled } from './utils/hasTracingEnabled';
54+
export { isSentryRequestUrl } from './utils/isSentryRequestUrl';
5455
export { DEFAULT_ENVIRONMENT } from './constants';
5556
export { ModuleMetadata } from './integrations/metadata';
5657
import * as Integrations from './integrations';

‎packages/core/src/utils/isSentryRequestUrl.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import type { DsnComponents, Hub } from '@sentry/types';
2+
3+
/**
4+
* Checks whether given url points to Sentry server
5+
* @param url url to verify
6+
*/
7+
export function isSentryRequestUrl(url: string, hub: Hub): boolean {
8+
const client = hub.getClient();
9+
const dsn = client && client.getDsn();
10+
const tunnel = client && client.getOptions().tunnel;
11+
12+
return checkDsn(url, dsn) || checkTunnel(url, tunnel);
13+
}
14+
15+
function checkTunnel(url: string, tunnel: string | undefined): boolean {
16+
if (!tunnel) {
17+
return false;
18+
}
19+
20+
return removeTrailingSlash(url) === removeTrailingSlash(tunnel);
21+
}
22+
23+
function checkDsn(url: string, dsn: DsnComponents | undefined): boolean {
24+
return dsn ? url.includes(dsn.host) : false;
25+
}
26+
27+
function removeTrailingSlash(str: string): string {
28+
return str[str.length - 1] === '/' ? str.slice(0, -1) : str;
29+
}

‎packages/core/test/lib/utils/isSentryRequestUrl.test.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import type { Hub } from '@sentry/types';
2+
3+
import { isSentryRequestUrl } from '../../../src';
4+
5+
describe('isSentryRequestUrl', () => {
6+
it.each([
7+
['', 'sentry-dsn.com', '', false],
8+
['http://sentry-dsn.com/my-url', 'sentry-dsn.com', '', true],
9+
['http://sentry-dsn.com', 'sentry-dsn.com', '', true],
10+
['http://tunnel:4200', 'sentry-dsn.com', 'http://tunnel:4200', true],
11+
['http://tunnel:4200', 'sentry-dsn.com', 'http://tunnel:4200/', true],
12+
['http://tunnel:4200/', 'sentry-dsn.com', 'http://tunnel:4200', true],
13+
['http://tunnel:4200/a', 'sentry-dsn.com', 'http://tunnel:4200', false],
14+
])('works with url=%s, dsn=%s, tunnel=%s', (url: string, dsn: string, tunnel: string, expected: boolean) => {
15+
const hub = {
16+
getClient: () => {
17+
return {
18+
getOptions: () => ({ tunnel }),
19+
getDsn: () => ({ host: dsn }),
20+
};
21+
},
22+
} as unknown as Hub;
23+
24+
expect(isSentryRequestUrl(url, hub)).toBe(expected);
25+
});
26+
});

‎packages/integrations/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
}
2424
},
2525
"dependencies": {
26+
"@sentry/core": "7.72.0",
2627
"@sentry/types": "7.72.0",
2728
"@sentry/utils": "7.72.0",
2829
"localforage": "^1.8.1",

‎packages/integrations/src/httpclient.ts‎

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { getCurrentHub, isSentryRequestUrl } from '@sentry/core';
12
import type {
23
Event as SentryEvent,
34
EventProcessor,
@@ -345,30 +346,18 @@ export class HttpClient implements Integration {
345346
);
346347
}
347348

348-
/**
349-
* Checks whether given url points to Sentry server
350-
*
351-
* @param url url to verify
352-
*/
353-
private _isSentryRequest(url: string): boolean {
354-
const client = this._getCurrentHub && this._getCurrentHub().getClient();
355-
356-
if (!client) {
357-
return false;
358-
}
359-
360-
const dsn = client.getDsn();
361-
return dsn ? url.includes(dsn.host) : false;
362-
}
363-
364349
/**
365350
* Checks whether to capture given response as an event
366351
*
367352
* @param status response status code
368353
* @param url response url
369354
*/
370355
private _shouldCaptureResponse(status: number, url: string): boolean {
371-
return this._isInGivenStatusRanges(status) && this._isInGivenRequestTargets(url) && !this._isSentryRequest(url);
356+
return (
357+
this._isInGivenStatusRanges(status) &&
358+
this._isInGivenRequestTargets(url) &&
359+
!isSentryRequestUrl(url, getCurrentHub())
360+
);
372361
}
373362

374363
/**

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

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,15 @@ import { SpanKind } from '@opentelemetry/api';
33
import { registerInstrumentations } from '@opentelemetry/instrumentation';
44
import { HttpInstrumentation } from '@opentelemetry/instrumentation-http';
55
import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
6-
import { hasTracingEnabled, Transaction } from '@sentry/core';
6+
import { hasTracingEnabled, isSentryRequestUrl, Transaction } from '@sentry/core';
77
import { getCurrentHub } from '@sentry/node';
88
import { _INTERNAL_getSentrySpan } from '@sentry/opentelemetry-node';
99
import type { EventProcessor, Hub, Integration } from '@sentry/types';
1010
import type { ClientRequest, IncomingMessage, ServerResponse } from 'http';
1111

1212
import type { NodeExperimentalClient, OtelSpan } from '../types';
1313
import { getRequestSpanData } from '../utils/getRequestSpanData';
14+
import { getRequestUrl } from '../utils/getRequestUrl';
1415

1516
interface TracingOptions {
1617
/**
@@ -93,8 +94,8 @@ export class Http implements Integration {
9394
instrumentations: [
9495
new HttpInstrumentation({
9596
ignoreOutgoingRequestHook: request => {
96-
const host = request.host || request.hostname;
97-
return isSentryHost(host);
97+
const url = getRequestUrl(request);
98+
return url ? isSentryRequestUrl(url, getCurrentHub()) : false;
9899
},
99100

100101
ignoreIncomingRequestHook: request => {
@@ -224,11 +225,3 @@ function getHttpUrl(attributes: Attributes): string | undefined {
224225
const url = attributes[SemanticAttributes.HTTP_URL];
225226
return typeof url === 'string' ? url : undefined;
226227
}
227-
228-
/**
229-
* Checks whether given host points to Sentry server
230-
*/
231-
function isSentryHost(host: string | undefined): boolean {
232-
const dsn = getCurrentHub().getClient()?.getDsn();
233-
return dsn && host ? host.includes(dsn.host) : false;
234-
}

‎packages/node-experimental/src/utils/getRequestUrl.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import type { RequestOptions } from 'http';
2+
3+
/** Build a full URL from request options. */
4+
export function getRequestUrl(requestOptions: RequestOptions): string {
5+
const protocol = requestOptions.protocol || '';
6+
const hostname = requestOptions.hostname || requestOptions.host || '';
7+
// Don't log standard :80 (http) and :443 (https) ports to reduce the noise
8+
// Also don't add port if the hostname already includes a port
9+
const port =
10+
!requestOptions.port || requestOptions.port === 80 || requestOptions.port === 443 || /^(.*):(\d+)$/.test(hostname)
11+
? ''
12+
: `:${requestOptions.port}`;
13+
const path = requestOptions.path ? requestOptions.path : '/';
14+
return `${protocol}//${hostname}${port}${path}`;
15+
}

‎packages/node-experimental/test/utils/getRequestUrl.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import type { RequestOptions } from 'http';
2+
3+
import { getRequestUrl } from '../../src/utils/getRequestUrl';
4+
5+
describe('getRequestUrl', () => {
6+
it.each([
7+
[{ protocol: 'http:', hostname: 'localhost', port: 80 }, 'http://localhost/'],
8+
[{ protocol: 'http:', hostname: 'localhost', host: 'localhost:80', port: 80 }, 'http://localhost/'],
9+
[{ protocol: 'http:', hostname: 'localhost', port: 3000 }, 'http://localhost:3000/'],
10+
[{ protocol: 'http:', host: 'localhost:3000', port: 3000 }, 'http://localhost:3000/'],
11+
[{ protocol: 'https:', hostname: 'localhost', port: 443 }, 'https://localhost/'],
12+
[{ protocol: 'https:', hostname: 'localhost', port: 443, path: '/my-path' }, 'https://localhost/my-path'],
13+
[
14+
{ protocol: 'https:', hostname: 'www.example.com', port: 443, path: '/my-path' },
15+
'https://www.example.com/my-path',
16+
],
17+
])('works with %s', (input: RequestOptions, expected: string | undefined) => {
18+
expect(getRequestUrl(input)).toBe(expected);
19+
});
20+
});

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { Hub } from '@sentry/core';
2-
import { getCurrentHub, getDynamicSamplingContextFromClient } from '@sentry/core';
2+
import { getCurrentHub, getDynamicSamplingContextFromClient, isSentryRequestUrl } from '@sentry/core';
33
import type {
44
DynamicSamplingContext,
55
EventProcessor,
@@ -21,7 +21,7 @@ import { LRUMap } from 'lru_map';
2121
import type { NodeClient } from '../client';
2222
import { NODE_VERSION } from '../nodeVersion';
2323
import type { RequestMethod, RequestMethodArgs, RequestOptions } from './utils/http';
24-
import { cleanSpanDescription, extractRawUrl, extractUrl, isSentryRequest, normalizeRequestArgs } from './utils/http';
24+
import { cleanSpanDescription, extractRawUrl, extractUrl, normalizeRequestArgs } from './utils/http';
2525

2626
interface TracingOptions {
2727
/**
@@ -238,7 +238,7 @@ function _createWrappedRequestMethodFactory(
238238
const requestUrl = extractUrl(requestOptions);
239239

240240
// we don't want to record requests to Sentry as either breadcrumbs or spans, so just use the original method
241-
if (isSentryRequest(requestUrl)) {
241+
if (isSentryRequestUrl(requestUrl, getCurrentHub())) {
242242
return originalRequestMethod.apply(httpModule, requestArgs);
243243
}
244244

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { getCurrentHub, getDynamicSamplingContextFromClient } from '@sentry/core';
1+
import { getCurrentHub, getDynamicSamplingContextFromClient, isSentryRequestUrl } from '@sentry/core';
22
import type { EventProcessor, Integration, Span } from '@sentry/types';
33
import {
44
dynamicRequire,
@@ -12,7 +12,6 @@ import { LRUMap } from 'lru_map';
1212

1313
import type { NodeClient } from '../../client';
1414
import { NODE_VERSION } from '../../nodeVersion';
15-
import { isSentryRequest } from '../utils/http';
1615
import type {
1716
DiagnosticsChannel,
1817
RequestCreateMessage,
@@ -138,7 +137,7 @@ export class Undici implements Integration {
138137

139138
const stringUrl = request.origin ? request.origin.toString() + request.path : request.path;
140139

141-
if (isSentryRequest(stringUrl) || request.__sentry_span__ !== undefined) {
140+
if (isSentryRequestUrl(stringUrl, hub) || request.__sentry_span__ !== undefined) {
142141
return;
143142
}
144143

@@ -198,7 +197,7 @@ export class Undici implements Integration {
198197

199198
const stringUrl = request.origin ? request.origin.toString() + request.path : request.path;
200199

201-
if (isSentryRequest(stringUrl)) {
200+
if (isSentryRequestUrl(stringUrl, hub)) {
202201
return;
203202
}
204203

@@ -238,7 +237,7 @@ export class Undici implements Integration {
238237

239238
const stringUrl = request.origin ? request.origin.toString() + request.path : request.path;
240239

241-
if (isSentryRequest(stringUrl)) {
240+
if (isSentryRequestUrl(stringUrl, hub)) {
242241
return;
243242
}
244243

0 commit comments

Comments
 (0)