Skip to content

Commit

Permalink
Add a custom httpIntegration for Remix.
Browse files Browse the repository at this point in the history
  • Loading branch information
onurtemizkan committed May 24, 2024
1 parent be79801 commit 3ec1138
Show file tree
Hide file tree
Showing 10 changed files with 108 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ test('Sends two linked transactions (server & client) to Sentry', async ({ page
const httpServerTransactionPromise = waitForTransaction('create-remix-app-express-vite-dev', transactionEvent => {
return (
transactionEvent.type === 'transaction' &&
transactionEvent.contexts?.trace?.op === 'http' &&
transactionEvent.contexts?.trace?.op === 'http.server' &&
transactionEvent.tags?.['sentry_test'] === testTag
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ test('Sends form data with action error to Sentry', async ({ page }) => {
);

expect(actionSpan).toBeDefined();
expect(actionSpan.op).toBe('http');
expect(actionSpan.op).toBe('action.remix');
expect(actionSpan.data).toMatchObject({
'formData.text': 'test',
'formData.file': 'file.txt',
Expand All @@ -54,17 +54,17 @@ test('Sends a loader span to Sentry', async ({ page }) => {
);

expect(loaderSpan).toBeDefined();
expect(loaderSpan.op).toBe('http');
expect(loaderSpan.op).toBe('loader.remix');
});

test('Sends two linked transactions (server & client) to Sentry', async ({ page }) => {
test('Propagates trace when ErrorBoundary is triggered', async ({ page }) => {
// We use this to identify the transactions
const testTag = uuid4();

const httpServerTransactionPromise = waitForTransaction('create-remix-app-express', transactionEvent => {
return (
transactionEvent.type === 'transaction' &&
transactionEvent.contexts?.trace?.op === 'http' &&
transactionEvent.contexts?.trace?.op === 'http.server' &&
transactionEvent.tags?.['sentry_test'] === testTag
);
});
Expand All @@ -77,7 +77,7 @@ test('Sends two linked transactions (server & client) to Sentry', async ({ page
);
});

page.goto(`/?tag=${testTag}`);
page.goto(`/client-error?tag=${testTag}`);

const pageloadTransaction = await pageLoadTransactionPromise;
const httpServerTransaction = await httpServerTransactionPromise;
Expand All @@ -95,8 +95,8 @@ test('Sends two linked transactions (server & client) to Sentry', async ({ page
const pageLoadSpanId = pageloadTransaction.contexts?.trace?.span_id;
const pageLoadParentSpanId = pageloadTransaction.contexts?.trace?.parent_span_id;

expect(httpServerTransaction.transaction).toBe('GET http://localhost:3030/');
expect(pageloadTransaction.transaction).toBe('routes/_index');
expect(httpServerTransaction.transaction).toBe('GET client-error');
expect(pageloadTransaction.transaction).toBe('routes/client-error');

expect(httpServerTraceId).toBeDefined();
expect(httpServerSpanId).toBeDefined();
Expand All @@ -106,14 +106,14 @@ test('Sends two linked transactions (server & client) to Sentry', async ({ page
expect(pageLoadSpanId).not.toEqual(httpServerSpanId);
});

test('Propagates trace when ErrorBoundary is triggered', async ({ page }) => {
test('Sends two linked transactions (server & client) to Sentry', async ({ page }) => {
// We use this to identify the transactions
const testTag = uuid4();

const httpServerTransactionPromise = waitForTransaction('create-remix-app-express', transactionEvent => {
return (
transactionEvent.type === 'transaction' &&
transactionEvent.contexts?.trace?.op === 'http' &&
transactionEvent.contexts?.trace?.op === 'http.server' &&
transactionEvent.tags?.['sentry_test'] === testTag
);
});
Expand All @@ -126,7 +126,7 @@ test('Propagates trace when ErrorBoundary is triggered', async ({ page }) => {
);
});

page.goto(`/client-error?tag=${testTag}`);
page.goto(`/?tag=${testTag}`);

const pageloadTransaction = await pageLoadTransactionPromise;
const httpServerTransaction = await httpServerTransactionPromise;
Expand All @@ -144,8 +144,8 @@ test('Propagates trace when ErrorBoundary is triggered', async ({ page }) => {
const pageLoadSpanId = pageloadTransaction.contexts?.trace?.span_id;
const pageLoadParentSpanId = pageloadTransaction.contexts?.trace?.parent_span_id;

expect(httpServerTransaction.transaction).toBe('GET client-error');
expect(pageloadTransaction.transaction).toBe('routes/client-error');
expect(httpServerTransaction.transaction).toBe('GET http://localhost:3030/');
expect(pageloadTransaction.transaction).toBe('routes/_index');

expect(httpServerTraceId).toBeDefined();
expect(httpServerSpanId).toBeDefined();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ test('Sends two linked transactions (server & client) to Sentry', async ({ page
const httpServerTransactionPromise = waitForTransaction('create-remix-app-v2', transactionEvent => {
return (
transactionEvent.type === 'transaction' &&
transactionEvent.contexts?.trace?.op === 'http' &&
transactionEvent.contexts?.trace?.op === 'http.server' &&
transactionEvent.tags?.['sentry_test'] === testTag
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ test('Sends two linked transactions (server & client) to Sentry', async ({ page
const httpServerTransactionPromise = waitForTransaction('create-remix-app', transactionEvent => {
return (
transactionEvent.type === 'transaction' &&
transactionEvent.contexts?.trace?.op === 'http' &&
transactionEvent.contexts?.trace?.op === 'http.server' &&
transactionEvent.tags?.['sentry_test'] === testTag
);
});
Expand Down
1 change: 1 addition & 0 deletions packages/remix/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
"access": "public"
},
"dependencies": {
"@opentelemetry/instrumentation-http": "0.51.1",
"@remix-run/router": "1.x",
"@sentry/cli": "^2.31.0",
"@sentry/core": "8.4.0",
Expand Down
2 changes: 2 additions & 0 deletions packages/remix/src/index.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { logger } from '@sentry/utils';

import { DEBUG_BUILD } from './utils/debug-build';
import { instrumentServer } from './utils/instrumentServer';
import { httpIntegration } from './utils/integrations/http';
import { remixIntegration } from './utils/integrations/opentelemetry';
import type { RemixOptions } from './utils/remixOptions';

Expand Down Expand Up @@ -135,6 +136,7 @@ export type { SentryMetaArgs } from './utils/types';
export function getDefaultIntegrations(options: RemixOptions): Integration[] {
return [
...getDefaultNodeIntegrations(options).filter(integration => integration.name !== 'Http'),
httpIntegration(),
remixIntegration(options),
];
}
Expand Down
42 changes: 42 additions & 0 deletions packages/remix/src/utils/integrations/http.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// This integration is ported from the Next.JS SDK.
import { HttpInstrumentation } from '@opentelemetry/instrumentation-http';
import { httpIntegration as originalHttpIntegration } from '@sentry/node';
import type { IntegrationFn } from '@sentry/types';

class RemixHttpIntegration extends HttpInstrumentation {
// Instead of the default behavior, we just don't do any wrapping for incoming requests
protected _getPatchIncomingRequestFunction(_component: 'http' | 'https') {
return (
original: (event: string, ...args: unknown[]) => boolean,
): ((this: unknown, event: string, ...args: unknown[]) => boolean) => {
return function incomingRequest(this: unknown, event: string, ...args: unknown[]): boolean {
return original.apply(this, [event, ...args]);
};
};
}
}

interface HttpOptions {
/**
* Whether breadcrumbs should be recorded for requests.
* Defaults to true
*/
breadcrumbs?: boolean;

/**
* Do not capture spans or breadcrumbs for outgoing HTTP requests to URLs where the given callback returns `true`.
* This controls both span & breadcrumb creation - spans will be non recording if tracing is disabled.
*/
ignoreOutgoingRequests?: (url: string) => boolean;
}

/**
* The http integration instruments Node's internal http and https modules.
* It creates breadcrumbs and spans for outgoing HTTP requests which will be attached to the currently active span.
*/
export const httpIntegration = ((options: HttpOptions = {}) => {
return originalHttpIntegration({
...options,
_instrumentation: RemixHttpIntegration,
});
}) satisfies IntegrationFn;
32 changes: 31 additions & 1 deletion packages/remix/src/utils/integrations/opentelemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import { RemixInstrumentation } from 'opentelemetry-instrumentation-remix';

import { defineIntegration } from '@sentry/core';
import { addOpenTelemetryInstrumentation } from '@sentry/node';
import type { IntegrationFn } from '@sentry/types';
import type { Client, IntegrationFn, Span } from '@sentry/types';
import { SEMANTIC_ATTRIBUTE_SENTRY_OP, spanToJSON } from '../../index.server';
import type { RemixOptions } from '../remixOptions';

const _remixIntegration = ((options?: RemixOptions) => {
Expand All @@ -15,9 +16,38 @@ const _remixIntegration = ((options?: RemixOptions) => {
}),
);
},

setup(client: Client) {
client.on('spanStart', span => {
addRemixSpanAttributes(span);
});
},
};
}) satisfies IntegrationFn;

const addRemixSpanAttributes = (span: Span): void => {
const attributes = spanToJSON(span).data || {};

// this is one of: loader, action, requestHandler
const type = attributes['code.function'];

// If this is already set, or we have no remix span, no need to process again...
if (attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] || !type) {
return;
}

// `requestHandler` span from `opentelemetry-instrumentation-remix` is the main server span.
// It should be marked as the `http.server` operation.
// The incoming requests are skipped by the custom `RemixHttpIntegration` package.
if (type === 'requestHandler') {
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'http.server');
return;
}

// All other spans are marked as `remix` operations with their specific type [loader, action]
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, `${type}.remix`);
};

/**
* Instrumentation for aws-sdk package
*/
Expand Down
22 changes: 11 additions & 11 deletions packages/remix/test/integration/test/server/action.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe('Remix API Actions', () => {
{
data: {
'code.function': 'action',
'sentry.op': 'http',
'sentry.op': 'action.remix',
'otel.kind': 'INTERNAL',
'match.route.id': `routes/action-json-response${useV2 ? '.' : '/'}$id`,
'match.params.id': '123123',
Expand All @@ -30,7 +30,7 @@ describe('Remix API Actions', () => {
{
data: {
'code.function': 'loader',
'sentry.op': 'http',
'sentry.op': 'loader.remix',
'otel.kind': 'INTERNAL',
'match.route.id': `routes/action-json-response${useV2 ? '.' : '/'}$id`,
'match.params.id': '123123',
Expand All @@ -39,7 +39,7 @@ describe('Remix API Actions', () => {
{
data: {
'code.function': 'loader',
'sentry.op': 'http',
'sentry.op': 'loader.remix',
'otel.kind': 'INTERNAL',
'match.route.id': 'root',
'match.params.id': '123123',
Expand Down Expand Up @@ -168,7 +168,7 @@ describe('Remix API Actions', () => {
assertSentryTransaction(transaction_1[2], {
contexts: {
trace: {
op: 'http',
op: 'http.server',
status: 'ok',
data: {
'http.response.status_code': 302,
Expand All @@ -181,7 +181,7 @@ describe('Remix API Actions', () => {
assertSentryTransaction(transaction_2[2], {
contexts: {
trace: {
op: 'http',
op: 'http.server',
status: 'internal_error',
data: {
'http.response.status_code': 500,
Expand Down Expand Up @@ -228,7 +228,7 @@ describe('Remix API Actions', () => {
assertSentryTransaction(transaction[2], {
contexts: {
trace: {
op: 'http',
op: 'http.server',
status: 'internal_error',
data: {
'http.response.status_code': 500,
Expand Down Expand Up @@ -275,7 +275,7 @@ describe('Remix API Actions', () => {
assertSentryTransaction(transaction[2], {
contexts: {
trace: {
op: 'http',
op: 'http.server',
status: 'internal_error',
data: {
'http.response.status_code': 500,
Expand Down Expand Up @@ -322,7 +322,7 @@ describe('Remix API Actions', () => {
assertSentryTransaction(transaction[2], {
contexts: {
trace: {
op: 'http',
op: 'http.server',
status: 'internal_error',
data: {
'http.response.status_code': 500,
Expand Down Expand Up @@ -369,7 +369,7 @@ describe('Remix API Actions', () => {
assertSentryTransaction(transaction[2], {
contexts: {
trace: {
op: 'http',
op: 'http.server',
status: 'internal_error',
data: {
'http.response.status_code': 500,
Expand Down Expand Up @@ -416,7 +416,7 @@ describe('Remix API Actions', () => {
assertSentryTransaction(transaction[2], {
contexts: {
trace: {
op: 'http',
op: 'http.server',
status: 'internal_error',
data: {
'http.response.status_code': 500,
Expand Down Expand Up @@ -463,7 +463,7 @@ describe('Remix API Actions', () => {
assertSentryTransaction(transaction[2], {
contexts: {
trace: {
op: 'http',
op: 'http.server',
status: 'internal_error',
data: {
'http.response.status_code': 500,
Expand Down
12 changes: 6 additions & 6 deletions packages/remix/test/integration/test/server/loader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,15 +103,15 @@ describe('Remix API Loaders', () => {
data: {
'code.function': 'loader',
'otel.kind': 'INTERNAL',
'sentry.op': 'http',
'sentry.op': 'loader.remix',
},
origin: 'manual',
},
{
data: {
'code.function': 'loader',
'otel.kind': 'INTERNAL',
'sentry.op': 'http',
'sentry.op': 'loader.remix',
},
origin: 'manual',
},
Expand All @@ -135,7 +135,7 @@ describe('Remix API Loaders', () => {
assertSentryTransaction(transaction_1[2], {
contexts: {
trace: {
op: 'http',
op: 'http.server',
status: 'ok',
data: {
'http.response.status_code': 302,
Expand All @@ -148,7 +148,7 @@ describe('Remix API Loaders', () => {
assertSentryTransaction(transaction_2[2], {
contexts: {
trace: {
op: 'http',
op: 'http.server',
status: 'internal_error',
data: {
'http.response.status_code': 500,
Expand Down Expand Up @@ -243,15 +243,15 @@ describe('Remix API Loaders', () => {
{
data: {
'code.function': 'loader',
'sentry.op': 'http',
'sentry.op': 'loader.remix',
'otel.kind': 'INTERNAL',
'match.route.id': `routes/loader-defer-response${useV2 ? '.' : '/'}$id`,
},
},
{
data: {
'code.function': 'loader',
'sentry.op': 'http',
'sentry.op': 'loader.remix',
'otel.kind': 'INTERNAL',
'match.route.id': 'root',
},
Expand Down

0 comments on commit 3ec1138

Please sign in to comment.