Skip to content

Commit

Permalink
fix: browserMetrics.ts LoAF for loop
Browse files Browse the repository at this point in the history
  • Loading branch information
chargome authored and horochx committed Jul 26, 2024
1 parent 36f62bb commit 57c3196
Show file tree
Hide file tree
Showing 13 changed files with 370 additions and 24 deletions.
1 change: 1 addition & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -990,6 +990,7 @@ jobs:
'node-express-esm-without-loader',
'node-express-cjs-preload',
'node-otel-sdk-node',
'node-otel-custom-sampler',
'ember-classic',
'ember-embroider',
'nextjs-app-dir',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
dist
.vscode
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,31 @@
{
"name": "node-otel-custom-sampler",
"version": "1.0.0",
"private": true,
"scripts": {
"build": "tsc",
"start": "node dist/app.js",
"test": "playwright test",
"clean": "npx rimraf node_modules pnpm-lock.yaml",
"test:build": "pnpm install && pnpm build",
"test:assert": "pnpm test"
},
"dependencies": {
"@opentelemetry/api": "^1.9.0",
"@opentelemetry/sdk-trace-node": "^1.25.1",
"@sentry/node": "latest || *",
"@sentry/opentelemetry": "latest || *",
"@sentry/types": "latest || *",
"@types/express": "4.17.17",
"@types/node": "18.15.1",
"express": "4.19.2",
"typescript": "4.9.5"
},
"devDependencies": {
"@playwright/test": "^1.44.1",
"@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;
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import './instrument';

import * as Sentry from '@sentry/node';
import express from 'express';

const PORT = 3030;
const app = express();

const wait = (duration: number) => {
return new Promise<void>(res => {
setTimeout(() => res(), duration);
});
};

app.get('/task', async (_req, res) => {
await Sentry.startSpan({ name: 'Long task', op: 'custom.op' }, async () => {
await wait(200);
});
res.send('ok');
});

app.get('/unsampled/task', async (_req, res) => {
await wait(200);
res.send('ok');
});

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('/test-exception/:id', function (req, _res) {
throw new Error(`This is an exception with id ${req.params.id}`);
});

Sentry.setupExpressErrorHandler(app);

app.use(function onError(err: unknown, req: any, res: any, next: any) {
// The error id is attached to `res.sentry` to be returned
// and optionally displayed to the user for support.
res.statusCode = 500;
res.end(res.sentry + '\n');
});

app.listen(PORT, () => {
console.log('App listening on ', PORT);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { Attributes, Context, Link, SpanKind } from '@opentelemetry/api';
import { Sampler, SamplingResult } from '@opentelemetry/sdk-trace-node';
import { wrapSamplingDecision } from '@sentry/opentelemetry';

export class CustomSampler implements Sampler {
public shouldSample(
context: Context,
_traceId: string,
_spanName: string,
_spanKind: SpanKind,
attributes: Attributes,
_links: Link[],
): SamplingResult {
const route = attributes['http.route'];
const target = attributes['http.target'];
const decision =
(typeof route === 'string' && route.includes('/unsampled')) ||
(typeof target === 'string' && target.includes('/unsampled'))
? 0
: 1;
return wrapSamplingDecision({
decision,
context,
spanAttributes: attributes,
});
}

public toString(): string {
return CustomSampler.name;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node';
import * as Sentry from '@sentry/node';
import { SentryPropagator, SentrySpanProcessor } from '@sentry/opentelemetry';
import { CustomSampler } from './custom-sampler';

Sentry.init({
environment: 'qa', // dynamic sampling bias to keep transactions
dsn:
process.env.E2E_TEST_DSN ||
'https://3b6c388182fb435097f41d181be2b2ba@o4504321058471936.ingest.sentry.io/4504321066008576',
includeLocalVariables: true,
debug: !!process.env.DEBUG,
tunnel: `http://localhost:3031/`, // proxy server
skipOpenTelemetrySetup: true,
// By defining _any_ sample rate, tracing intergations will be added by default
tracesSampleRate: 0,
});

const provider = new NodeTracerProvider({
sampler: new CustomSampler(),
});

provider.addSpanProcessor(new SentrySpanProcessor());

provider.register({
propagator: new SentryPropagator(),
contextManager: new Sentry.SentryContextManager(),
});

Sentry.validateOpenTelemetrySetup();
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-otel-custom-sampler',
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { expect, test } from '@playwright/test';
import { waitForError } from '@sentry-internal/test-utils';

test('Sends correct error event', async ({ baseURL }) => {
const errorEventPromise = waitForError('node-otel-custom-sampler', 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.any(String),
span_id: expect.any(String),
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
import { expect, test } from '@playwright/test';
import { waitForTransaction } from '@sentry-internal/test-utils';

test('Sends a sampled API route transaction', async ({ baseURL }) => {
const transactionEventPromise = waitForTransaction('node-otel-custom-sampler', transactionEvent => {
return transactionEvent?.contexts?.trace?.op === 'http.server' && transactionEvent?.transaction === 'GET /task';
});

await fetch(`${baseURL}/task`);

const transactionEvent = await transactionEventPromise;

expect(transactionEvent.contexts?.trace).toEqual({
span_id: expect.any(String),
trace_id: expect.any(String),
data: {
'sentry.source': 'route',
'sentry.op': 'http.server',
'sentry.origin': 'auto.http.otel.http',
url: 'http://localhost:3030/task',
'otel.kind': 'SERVER',
'http.response.status_code': 200,
'http.url': 'http://localhost:3030/task',
'http.host': 'localhost:3030',
'net.host.name': 'localhost',
'http.method': 'GET',
'http.scheme': 'http',
'http.target': '/task',
'http.user_agent': 'node',
'http.flavor': '1.1',
'net.transport': 'ip_tcp',
'net.host.ip': expect.any(String),
'net.host.port': 3030,
'net.peer.ip': expect.any(String),
'net.peer.port': expect.any(Number),
'http.status_code': 200,
'http.status_text': 'OK',
'http.route': '/task',
},
origin: 'auto.http.otel.http',
op: 'http.server',
status: 'ok',
});

expect(transactionEvent.spans?.length).toBe(4);

expect(transactionEvent.spans).toContainEqual({
span_id: expect.any(String),
trace_id: expect.any(String),
data: {
'sentry.origin': 'auto.http.otel.express',
'sentry.op': 'middleware.express',
'http.route': '/',
'express.name': 'query',
'express.type': 'middleware',
},
description: 'query',
parent_span_id: expect.any(String),
start_timestamp: expect.any(Number),
timestamp: expect.any(Number),
status: 'ok',
op: 'middleware.express',
origin: 'auto.http.otel.express',
});

expect(transactionEvent.spans).toContainEqual({
span_id: expect.any(String),
trace_id: expect.any(String),
data: {
'sentry.origin': 'auto.http.otel.express',
'sentry.op': 'middleware.express',
'http.route': '/',
'express.name': 'expressInit',
'express.type': 'middleware',
},
description: 'expressInit',
parent_span_id: expect.any(String),
start_timestamp: expect.any(Number),
timestamp: expect.any(Number),
status: 'ok',
op: 'middleware.express',
origin: 'auto.http.otel.express',
});

expect(transactionEvent.spans).toContainEqual({
span_id: expect.any(String),
trace_id: expect.any(String),
data: {
'sentry.origin': 'auto.http.otel.express',
'sentry.op': 'request_handler.express',
'http.route': '/task',
'express.name': '/task',
'express.type': 'request_handler',
},
description: '/task',
parent_span_id: expect.any(String),
start_timestamp: expect.any(Number),
timestamp: expect.any(Number),
status: 'ok',
op: 'request_handler.express',
origin: 'auto.http.otel.express',
});

expect(transactionEvent.spans).toContainEqual({
span_id: expect.any(String),
trace_id: expect.any(String),
data: {
'sentry.origin': 'manual',
'sentry.op': 'custom.op',
},
description: 'Long task',
parent_span_id: expect.any(String),
start_timestamp: expect.any(Number),
timestamp: expect.any(Number),
status: 'ok',
op: 'custom.op',
origin: 'manual',
});
});

test('Does not send an unsampled API route transaction', async ({ baseURL }) => {
const unsampledTransactionEventPromise = waitForTransaction('node-otel-custom-sampler', transactionEvent => {
return (
transactionEvent?.contexts?.trace?.op === 'http.server' && transactionEvent?.transaction === 'GET /unsampled/task'
);
});

await fetch(`${baseURL}/unsampled/task`);

const promiseShouldNotResolve = () =>
new Promise<void>((resolve, reject) => {
const timeout = setTimeout(() => {
resolve(); // Test passes because promise did not resolve within timeout
}, 1000);

unsampledTransactionEventPromise.then(
() => {
clearTimeout(timeout);
reject(new Error('Promise should not have resolved'));
},
() => {
clearTimeout(timeout);
reject(new Error('Promise should not have been rejected'));
},
);
});

expect(promiseShouldNotResolve()).resolves.not.toThrow();
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"compilerOptions": {
"types": ["node"],
"esModuleInterop": true,
"lib": ["es2018"],
"strict": true,
"outDir": "dist"
},
"include": ["src/**/*.ts"]
}
Loading

0 comments on commit 57c3196

Please sign in to comment.