Skip to content

Commit a4e888f

Browse files
authored
fix(opentelemetry): Add conditional browser export to avoid node deps (#20556)
We accidentally added a `node` dependency to an export from `opentelemetry` package, leading to problems for users using this in a browser environment. This PR adds conditional exports to the opentelemetry package, where for `browser` targets we have stubs for the node-only thing. This should generally work the same as before, but stop failing builds in browser envs. I had to adjust browser integration tests for this a bit, as they did some unnecessary aliasing which prevented webpack from using normal conditional exports. We are simply doing less now and doing regular dependency resolution which should work as expected (hopefully).
1 parent d9a94ba commit a4e888f

12 files changed

Lines changed: 155 additions & 56 deletions

File tree

dev-packages/browser-integration-tests/package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@
6161
"@playwright/test": "~1.56.0",
6262
"@sentry-internal/rrweb": "2.34.0",
6363
"@sentry/browser": "10.50.0",
64+
"@sentry-internal/replay": "10.50.0",
65+
"@sentry/opentelemetry": "10.50.0",
6466
"@supabase/supabase-js": "2.49.3",
6567
"axios": "1.15.0",
6668
"babel-loader": "^10.1.1",
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import * as Sentry from '@sentry/browser';
2+
3+
window.Sentry = Sentry;
4+
5+
Sentry.init({
6+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
7+
});
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import * as SentryOpenTelemetry from '@sentry/opentelemetry';
2+
import * as Sentry from '@sentry/browser';
3+
4+
// Verify that generally all imports can be resolved
5+
// oxlint-disable-next-line no-console
6+
for (const key in SentryOpenTelemetry) {
7+
console.log(key, SentryOpenTelemetry[key]);
8+
}
9+
10+
// Verify that it console.errors if calling node-only thing
11+
new SentryOpenTelemetry.SentryAsyncLocalStorageContextManager();
12+
13+
Sentry.captureException(new Error('test'));
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { expect } from '@playwright/test';
2+
import { sentryTest } from '../../../utils/fixtures';
3+
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../utils/helpers';
4+
5+
sentryTest('Should allow importing from @sentry/opentelemetry package', async ({ getLocalTestUrl, page }) => {
6+
const bundle = process.env.PW_BUNDLE;
7+
8+
if (bundle && bundle.includes('bundle')) {
9+
sentryTest.skip();
10+
return;
11+
}
12+
13+
const consoleMessages: string[] = [];
14+
page.on('console', msg => {
15+
consoleMessages.push(msg.text());
16+
});
17+
18+
const url = await getLocalTestUrl({ testDir: __dirname });
19+
const req = await waitForErrorRequestOnUrl(page, url);
20+
const eventData = envelopeRequestParser(req);
21+
22+
expect(eventData.exception?.values).toHaveLength(1);
23+
expect(eventData.exception?.values?.[0].value).toBe('test');
24+
25+
expect(consoleMessages).toContainEqual('SentryAsyncLocalStorageContextManager is not supported in the browser');
26+
});

dev-packages/browser-integration-tests/utils/generatePage.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,13 @@ export async function generatePage(
3838
compiler.run(err => {
3939
if (err) {
4040
reject(err);
41+
return;
4142
}
4243

4344
compiler.close(err => {
4445
if (err) {
4546
reject(err);
47+
return;
4648
}
4749

4850
resolve();

dev-packages/browser-integration-tests/utils/generatePlugin.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,10 @@ export const LOADER_CONFIGS: Record<string, { options: Record<string, unknown>;
147147
* so that the compiled versions aren't included
148148
*/
149149
function generateSentryAlias(): Record<string, string> {
150+
if (!useBundleOrLoader) {
151+
return {};
152+
}
153+
150154
const rootPackageJson = JSON.parse(fs.readFileSync(ROOT_PACKAGE_JSON_PATH, 'utf8')) as { workspaces: string[] };
151155
const packageNames = rootPackageJson.workspaces
152156
.filter(workspace => !workspace.startsWith('dev-packages/'))
@@ -189,7 +193,10 @@ class SentryScenarioGenerationPlugin {
189193
}
190194

191195
public apply(compiler: Compiler): void {
192-
compiler.options.resolve.alias = generateSentryAlias();
196+
const sentryAlias = generateSentryAlias();
197+
if (Object.keys(sentryAlias).length > 0) {
198+
compiler.options.resolve.alias = sentryAlias;
199+
}
193200
compiler.options.externals = useBundleOrLoader
194201
? {
195202
// To help Webpack resolve Sentry modules in `import` statements in cases where they're provided in bundles rather than in `node_modules`

dev-packages/browser-integration-tests/webpack.config.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,11 @@ import type { Configuration } from 'webpack';
33
const config = function (userConfig: Record<string, unknown>): Configuration {
44
return {
55
...userConfig,
6+
target: 'web',
67
mode: 'none',
8+
resolve: {
9+
conditionNames: ['webpack', 'import', 'require', 'browser', 'default'],
10+
},
711
module: {
812
rules: [
913
{

packages/opentelemetry/package.json

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,26 @@
1919
"./package.json": "./package.json",
2020
".": {
2121
"import": {
22+
"node": {
23+
"types": "./build/types/index.d.ts",
24+
"default": "./build/esm/index.js"
25+
},
26+
"browser": {
27+
"types": "./build/types/index.d.ts",
28+
"default": "./build/esm/index.browser.js"
29+
},
2230
"types": "./build/types/index.d.ts",
2331
"default": "./build/esm/index.js"
2432
},
2533
"require": {
34+
"node": {
35+
"types": "./build/types/index.d.ts",
36+
"default": "./build/cjs/index.js"
37+
},
38+
"browser": {
39+
"types": "./build/types/index.d.ts",
40+
"default": "./build/cjs/index.browser.js"
41+
},
2642
"types": "./build/types/index.d.ts",
2743
"default": "./build/cjs/index.js"
2844
}

packages/opentelemetry/rollup.npm.config.mjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ export default makeNPMConfigVariants(
44
makeBaseNPMConfig({
55
// `tracingChannel` is a Node.js-only subpath so `node:diagnostics_channel`
66
// isn't pulled into the main bundle (breaks edge/browser builds).
7-
entrypoints: ['src/index.ts', 'src/tracingChannel.ts'],
7+
entrypoints: ['src/index.ts', 'src/tracingChannel.ts', 'src/index.browser.ts'],
88
packageSpecificConfig: {
99
output: {
1010
// set exports to 'named' or 'auto' so that rollup doesn't warn
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
export { SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION } from './semanticAttributes';
2+
3+
export { getRequestSpanData } from './utils/getRequestSpanData';
4+
5+
export type { OpenTelemetryClient } from './types';
6+
export { wrapClientClass } from './custom/client';
7+
8+
export { getSpanKind } from './utils/getSpanKind';
9+
10+
export { getScopesFromContext } from './utils/contextData';
11+
12+
export {
13+
spanHasAttributes,
14+
spanHasEvents,
15+
spanHasKind,
16+
spanHasName,
17+
spanHasParentId,
18+
spanHasStatus,
19+
} from './utils/spanTypes';
20+
21+
// Re-export this for backwards compatibility (this used to be a different implementation)
22+
export { getDynamicSamplingContextFromSpan } from '@sentry/core';
23+
24+
export { isSentryRequestSpan } from './utils/isSentryRequest';
25+
26+
export { enhanceDscWithOpenTelemetryRootSpanName } from './utils/enhanceDscWithOpenTelemetryRootSpanName';
27+
28+
export { getActiveSpan } from './utils/getActiveSpan';
29+
export {
30+
startSpan,
31+
startSpanManual,
32+
startInactiveSpan,
33+
withActiveSpan,
34+
continueTrace,
35+
getTraceContextForScope,
36+
} from './trace';
37+
38+
export { suppressTracing } from './utils/suppressTracing';
39+
40+
export { setupEventContextTrace } from './setupEventContextTrace';
41+
42+
export { setOpenTelemetryContextAsyncContextStrategy } from './asyncContextStrategy';
43+
export { wrapContextManagerClass } from './contextManager';
44+
45+
export { SentryPropagator, shouldPropagateTraceForUrl } from './propagator';
46+
export { SentrySpanProcessor } from './spanProcessor';
47+
export { SentrySampler, wrapSamplingDecision } from './sampler';
48+
49+
export { openTelemetrySetupCheck } from './utils/setupCheck';
50+
51+
export { getSentryResource } from './resource';
52+
53+
export { withStreamedSpan } from '@sentry/core';
54+
55+
// Legacy
56+
export { getClient } from '@sentry/core';

0 commit comments

Comments
 (0)