feat(remix): Update remix SDK to be OTEL-powered (#11031)

mydea · web-flow · commit fc208fe43d12 · 2024-03-14T14:46:02.000Z
This updates the Remix SDK to use the new OTEL-powered `@sentry/node`
under the hood.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -871,7 +871,7 @@ jobs:
           yarn test
 
   job_remix_integration_tests:
-    name: Remix v${{ matrix.remix }} (Node ${{ matrix.node }}) ${{ matrix.tracingIntegration && 'TracingIntegration'}} Tests
+    name: Remix v${{ matrix.remix }} (Node ${{ matrix.node }}) Tests
     needs: [job_get_metadata, job_build]
     if: needs.job_get_metadata.outputs.changed_remix == 'true' || github.event_name != 'pull_request'
     runs-on: ubuntu-20.04
diff --git a/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/app/entry.server.tsx b/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/app/entry.server.tsx
@@ -1,25 +1,25 @@
+import * as Sentry from '@sentry/remix';
+
+Sentry.init({
+  tracesSampleRate: 1.0, // Capture 100% of the transactions, reduce in production!
+  environment: 'qa', // dynamic sampling bias to keep transactions
+  dsn: process.env.E2E_TEST_DSN,
+  tunnel: 'http://localhost:3031/', // proxy server
+});
+
 import { PassThrough } from 'node:stream';
 
 import type { AppLoadContext, EntryContext } from '@remix-run/node';
 import { createReadableStreamFromReadable } from '@remix-run/node';
 import { installGlobals } from '@remix-run/node';
 import { RemixServer } from '@remix-run/react';
-import * as Sentry from '@sentry/remix';
 import * as isbotModule from 'isbot';
 import { renderToPipeableStream } from 'react-dom/server';
 
 installGlobals();
 
 const ABORT_DELAY = 5_000;
 
-Sentry.init({
-  environment: 'qa', // dynamic sampling bias to keep transactions
-  dsn: process.env.E2E_TEST_DSN,
-  // Performance Monitoring
-  tracesSampleRate: 1.0, // Capture 100% of the transactions, reduce in production!
-  tunnel: 'http://localhost:3031/', // proxy server
-});
-
 export const handleError = Sentry.wrapRemixHandleError;
 
 export default function handleRequest(
diff --git a/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/tests/behaviour-server.test.ts b/dev-packages/e2e-tests/test-applications/create-remix-app-express-vite-dev/tests/behaviour-server.test.ts
@@ -7,7 +7,13 @@ test('Sends two linked transactions (server & client) to Sentry', async ({ page
   // We use this to identify the transactions
   const testTag = uuid4();
 
-  // no server span here!
+  const httpServerTransactionPromise = waitForTransaction('create-remix-app-express-vite-dev', transactionEvent => {
+    return (
+      transactionEvent.type === 'transaction' &&
+      transactionEvent.contexts?.trace?.op === 'http.server' &&
+      transactionEvent.tags?.['sentry_test'] === testTag
+    );
+  });
 
   const pageLoadTransactionPromise = waitForTransaction('create-remix-app-express-vite-dev', transactionEvent => {
     return (
@@ -20,16 +26,25 @@ test('Sends two linked transactions (server & client) to Sentry', async ({ page
   page.goto(`/?tag=${testTag}`);
 
   const pageloadTransaction = await pageLoadTransactionPromise;
+  const httpServerTransaction = await httpServerTransactionPromise;
 
   expect(pageloadTransaction).toBeDefined();
+  expect(httpServerTransaction).toBeDefined();
+
+  const httpServerTraceId = httpServerTransaction.contexts?.trace?.trace_id;
+  const httpServerSpanId = httpServerTransaction.contexts?.trace?.span_id;
 
   const pageLoadTraceId = pageloadTransaction.contexts?.trace?.trace_id;
   const pageLoadSpanId = pageloadTransaction.contexts?.trace?.span_id;
   const pageLoadParentSpanId = pageloadTransaction.contexts?.trace?.parent_span_id;
 
+  expect(httpServerTransaction.transaction).toBe('routes/_index');
   expect(pageloadTransaction.transaction).toBe('routes/_index');
 
-  expect(pageLoadTraceId).toBeDefined();
-  expect(pageLoadParentSpanId).toBeUndefined();
-  expect(pageLoadSpanId).toBeDefined();
+  expect(httpServerTraceId).toBeDefined();
+  expect(httpServerSpanId).toBeDefined();
+
+  expect(pageLoadTraceId).toEqual(httpServerTraceId);
+  expect(pageLoadParentSpanId).toEqual(httpServerSpanId);
+  expect(pageLoadSpanId).not.toEqual(httpServerSpanId);
 });
diff --git a/dev-packages/e2e-tests/test-applications/create-remix-app-v2/app/entry.server.tsx b/dev-packages/e2e-tests/test-applications/create-remix-app-v2/app/entry.server.tsx
@@ -1,3 +1,12 @@
+import * as Sentry from '@sentry/remix';
+
+Sentry.init({
+  tracesSampleRate: 1.0, // Capture 100% of the transactions, reduce in production!
+  environment: 'qa', // dynamic sampling bias to keep transactions
+  dsn: process.env.E2E_TEST_DSN,
+  tunnel: 'http://localhost:3031/', // proxy server
+});
+
 /**
  * By default, Remix will handle generating the HTTP Response for you.
  * You are free to delete this file if you'd like to, but if you ever want it revealed again, you can run `npx remix reveal` ✨
@@ -10,22 +19,13 @@ import type { AppLoadContext, EntryContext } from '@remix-run/node';
 import { createReadableStreamFromReadable } from '@remix-run/node';
 import { installGlobals } from '@remix-run/node';
 import { RemixServer } from '@remix-run/react';
-import * as Sentry from '@sentry/remix';
 import isbot from 'isbot';
 import { renderToPipeableStream } from 'react-dom/server';
 
 installGlobals();
 
 const ABORT_DELAY = 5_000;
 
-Sentry.init({
-  environment: 'qa', // dynamic sampling bias to keep transactions
-  dsn: process.env.E2E_TEST_DSN,
-  // Performance Monitoring
-  tracesSampleRate: 1.0, // Capture 100% of the transactions, reduce in production!
-  tunnel: 'http://localhost:3031/', // proxy server
-});
-
 export const handleError = Sentry.wrapRemixHandleError;
 
 export default function handleRequest(
diff --git a/dev-packages/e2e-tests/test-applications/create-remix-app/app/entry.server.tsx b/dev-packages/e2e-tests/test-applications/create-remix-app/app/entry.server.tsx
@@ -1,3 +1,12 @@
+import * as Sentry from '@sentry/remix';
+
+Sentry.init({
+  tracesSampleRate: 1.0, // Capture 100% of the transactions, reduce in production!
+  environment: 'qa', // dynamic sampling bias to keep transactions
+  dsn: process.env.E2E_TEST_DSN,
+  tunnel: 'http://localhost:3031/', // proxy server
+});
+
 /**
  * By default, Remix will handle generating the HTTP Response for you.
  * You are free to delete this file if you'd like to, but if you ever want it revealed again, you can run `npx remix reveal` ✨
@@ -9,20 +18,11 @@ import { PassThrough } from 'node:stream';
 import type { AppLoadContext, EntryContext } from '@remix-run/node';
 import { Response } from '@remix-run/node';
 import { RemixServer } from '@remix-run/react';
-import * as Sentry from '@sentry/remix';
 import isbot from 'isbot';
 import { renderToPipeableStream } from 'react-dom/server';
 
 const ABORT_DELAY = 5_000;
 
-Sentry.init({
-  environment: 'qa', // dynamic sampling bias to keep transactions
-  dsn: process.env.E2E_TEST_DSN,
-  // Performance Monitoring
-  tracesSampleRate: 1.0, // Capture 100% of the transactions, reduce in production!
-  tunnel: 'http://localhost:3031/', // proxy server
-});
-
 export const handleError = Sentry.wrapRemixHandleError;
 
 export default function handleRequest(
diff --git a/dev-packages/e2e-tests/test-applications/node-exports-test-app/scripts/consistentExports.ts b/dev-packages/e2e-tests/test-applications/node-exports-test-app/scripts/consistentExports.ts
@@ -71,7 +71,7 @@ const DEPENDENTS: Dependent[] = [
   },
   {
     package: '@sentry/remix',
-    compareWith: nodeExperimentalExports,
+    compareWith: nodeExports,
     exports: Object.keys(SentryRemix),
   },
   {
diff --git a/packages/astro/src/index.types.ts b/packages/astro/src/index.types.ts
@@ -24,7 +24,6 @@ export declare function flush(timeout?: number | undefined): PromiseLike<boolean
 
 // eslint-disable-next-line deprecation/deprecation
 export declare const makeMain: typeof clientSdk.makeMain;
-export declare const getActiveSpan: typeof clientSdk.getActiveSpan;
 // eslint-disable-next-line deprecation/deprecation
 export declare const getCurrentHub: typeof clientSdk.getCurrentHub;
 export declare const getClient: typeof clientSdk.getClient;
diff --git a/packages/opentelemetry/src/index.ts b/packages/opentelemetry/src/index.ts
@@ -18,6 +18,8 @@ export {
   spanHasStatus,
 } from './utils/spanTypes';
 
+export { getDynamicSamplingContextFromSpan } from './utils/dynamicSamplingContext';
+
 export { isSentryRequestSpan } from './utils/isSentryRequest';
 
 export { getActiveSpan } from './utils/getActiveSpan';
diff --git a/packages/remix/jest.config.js b/packages/remix/jest.config.js
@@ -3,4 +3,6 @@ const baseConfig = require('../../jest/jest.config.js');
 module.exports = {
   ...baseConfig,
   testPathIgnorePatterns: ['<rootDir>/build/', '<rootDir>/node_modules/', '<rootDir>/test/integration/'],
+  // Some tests take longer to finish, as flushing spans with OpenTelemetry takes some more time
+  testTimeout: 15000,
 };
diff --git a/packages/remix/package.json b/packages/remix/package.json
@@ -48,7 +48,8 @@
     "@remix-run/router": "1.x",
     "@sentry/cli": "^2.30.0",
     "@sentry/core": "8.0.0-alpha.2",
-    "@sentry/node-experimental": "8.0.0-alpha.2",
+    "@sentry/node": "8.0.0-alpha.2",
+    "@sentry/opentelemetry": "8.0.0-alpha.2",
     "@sentry/react": "8.0.0-alpha.2",
     "@sentry/types": "8.0.0-alpha.2",
     "@sentry/utils": "8.0.0-alpha.2",
@@ -82,7 +83,7 @@
     "fix": "eslint . --format stylish --fix",
     "lint": "eslint . --format stylish",
     "test": "yarn test:unit",
-    "test:integration": "run-s test:integration:v1 test:integration:v2 test:integration:tracingIntegration",
+    "test:integration": "run-s test:integration:v1 test:integration:v2",
     "test:integration:v1": "run-s test:integration:clean test:integration:prepare test:integration:client test:integration:server",
     "test:integration:v2": "export REMIX_VERSION=2 && run-s test:integration:v1",
     "test:integration:ci": "run-s test:integration:clean test:integration:prepare test:integration:client:ci test:integration:server",
diff --git a/packages/remix/src/index.server.ts b/packages/remix/src/index.server.ts
@@ -1,6 +1,6 @@
-import { applySdkMetadata } from '@sentry/core';
-import type { NodeOptions } from '@sentry/node-experimental';
-import { getClient, init as nodeInit, setTag } from '@sentry/node-experimental';
+import { applySdkMetadata, isInitialized } from '@sentry/core';
+import type { NodeOptions } from '@sentry/node';
+import { init as nodeInit, setTag } from '@sentry/node';
 import { logger } from '@sentry/utils';
 
 import { DEBUG_BUILD } from './utils/debug-build';
@@ -10,8 +10,6 @@ import type { RemixOptions } from './utils/remixOptions';
 // We need to explicitly export @sentry/node as they end up under `default` in ESM builds
 // See: https://github.com/getsentry/sentry-javascript/issues/8474
 export {
-  // eslint-disable-next-line deprecation/deprecation
-  addGlobalEventProcessor,
   addEventProcessor,
   addBreadcrumb,
   addIntegration,
@@ -22,8 +20,6 @@ export {
   captureMessage,
   createTransport,
   // eslint-disable-next-line deprecation/deprecation
-  getActiveTransaction,
-  // eslint-disable-next-line deprecation/deprecation
   getCurrentHub,
   getClient,
   getCurrentScope,
@@ -46,7 +42,6 @@ export {
   setHttpStatus,
   withScope,
   withIsolationScope,
-  autoDiscoverNodePerformanceMonitoringIntegrations,
   makeNodeTransport,
   getDefaultIntegrations,
   defaultStackParser,
@@ -56,7 +51,6 @@ export {
   addRequestDataToEvent,
   DEFAULT_USER_INCLUDES,
   extractRequestData,
-  Integrations,
   consoleIntegration,
   onUncaughtExceptionIntegration,
   onUnhandledRejectionIntegration,
@@ -68,7 +62,6 @@ export {
   functionToStringIntegration,
   inboundFiltersIntegration,
   linkedErrorsIntegration,
-  Handlers,
   setMeasurement,
   getActiveSpan,
   getRootSpan,
@@ -83,15 +76,30 @@ export {
   parameterize,
   metrics,
   createGetModuleFromFilename,
-  hapiErrorPlugin,
   SEMANTIC_ATTRIBUTE_SENTRY_OP,
   SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
   SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
   SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE,
-} from '@sentry/node-experimental';
+  expressIntegration,
+  expressErrorHandler,
+  setupExpressErrorHandler,
+  fastifyIntegration,
+  graphqlIntegration,
+  mongoIntegration,
+  mongooseIntegration,
+  mysqlIntegration,
+  mysql2Integration,
+  nestIntegration,
+  postgresIntegration,
+  prismaIntegration,
+  hapiIntegration,
+  setupHapiErrorHandler,
+  spotlightIntegration,
+  setupFastifyErrorHandler,
+} from '@sentry/node';
 
 // Keeping the `*` exports for backwards compatibility and types
-export * from '@sentry/node-experimental';
+export * from '@sentry/node';
 
 export { captureRemixServerException, wrapRemixHandleError } from './utils/instrumentServer';
 export { ErrorBoundary, withErrorBoundary } from '@sentry/react';
@@ -102,15 +110,11 @@ export { wrapExpressCreateRequestHandler } from './utils/serverAdapters/express'
 
 export type { SentryMetaArgs } from './utils/types';
 
-function sdkAlreadyInitialized(): boolean {
-  return !!getClient();
-}
-
 /** Initializes Sentry Remix SDK on Node. */
 export function init(options: RemixOptions): void {
   applySdkMetadata(options, 'remix', ['remix', 'node']);
 
-  if (sdkAlreadyInitialized()) {
+  if (isInitialized()) {
     DEBUG_BUILD && logger.log('SDK already initialized');
 
     return;
diff --git a/packages/remix/src/index.types.ts b/packages/remix/src/index.types.ts
@@ -14,7 +14,7 @@ export declare function init(options: RemixOptions): void;
 
 // We export a merged Integrations object so that users can (at least typing-wise) use all integrations everywhere.
 // eslint-disable-next-line deprecation/deprecation
-export declare const Integrations: typeof clientSdk.Integrations & typeof serverSdk.Integrations;
+export declare const Integrations: typeof clientSdk.Integrations;
 
 export declare const linkedErrorsIntegration: typeof clientSdk.linkedErrorsIntegration;
 export declare const contextLinesIntegration: typeof clientSdk.contextLinesIntegration;
@@ -27,6 +27,13 @@ export declare const defaultStackParser: StackParser;
 // methods from `@sentry/core`.
 declare const runtime: 'client' | 'server';
 
+// eslint-disable-next-line deprecation/deprecation
+export declare const makeMain: typeof clientSdk.makeMain;
+// eslint-disable-next-line deprecation/deprecation
+export declare const getCurrentHub: typeof clientSdk.getCurrentHub;
+export declare const getClient: typeof clientSdk.getClient;
+export declare const continueTrace: typeof clientSdk.continueTrace;
+
 export const close = runtime === 'client' ? clientSdk.close : serverSdk.close;
 export const flush = runtime === 'client' ? clientSdk.flush : serverSdk.flush;
 
diff --git a/packages/remix/src/utils/instrumentServer.ts b/packages/remix/src/utils/instrumentServer.ts
@@ -3,9 +3,11 @@ import {
   SEMANTIC_ATTRIBUTE_SENTRY_OP,
   SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
   SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
+  captureException,
   continueTrace,
+  getActiveSpan,
   getClient,
-  getDynamicSamplingContextFromSpan,
+  getRootSpan,
   handleCallbackErrors,
   hasTracingEnabled,
   setHttpStatus,
@@ -14,7 +16,7 @@ import {
   startSpan,
   withIsolationScope,
 } from '@sentry/core';
-import { captureException, getActiveSpan, getRootSpan } from '@sentry/node-experimental';
+import { getDynamicSamplingContextFromSpan } from '@sentry/opentelemetry';
 import type { Span, TransactionSource, WrappedFunction } from '@sentry/types';
 import {
   addExceptionMechanism,
diff --git a/packages/remix/src/utils/remixOptions.ts b/packages/remix/src/utils/remixOptions.ts
@@ -1,4 +1,4 @@
-import type { NodeOptions } from '@sentry/node-experimental';
+import type { NodeOptions } from '@sentry/node';
 import type { BrowserOptions } from '@sentry/react';
 import type { Options } from '@sentry/types';
 
diff --git a/packages/remix/src/utils/serverAdapters/express.ts b/packages/remix/src/utils/serverAdapters/express.ts
@@ -1,5 +1,5 @@
 import { getClient, getCurrentHub, hasTracingEnabled, setHttpStatus, withIsolationScope } from '@sentry/core';
-import { flush } from '@sentry/node-experimental';
+import { flush } from '@sentry/node';
 import type { Hub, Span } from '@sentry/types';
 import { extractRequestData, fill, isString, logger } from '@sentry/utils';
 
diff --git a/packages/remix/test/index.server.test.ts b/packages/remix/test/index.server.test.ts
@@ -1,6 +1,6 @@
-import * as SentryNode from '@sentry/node-experimental';
+import * as SentryNode from '@sentry/node';
 
-import { Integrations, init } from '../src/index.server';
+import { init } from '../src/index.server';
 
 const nodeInit = jest.spyOn(SentryNode, 'init');
 
@@ -55,9 +55,4 @@ describe('Server init()', () => {
 
     expect(SentryNode.getIsolationScope().getScopeData().tags).toEqual({ runtime: 'node' });
   });
-
-  it('has both node and tracing integrations', () => {
-    expect(Integrations.Apollo).not.toBeUndefined();
-    expect(Integrations.Http).not.toBeUndefined();
-  });
 });
diff --git a/packages/remix/test/integration/app_v1/entry.server.tsx b/packages/remix/test/integration/app_v1/entry.server.tsx
diff --git a/packages/remix/test/integration/app_v2/entry.server.tsx b/packages/remix/test/integration/app_v2/entry.server.tsx
diff --git a/packages/remix/test/integration/package.json b/packages/remix/test/integration/package.json
diff --git a/packages/remix/test/integration/test/server/action.test.ts b/packages/remix/test/integration/test/server/action.test.ts
diff --git a/packages/remix/test/integration/test/server/loader.test.ts b/packages/remix/test/integration/test/server/loader.test.ts
diff --git a/packages/remix/test/integration/test/server/ssr.test.ts b/packages/remix/test/integration/test/server/ssr.test.ts

Original file line number	Diff line number	Diff line change
`@@ -71,7 +71,7 @@ const DEPENDENTS: Dependent[] = [`
`71`	`71`	`},`
`72`	`72`	`{`
`73`	`73`	`package: '@sentry/remix',`
`74`		`- compareWith: nodeExperimentalExports,`
	`74`	`+ compareWith: nodeExports,`
`75`	`75`	`exports: Object.keys(SentryRemix),`
`76`	`76`	`},`
`77`	`77`	`{`