Skip to content
Open
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
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

Also, the server no longer sends spans of tunnel requests when span streaming is on, because these spans only show the SDK's own traffic.

Work in this release was contributed by @adityareddy-dev, @jpdefo, @Jxxunnn, @philipatkinson, @Shubham-Padkonde, @mikamikasuki, and @mketiku. Thank you for your contributions!
Work in this release was contributed by @adityareddy-dev, @jpdefo, @Jxxunnn, @philipatkinson, @Shubham-Padkonde, @mikamikasuki, @mketiku, and @itisrmk. Thank you for your contributions!

## 11.8.0

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import * as Sentry from '@sentry/node';
import { loggingTransport } from '@sentry-internal/node-integration-tests';

Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
release: '1.0',
tracesSampleRate: 1.0,
transport: loggingTransport,
traceLifecycle: 'static',
});
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,18 @@ const run = async () => {
},
});

server.route({
method: ['GET', 'POST'],
path: '/multi',
handler: () => 'Hello World!',
});

server.route({
method: '*',
path: '/wildcard',
handler: () => 'Hello World!',
});

server.route({
method: 'GET',
path: '/error',
Expand All @@ -34,6 +46,14 @@ const run = async () => {
},
});

server.route({
method: '*',
path: '/wildcard-error',
handler: () => {
return new Error('Sentry Test Error');
},
});

server.route({
method: 'GET',
path: '/boom-error',
Expand Down
72 changes: 72 additions & 0 deletions dev-packages/node-integration-tests/suites/tracing/hapi/test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { HTTP_REQUEST_METHOD, SENTRY_OP } from '@sentry/conventions/attributes';
import { ROUTER } from '@sentry/conventions/op';
import { afterAll, describe, expect } from 'vitest';
import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner';

Expand All @@ -19,7 +21,36 @@ describe('hapi auto-instrumentation', () => {
},
};

const MULTI_METHOD_REQUESTS = [
{ method: 'get', path: '/multi' },
{ method: 'post', path: '/multi' },
{ method: 'put', path: '/wildcard' },
] as const;

createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => {
test('should record the request method for multi-method and wildcard routes.', async () => {
const runner = createRunner().unordered();
for (const { method, path } of MULTI_METHOD_REQUESTS) {
const httpMethod = method.toUpperCase();
runner.expect({
span: container => {
expect(container.items.find(item => item.is_segment)?.name).toBe(`${httpMethod} ${path}`);

const routerSpans = container.items.filter(item => item.attributes[SENTRY_OP]?.value === ROUTER);
expect(routerSpans).toHaveLength(1);
expect(routerSpans[0]?.name).toBe(path);
expect(routerSpans[0]?.attributes[HTTP_REQUEST_METHOD]).toEqual({ type: 'string', value: httpMethod });
},
});
}

const started = runner.start();
for (const { method, path } of MULTI_METHOD_REQUESTS) {
await started.makeRequest(method, path);
}
await started.completed();
});

test('should auto-instrument `@hapi/hapi` package.', async () => {
const runner = createRunner()
.expect({
Expand Down Expand Up @@ -114,6 +145,20 @@ describe('hapi auto-instrumentation', () => {
await runner.completed();
});

test('should assign the request method to the transactionName of errors in wildcard routes.', async () => {
const runner = createRunner()
.expect({
event: {
...EXPECTED_ERROR_EVENT,
transaction: 'PUT /wildcard-error',
},
})
.ignore('span')
.start();
runner.makeRequest('put', '/wildcard-error', { expectError: true });
await runner.completed();
});

test('should handle returned Boom errors in routes.', async () => {
const runner = createRunner()
.unordered()
Expand Down Expand Up @@ -143,6 +188,33 @@ describe('hapi auto-instrumentation', () => {
});
});

// Only static spans put the method into the router span name, which crashed for multi-method routes.
createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument-static.mjs', (createRunner, test) => {
test('should name router spans after the request method for multi-method and wildcard routes.', async () => {
const runner = createRunner().unordered();
for (const { method, path } of MULTI_METHOD_REQUESTS) {
const httpMethod = method.toUpperCase();
const spanName = `${httpMethod} ${path}`;
runner.expect({
transaction: transaction => {
expect(transaction.transaction).toBe(spanName);

const routerSpans = transaction.spans?.filter(span => span.op === ROUTER) ?? [];
expect(routerSpans).toHaveLength(1);
expect(routerSpans[0]?.description).toBe(spanName);
expect(routerSpans[0]?.data[HTTP_REQUEST_METHOD]).toBe(httpMethod);
},
});
}

const started = runner.start();
for (const { method, path } of MULTI_METHOD_REQUESTS) {
await started.makeRequest(method, path);
}
await started.completed();
});
});

// Regression test: a `setupHapiErrorHandler` call before `server.start()` installs the default
// predicate. The integration's own auto-registration (with a custom `shouldHandleError`) fires later,
// at `server.start()`, and must still win. The custom predicate drops "Dropped error" (which the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export function attachHapiErrorHandler(server: HapiServer, shouldHandleError?: H
if (getIsolationScope() !== getDefaultIsolationScope()) {
const route = request.route;
if (route?.path) {
getIsolationScope().setTransactionName(`${route.method.toUpperCase()} ${route.path}`);
getIsolationScope().setTransactionName(`${request.method.toUpperCase()} ${route.path}`);
}
} else {
DEBUG_BUILD &&
Expand Down
3 changes: 2 additions & 1 deletion packages/server-utils/src/integrations/hapi/hapi-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export interface ServerRouteOptions {

export interface ServerRoute {
path: string;
method: string;
method: string | string[];
handler?: LifecycleMethod | unknown;
options?: ((server: unknown) => ServerRouteOptions) | ServerRouteOptions;
[key: string]: unknown;
Expand Down Expand Up @@ -96,6 +96,7 @@ export interface HapiResponse {

/** The subset of a hapi request the error listener reads. */
export interface HapiRequest {
method: string;
route: { path?: string; method: string };
response?: HapiResponse;
[key: string]: unknown;
Expand Down
26 changes: 20 additions & 6 deletions packages/server-utils/src/integrations/hapi/hapi-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import type {
ServerExtEventsObject,
ServerExtEventsRequestObject,
ServerRequestExtType,
ServerRoute,
ServerRouteOptions,
} from './hapi-types';
import { HTTP_REQUEST_METHOD, HTTP_ROUTE } from '@sentry/conventions/attributes';
Expand Down Expand Up @@ -108,10 +107,18 @@ const isPatchableExtMethod = (
};

/** Build the span name and attributes for a Hapi route. */
export const getRouteMetadata = (route: ServerRoute, pluginName?: string): SpanMetadata => {
export const getRouteMetadata = ({
path,
method,
pluginName,
}: {
path: string;
method?: string;
pluginName?: string;
}): SpanMetadata => {
const attributes: SpanAttributes = {
[HTTP_ROUTE]: route.path,
[HTTP_REQUEST_METHOD]: route.method,
[HTTP_ROUTE]: path,
[HTTP_REQUEST_METHOD]: method,
};

if (pluginName) {
Expand All @@ -126,10 +133,11 @@ export const getRouteMetadata = (route: ServerRoute, pluginName?: string): SpanM
// handler spans are named after their route alone, without the method prefix.
const isStreamedSpan = !!client && hasSpanStreamingEnabled(client);
const fallbackName = pluginName ? REQUEST_HANDLER_SPAN_NAME_FALLBACK : ROUTER_SPAN_NAME_FALLBACK;
const staticName = method ? `${method} ${path}` : path;

return {
attributes,
name: isStreamedSpan ? route.path || fallbackName : `${route.method.toUpperCase()} ${route.path}`,
name: isStreamedSpan ? path || fallbackName : staticName,
};
};

Expand Down Expand Up @@ -185,6 +193,12 @@ function startMetadataSpan(metadata: SpanMetadata, original: () => unknown): unk
);
}

// The route config can register several methods (or `'*'`), so read the method
// Hapi resolved for this request instead.
function getRequestMethod(request: unknown): string | undefined {
return isObjectLike(request) && typeof request.method === 'string' ? request.method.toUpperCase() : undefined;
}

/**
* Patches each individual route handler method in order to create the span. It
* does not create spans when there is no parent span.
Expand All @@ -199,7 +213,7 @@ function wrapRouteHandler(route: PatchableServerRoute, pluginName?: string): Pat
return oldHandler.call(this, ...params);
}
setHttpServerSpanRouteAttribute(route.path);
const metadata = getRouteMetadata(route, pluginName);
const metadata = getRouteMetadata({ path: route.path, method: getRequestMethod(params[0]), pluginName });
return startMetadataSpan(metadata, () => oldHandler.call(this, ...params));
};
};
Expand Down
78 changes: 63 additions & 15 deletions packages/server-utils/test/integrations/hapi-utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import { setCurrentClient } from '@sentry/core';
import { HTTP_REQUEST_METHOD, HTTP_ROUTE, SENTRY_OP, SENTRY_ORIGIN } from '@sentry/conventions/attributes';
import { ROUTER } from '@sentry/conventions/op';
import type { Span } from '@sentry/core';
import { setCurrentClient, spanToJSON, startSpan } from '@sentry/core';
import { afterEach, describe, expect, it } from 'vitest';
import { getExtMetadata, getRouteMetadata } from '../../src/integrations/hapi/hapi-utils';
import { AttributeNames } from '../../src/integrations/hapi/hapi-types';
import { getExtMetadata, getRouteMetadata, wrapRouteArguments } from '../../src/integrations/hapi/hapi-utils';
import { getDefaultTestClientOptions, TestClient } from '../mocks/client';

describe('getRouteMetadata', () => {
const route = { path: '/users/{id}', method: 'get' } as any;
const route = { path: '/users/{id}', method: 'GET' };

afterEach(() => {
setCurrentClient(undefined as unknown as TestClient);
Expand All @@ -14,21 +18,31 @@ describe('getRouteMetadata', () => {
expect(getRouteMetadata(route)).toEqual({
name: 'GET /users/{id}',
attributes: {
'http.route': '/users/{id}',
'http.request.method': 'get',
'hapi.type': 'router',
[HTTP_ROUTE]: '/users/{id}',
[HTTP_REQUEST_METHOD]: 'GET',
[AttributeNames.HAPI_TYPE]: 'router',
},
});
});

it('describes a plugin-registered route as a plugin layer', () => {
expect(getRouteMetadata(route, 'my-plugin')).toEqual({
expect(getRouteMetadata({ ...route, pluginName: 'my-plugin' })).toEqual({
name: 'GET /users/{id}',
attributes: {
'http.route': '/users/{id}',
'http.request.method': 'get',
'hapi.type': 'plugin',
'hapi.plugin.name': 'my-plugin',
[HTTP_ROUTE]: '/users/{id}',
[HTTP_REQUEST_METHOD]: 'GET',
[AttributeNames.HAPI_TYPE]: 'plugin',
[AttributeNames.PLUGIN_NAME]: 'my-plugin',
},
});
});

it('names the span after the path alone when the request method is unknown', () => {
expect(getRouteMetadata({ path: '/users/{id}' })).toEqual({
name: '/users/{id}',
attributes: {
[HTTP_ROUTE]: '/users/{id}',
[AttributeNames.HAPI_TYPE]: 'router',
},
});
});
Expand All @@ -44,17 +58,51 @@ describe('getRouteMetadata', () => {
const client = new TestClient(getDefaultTestClientOptions({ traceLifecycle: 'stream' }));
setCurrentClient(client);

expect(getRouteMetadata(route, 'my-plugin').name).toBe('/users/{id}');
expect(getRouteMetadata({ ...route, pluginName: 'my-plugin' }).name).toBe('/users/{id}');
});

it('falls back to a static span name when the route has no path', () => {
const client = new TestClient(getDefaultTestClientOptions({ traceLifecycle: 'stream' }));
setCurrentClient(client);

const pathlessRoute = { path: '', method: 'get' } as any;
expect(getRouteMetadata({ path: '', method: 'GET' }).name).toBe('Router');
expect(getRouteMetadata({ path: '', method: 'GET', pluginName: 'my-plugin' }).name).toBe('Request handler');
});
});

describe('wrapRouteArguments', () => {
afterEach(() => {
setCurrentClient(undefined as unknown as TestClient);
});

it.each([
{ request: { method: 'post' }, name: 'POST /multi', method: 'POST' },
{ request: {}, name: '/multi', method: undefined },
])('names the router span $name for a multi-method route', ({ request, name, method }) => {
const client = new TestClient(getDefaultTestClientOptions({ tracesSampleRate: 1 }));
setCurrentClient(client);
client.init();
const endedSpans: Span[] = [];
client.on('spanEnd', span => endedSpans.push(span));
const route = {
method: ['GET', 'POST'],
path: '/multi',
handler: (_request: unknown, _h: unknown): unknown => 'ok',
};
wrapRouteArguments([route]);

const response = startSpan({ name: 'incoming' }, () => route.handler(request, {}));

expect(getRouteMetadata(pathlessRoute).name).toBe('Router');
expect(getRouteMetadata(pathlessRoute, 'my-plugin').name).toBe('Request handler');
expect(response).toBe('ok');
const routerSpan = endedSpans.map(span => spanToJSON(span)).find(span => span.attributes[SENTRY_OP] === ROUTER);
expect(routerSpan?.name).toBe(name);
expect(routerSpan?.attributes).toEqual({
[HTTP_ROUTE]: '/multi',
[HTTP_REQUEST_METHOD]: method,
[AttributeNames.HAPI_TYPE]: 'router',
[SENTRY_OP]: ROUTER,
[SENTRY_ORIGIN]: 'auto.http.hapi',
});
});
});

Expand Down
Loading