Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(node): Migrate to @fastify/otel #15542

Draft
wants to merge 7 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ test('Sends an API route transaction', async ({ baseURL }) => {
data: {
'sentry.origin': 'manual',
'fastify.type': 'middleware',
'plugin.name': 'fastify -> @fastify/middie',
'plugin.name': '@fastify/middie',
'hook.name': 'onRequest',
},
description: 'middleware - runMiddie',
Expand All @@ -71,7 +71,7 @@ test('Sends an API route transaction', async ({ baseURL }) => {
data: {
'sentry.origin': 'auto.http.otel.fastify',
'sentry.op': 'request_handler.fastify',
'plugin.name': 'fastify -> @fastify/middie',
'plugin.name': '@fastify/middie',
'fastify.type': 'request_handler',
'http.route': '/test-transaction',
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"name": "node-fastify",
"name": "node-fastify-3",
"version": "1.0.0",
"private": true,
"scripts": {
Expand All @@ -15,7 +15,7 @@
"@sentry/core": "latest || *",
"@sentry/opentelemetry": "latest || *",
"@types/node": "^18.19.1",
"fastify": "4.23.2",
"fastify": "3.29.5",
"typescript": "~5.0.0",
"ts-node": "10.9.1"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ import { startEventProxyServer } from '@sentry-internal/test-utils';

startEventProxyServer({
port: 3031,
proxyServerName: 'node-fastify',
proxyServerName: 'node-fastify-3',
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { expect, test } from '@playwright/test';
import { waitForError } from '@sentry-internal/test-utils';

test('Sends correct error event', async ({ baseURL }) => {
const errorEventPromise = waitForError('node-fastify', event => {
const errorEventPromise = waitForError('node-fastify-3', event => {
return !event.type && event.exception?.values?.[0]?.value === 'This is an exception with id 123';
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ import { SpanJSON } from '@sentry/core';
test('Propagates trace for outgoing http requests', async ({ baseURL }) => {
const id = crypto.randomUUID();

const inboundTransactionPromise = waitForTransaction('node-fastify', transactionEvent => {
const inboundTransactionPromise = waitForTransaction('node-fastify-3', transactionEvent => {
return (
transactionEvent.contexts?.trace?.op === 'http.server' &&
transactionEvent.contexts?.trace?.data?.['http.target'] === `/test-inbound-headers/${id}`
);
});

const outboundTransactionPromise = waitForTransaction('node-fastify', transactionEvent => {
const outboundTransactionPromise = waitForTransaction('node-fastify-3', transactionEvent => {
return (
transactionEvent.contexts?.trace?.op === 'http.server' &&
transactionEvent.contexts?.trace?.data?.['http.target'] === `/test-outgoing-http/${id}`
Expand Down Expand Up @@ -120,14 +120,14 @@ test('Propagates trace for outgoing http requests', async ({ baseURL }) => {
test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => {
const id = crypto.randomUUID();

const inboundTransactionPromise = waitForTransaction('node-fastify', transactionEvent => {
const inboundTransactionPromise = waitForTransaction('node-fastify-3', transactionEvent => {
return (
transactionEvent?.contexts?.trace?.op === 'http.server' &&
transactionEvent.contexts?.trace?.data?.['http.target'] === `/test-inbound-headers/${id}`
);
});

const outboundTransactionPromise = waitForTransaction('node-fastify', transactionEvent => {
const outboundTransactionPromise = waitForTransaction('node-fastify-3', transactionEvent => {
return (
transactionEvent?.contexts?.trace?.op === 'http.server' &&
transactionEvent.contexts?.trace?.data?.['http.target'] === `/test-outgoing-fetch/${id}`
Expand Down Expand Up @@ -232,7 +232,7 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => {
});

test('Propagates trace for outgoing external http requests', async ({ baseURL }) => {
const inboundTransactionPromise = waitForTransaction('node-fastify', transactionEvent => {
const inboundTransactionPromise = waitForTransaction('node-fastify-3', transactionEvent => {
return (
transactionEvent?.contexts?.trace?.op === 'http.server' &&
transactionEvent.contexts?.trace?.data?.['http.target'] === `/test-outgoing-http-external-allowed`
Expand Down Expand Up @@ -269,7 +269,7 @@ test('Propagates trace for outgoing external http requests', async ({ baseURL })
});

test('Does not propagate outgoing http requests not covered by tracePropagationTargets', async ({ baseURL }) => {
const inboundTransactionPromise = waitForTransaction('node-fastify', transactionEvent => {
const inboundTransactionPromise = waitForTransaction('node-fastify-3', transactionEvent => {
return (
transactionEvent?.contexts?.trace?.op === 'http.server' &&
transactionEvent.contexts?.trace?.data?.['http.target'] === `/test-outgoing-http-external-disallowed`
Expand All @@ -293,7 +293,7 @@ test('Does not propagate outgoing http requests not covered by tracePropagationT
});

test('Propagates trace for outgoing external fetch requests', async ({ baseURL }) => {
const inboundTransactionPromise = waitForTransaction('node-fastify', transactionEvent => {
const inboundTransactionPromise = waitForTransaction('node-fastify-3', transactionEvent => {
return (
transactionEvent?.contexts?.trace?.op === 'http.server' &&
transactionEvent.contexts?.trace?.data?.['http.target'] === `/test-outgoing-fetch-external-allowed`
Expand Down Expand Up @@ -330,7 +330,7 @@ test('Propagates trace for outgoing external fetch requests', async ({ baseURL }
});

test('Does not propagate outgoing fetch requests not covered by tracePropagationTargets', async ({ baseURL }) => {
const inboundTransactionPromise = waitForTransaction('node-fastify', transactionEvent => {
const inboundTransactionPromise = waitForTransaction('node-fastify-3', transactionEvent => {
return (
transactionEvent?.contexts?.trace?.op === 'http.server' &&
transactionEvent.contexts?.trace?.data?.['http.target'] === `/test-outgoing-fetch-external-disallowed`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { expect, test } from '@playwright/test';
import { waitForTransaction } from '@sentry-internal/test-utils';

test('Sends an API route transaction', async ({ baseURL }) => {
const pageloadTransactionEventPromise = waitForTransaction('node-fastify', transactionEvent => {
const pageloadTransactionEventPromise = waitForTransaction('node-fastify-3', transactionEvent => {
return (
transactionEvent?.contexts?.trace?.op === 'http.server' &&
transactionEvent?.transaction === 'GET /test-transaction'
Expand Down Expand Up @@ -60,7 +60,7 @@ test('Sends an API route transaction', async ({ baseURL }) => {

expect(spans).toContainEqual({
data: {
'plugin.name': 'fastify -> sentry-fastify-error-handler',
'plugin.name': 'sentry-fastify-error-handler',
'fastify.type': 'middleware',
'hook.name': 'onRequest',
'sentry.origin': 'auto.http.otel.fastify',
Expand All @@ -79,7 +79,7 @@ test('Sends an API route transaction', async ({ baseURL }) => {

expect(spans).toContainEqual({
data: {
'plugin.name': 'fastify -> sentry-fastify-error-handler',
'plugin.name': 'sentry-fastify-error-handler',
'fastify.type': 'request_handler',
'http.route': '/test-transaction',
'sentry.op': 'request_handler.fastify',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
dist
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
@sentry:registry=http://127.0.0.1:4873
@sentry-internal:registry=http://127.0.0.1:4873
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{
"name": "node-fastify-4",
"version": "1.0.0",
"private": true,
"scripts": {
"start": "ts-node src/app.ts",
"test": "playwright test",
"clean": "npx rimraf node_modules pnpm-lock.yaml",
"typecheck": "tsc",
"test:build": "pnpm install && pnpm run typecheck",
"test:assert": "pnpm test"
},
"dependencies": {
"@sentry/node": "latest || *",
"@sentry/core": "latest || *",
"@sentry/opentelemetry": "latest || *",
"@types/node": "^18.19.1",
"fastify": "4.29.0",
"typescript": "5.6.3",
"ts-node": "10.9.2"
},
"devDependencies": {
"@playwright/test": "~1.50.0",
"@sentry-internal/test-utils": "link:../../../test-utils"
},
"volta": {
"extends": "../../package.json"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { getPlaywrightConfig } from '@sentry-internal/test-utils';

const config = getPlaywrightConfig({
startCommand: `pnpm start`,
});

export default config;
153 changes: 153 additions & 0 deletions dev-packages/e2e-tests/test-applications/node-fastify-4/src/app.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
import type * as S from '@sentry/node';
const Sentry = require('@sentry/node') as typeof S;

// We wrap console.warn to find out if a warning is incorrectly logged
console.warn = new Proxy(console.warn, {
apply: function (target, thisArg, argumentsList) {
const msg = argumentsList[0];
if (typeof msg === 'string' && msg.startsWith('[Sentry]')) {
console.error(`Sentry warning was triggered: ${msg}`);
process.exit(1);
}

return target.apply(thisArg, argumentsList);
},
});

Sentry.init({
environment: 'qa', // dynamic sampling bias to keep transactions
dsn: process.env.E2E_TEST_DSN,
integrations: [],
tracesSampleRate: 1,
tunnel: 'http://localhost:3031/', // proxy server
tracePropagationTargets: ['http://localhost:3030', '/external-allowed'],
});

import type * as H from 'http';
import type * as F from 'fastify';

// Make sure fastify is imported after Sentry is initialized
const { fastify } = require('fastify') as typeof F;
const http = require('http') as typeof H;

const app = fastify();
const port = 3030;
const port2 = 3040;

Sentry.setupFastifyErrorHandler(app);

app.get('/test-success', function (_req, res) {
res.send({ version: 'v1' });
});

app.get<{ Params: { param: string } }>('/test-param/:param', function (req, res) {
res.send({ paramWas: req.params.param });
});

app.get<{ Params: { id: string } }>('/test-inbound-headers/:id', function (req, res) {
const headers = req.headers;

res.send({ headers, id: req.params.id });
});

app.get<{ Params: { id: string } }>('/test-outgoing-http/:id', async function (req, res) {
const id = req.params.id;
const data = await makeHttpRequest(`http://localhost:3030/test-inbound-headers/${id}`);

res.send(data);
});

app.get<{ Params: { id: string } }>('/test-outgoing-fetch/:id', async function (req, res) {
const id = req.params.id;
const response = await fetch(`http://localhost:3030/test-inbound-headers/${id}`);
const data = await response.json();

res.send(data);
});

app.get('/test-transaction', async function (req, res) {
Sentry.startSpan({ name: 'test-span' }, () => {
Sentry.startSpan({ name: 'child-span' }, () => {});
});

res.send({});
});

app.get('/test-error', async function (req, res) {
const exceptionId = Sentry.captureException(new Error('This is an error'));

await Sentry.flush(2000);

res.send({ exceptionId });
});

app.get<{ Params: { id: string } }>('/test-exception/:id', async function (req, res) {
throw new Error(`This is an exception with id ${req.params.id}`);
});

app.get('/test-outgoing-fetch-external-allowed', async function (req, res) {
const fetchResponse = await fetch(`http://localhost:${port2}/external-allowed`);
const data = await fetchResponse.json();

res.send(data);
});

app.get('/test-outgoing-fetch-external-disallowed', async function (req, res) {
const fetchResponse = await fetch(`http://localhost:${port2}/external-disallowed`);
const data = await fetchResponse.json();

res.send(data);
});

app.get('/test-outgoing-http-external-allowed', async function (req, res) {
const data = await makeHttpRequest(`http://localhost:${port2}/external-allowed`);
res.send(data);
});

app.get('/test-outgoing-http-external-disallowed', async function (req, res) {
const data = await makeHttpRequest(`http://localhost:${port2}/external-disallowed`);
res.send(data);
});

app.listen({ port: port });

// A second app so we can test header propagation between external URLs
const app2 = fastify();
app2.get('/external-allowed', function (req, res) {
const headers = req.headers;

res.send({ headers, route: '/external-allowed' });
});

app2.get('/external-disallowed', function (req, res) {
const headers = req.headers;

res.send({ headers, route: '/external-disallowed' });
});

app2.listen({ port: port2 });

function makeHttpRequest(url: string) {
return new Promise(resolve => {
const data: any[] = [];

http
.request(url, httpRes => {
httpRes.on('data', chunk => {
data.push(chunk);
});
httpRes.on('error', error => {
resolve({ error: error.message, url });
});
httpRes.on('end', () => {
try {
const json = JSON.parse(Buffer.concat(data).toString());
resolve(json);
} catch {
resolve({ data: Buffer.concat(data).toString(), url });
}
});
})
.end();
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { startEventProxyServer } from '@sentry-internal/test-utils';

startEventProxyServer({
port: 3031,
proxyServerName: 'node-fastify-4',
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { expect, test } from '@playwright/test';
import { waitForError } from '@sentry-internal/test-utils';

test('Sends correct error event', async ({ baseURL }) => {
const errorEventPromise = waitForError('node-fastify-4', event => {
return !event.type && event.exception?.values?.[0]?.value === 'This is an exception with id 123';
});

await fetch(`${baseURL}/test-exception/123`);

const errorEvent = await errorEventPromise;

expect(errorEvent.exception?.values).toHaveLength(1);
expect(errorEvent.exception?.values?.[0]?.value).toBe('This is an exception with id 123');

expect(errorEvent.request).toEqual({
method: 'GET',
cookies: {},
headers: expect.any(Object),
url: 'http://localhost:3030/test-exception/123',
});

expect(errorEvent.transaction).toEqual('GET /test-exception/:id');

expect(errorEvent.contexts?.trace).toEqual({
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
span_id: expect.stringMatching(/[a-f0-9]{16}/),
parent_span_id: expect.stringMatching(/[a-f0-9]{16}/),
});
});
Loading
Loading