Skip to content

Commit aad370b

Browse files
authored
Merge branch 'develop' into sig/hono-errorHandler
2 parents d5a46ad + b4bd21f commit aad370b

File tree

6 files changed

+57
-29
lines changed

6 files changed

+57
-29
lines changed

dev-packages/e2e-tests/test-applications/remix-hydrogen/package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
"@sentry/cloudflare": "latest || *",
2121
"@sentry/remix": "latest || *",
2222
"@sentry/vite-plugin": "^3.1.2",
23-
"@shopify/hydrogen": "^2025.1.0",
23+
"@shopify/hydrogen": "2025.4.0",
2424
"@shopify/remix-oxygen": "^2.0.10",
2525
"graphql": "^16.6.0",
2626
"graphql-tag": "^2.12.6",
@@ -34,7 +34,7 @@
3434
"@remix-run/dev": "^2.15.2",
3535
"@remix-run/eslint-config": "^2.15.2",
3636
"@sentry-internal/test-utils": "link:../../../test-utils",
37-
"@shopify/cli": "^3.74.1",
37+
"@shopify/cli": "3.74.1",
3838
"@shopify/hydrogen-codegen": "^0.3.1",
3939
"@shopify/mini-oxygen": "3.2.0",
4040
"@shopify/oxygen-workers-types": "^4.1.2",

packages/browser/src/client.ts

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -85,41 +85,41 @@ export class BrowserClient extends Client<BrowserClientOptions> {
8585

8686
super(opts);
8787

88-
// eslint-disable-next-line @typescript-eslint/no-this-alias
89-
const client = this;
90-
const { sendDefaultPii, _experiments } = client._options;
88+
const { sendDefaultPii, sendClientReports, _experiments } = this._options;
9189
const enableLogs = _experiments?.enableLogs;
9290

93-
if (opts.sendClientReports && WINDOW.document) {
91+
if (WINDOW.document && (sendClientReports || enableLogs)) {
9492
WINDOW.document.addEventListener('visibilitychange', () => {
9593
if (WINDOW.document.visibilityState === 'hidden') {
96-
this._flushOutcomes();
94+
if (sendClientReports) {
95+
this._flushOutcomes();
96+
}
9797
if (enableLogs) {
98-
_INTERNAL_flushLogsBuffer(client);
98+
_INTERNAL_flushLogsBuffer(this);
9999
}
100100
}
101101
});
102102
}
103103

104104
if (enableLogs) {
105-
client.on('flush', () => {
106-
_INTERNAL_flushLogsBuffer(client);
105+
this.on('flush', () => {
106+
_INTERNAL_flushLogsBuffer(this);
107107
});
108108

109-
client.on('afterCaptureLog', () => {
110-
if (client._logFlushIdleTimeout) {
111-
clearTimeout(client._logFlushIdleTimeout);
109+
this.on('afterCaptureLog', () => {
110+
if (this._logFlushIdleTimeout) {
111+
clearTimeout(this._logFlushIdleTimeout);
112112
}
113113

114-
client._logFlushIdleTimeout = setTimeout(() => {
115-
_INTERNAL_flushLogsBuffer(client);
114+
this._logFlushIdleTimeout = setTimeout(() => {
115+
_INTERNAL_flushLogsBuffer(this);
116116
}, DEFAULT_FLUSH_INTERVAL);
117117
});
118118
}
119119

120120
if (sendDefaultPii) {
121-
client.on('postprocessEvent', addAutoIpAddressToUser);
122-
client.on('beforeSendSession', addAutoIpAddressToSession);
121+
this.on('postprocessEvent', addAutoIpAddressToUser);
122+
this.on('beforeSendSession', addAutoIpAddressToSession);
123123
}
124124
}
125125

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,8 @@ export type SentryHttpInstrumentationOptions = InstrumentationConfig & {
7777
* Do not capture the request body for incoming HTTP requests to URLs where the given callback returns `true`.
7878
* This can be useful for long running requests where the body is not needed and we want to avoid capturing it.
7979
*
80-
* @param url Contains the entire URL, including query string (if any), protocol, host, etc. of the outgoing request.
81-
* @param request Contains the {@type RequestOptions} object used to make the outgoing request.
80+
* @param url Contains the entire URL, including query string (if any), protocol, host, etc. of the incoming request.
81+
* @param request Contains the {@type RequestOptions} object used to make the incoming request.
8282
*/
8383
ignoreIncomingRequestBody?: (url: string, request: http.RequestOptions) => boolean;
8484

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

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ import { diag } from '@opentelemetry/api';
33
import type { HttpInstrumentationConfig } from '@opentelemetry/instrumentation-http';
44
import { HttpInstrumentation } from '@opentelemetry/instrumentation-http';
55
import type { Span } from '@sentry/core';
6-
import { defineIntegration, getClient } from '@sentry/core';
6+
import { defineIntegration, getClient, hasSpansEnabled } from '@sentry/core';
7+
import { NODE_VERSION } from '../../nodeVersion';
78
import { generateInstrumentOnce } from '../../otel/instrument';
89
import type { NodeClient } from '../../sdk/client';
910
import type { HTTPModuleRequestIncomingMessage } from '../../transports/http-module';
@@ -85,8 +86,8 @@ interface HttpOptions {
8586
* Do not capture the request body for incoming HTTP requests to URLs where the given callback returns `true`.
8687
* This can be useful for long running requests where the body is not needed and we want to avoid capturing it.
8788
*
88-
* @param url Contains the entire URL, including query string (if any), protocol, host, etc. of the outgoing request.
89-
* @param request Contains the {@type RequestOptions} object used to make the outgoing request.
89+
* @param url Contains the entire URL, including query string (if any), protocol, host, etc. of the incoming request.
90+
* @param request Contains the {@type RequestOptions} object used to make the incoming request.
9091
*/
9192
ignoreIncomingRequestBody?: (url: string, request: RequestOptions) => boolean;
9293

@@ -159,8 +160,22 @@ export const instrumentOtelHttp = generateInstrumentOnce<HttpInstrumentationConf
159160
/** Exported only for tests. */
160161
export function _shouldInstrumentSpans(options: HttpOptions, clientOptions: Partial<NodeClientOptions> = {}): boolean {
161162
// If `spans` is passed in, it takes precedence
162-
// Else, we by default emit spans, unless `skipOpenTelemetrySetup` is set to `true`
163-
return typeof options.spans === 'boolean' ? options.spans : !clientOptions.skipOpenTelemetrySetup;
163+
// Else, we by default emit spans, unless `skipOpenTelemetrySetup` is set to `true` or spans are not enabled
164+
if (typeof options.spans === 'boolean') {
165+
return options.spans;
166+
}
167+
168+
if (clientOptions.skipOpenTelemetrySetup) {
169+
return false;
170+
}
171+
172+
// IMPORTANT: We only disable span instrumentation when spans are not enabled _and_ we are on Node 22+,
173+
// as otherwise the necessary diagnostics channel is not available yet
174+
if (!hasSpansEnabled(clientOptions) && NODE_VERSION.major >= 22) {
175+
return false;
176+
}
177+
178+
return true;
164179
}
165180

166181
/**

packages/node/src/integrations/node-fetch/index.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { UndiciInstrumentationConfig } from '@opentelemetry/instrumentation-undici';
22
import { UndiciInstrumentation } from '@opentelemetry/instrumentation-undici';
33
import type { IntegrationFn } from '@sentry/core';
4-
import { defineIntegration, getClient, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core';
4+
import { defineIntegration, getClient, hasSpansEnabled, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core';
55
import { generateInstrumentOnce } from '../../otel/instrument';
66
import type { NodeClient } from '../../sdk/client';
77
import type { NodeClientOptions } from '../../types';
@@ -86,8 +86,10 @@ function getAbsoluteUrl(origin: string, path: string = '/'): string {
8686

8787
function _shouldInstrumentSpans(options: NodeFetchOptions, clientOptions: Partial<NodeClientOptions> = {}): boolean {
8888
// If `spans` is passed in, it takes precedence
89-
// Else, we by default emit spans, unless `skipOpenTelemetrySetup` is set to `true`
90-
return typeof options.spans === 'boolean' ? options.spans : !clientOptions.skipOpenTelemetrySetup;
89+
// Else, we by default emit spans, unless `skipOpenTelemetrySetup` is set to `true` or spans are not enabled
90+
return typeof options.spans === 'boolean'
91+
? options.spans
92+
: !clientOptions.skipOpenTelemetrySetup && hasSpansEnabled(clientOptions);
9193
}
9294

9395
function getConfigWithDefaults(options: Partial<NodeFetchOptions> = {}): UndiciInstrumentationConfig {
Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,30 @@
11
import { describe, expect, it } from 'vitest';
22
import { _shouldInstrumentSpans } from '../../src/integrations/http';
3+
import { conditionalTest } from '../helpers/conditional';
34

45
describe('httpIntegration', () => {
56
describe('_shouldInstrumentSpans', () => {
67
it.each([
7-
[{}, {}, true],
88
[{ spans: true }, {}, true],
99
[{ spans: false }, {}, false],
1010
[{ spans: true }, { skipOpenTelemetrySetup: true }, true],
1111
[{ spans: false }, { skipOpenTelemetrySetup: true }, false],
1212
[{}, { skipOpenTelemetrySetup: true }, false],
13-
[{}, { skipOpenTelemetrySetup: false }, true],
13+
[{}, { tracesSampleRate: 0, skipOpenTelemetrySetup: true }, false],
14+
[{}, { tracesSampleRate: 0 }, true],
1415
])('returns the correct value for options=%j and clientOptions=%j', (options, clientOptions, expected) => {
1516
const actual = _shouldInstrumentSpans(options, clientOptions);
1617
expect(actual).toBe(expected);
1718
});
19+
20+
conditionalTest({ min: 22 })('returns false without tracesSampleRate on Node >=22', () => {
21+
const actual = _shouldInstrumentSpans({}, {});
22+
expect(actual).toBe(false);
23+
});
24+
25+
conditionalTest({ max: 21 })('returns true without tracesSampleRate on Node <22', () => {
26+
const actual = _shouldInstrumentSpans({}, {});
27+
expect(actual).toBe(true);
28+
});
1829
});
1930
});

0 commit comments

Comments
 (0)