Skip to content

feat(node): Add http.method to node http spans #7991

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

Merged
merged 5 commits into from
May 3, 2023
Merged
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
5 changes: 3 additions & 2 deletions packages/node/src/integrations/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,10 @@ function _createWrappedRequestMethodFactory(

const scope = getCurrentHub().getScope();

const method = requestOptions.method || 'GET';
const requestSpanData: SanitizedRequestData = {
url: requestUrl,
method: requestOptions.method || 'GET',
'http.method': method,
};
if (requestOptions.hash) {
// strip leading "#"
Expand All @@ -205,7 +206,7 @@ function _createWrappedRequestMethodFactory(

if (parentSpan) {
requestSpan = parentSpan.startChild({
description: `${requestSpanData.method} ${requestSpanData.url}`,
description: `${method} ${requestSpanData.url}`,
op: 'http.client',
data: requestSpanData,
});
Expand Down
8 changes: 5 additions & 3 deletions packages/node/src/integrations/undici/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,18 +111,20 @@ export class Undici implements Integration {
const shouldCreateSpan = this._options.shouldCreateSpanForRequest(stringUrl);

if (shouldCreateSpan) {
const data: Record<string, unknown> = {};
const method = request.method || 'GET';
const data: Record<string, unknown> = {
'http.method': method,
};
const params = url.searchParams.toString();
if (params) {
data['http.query'] = `?${params}`;
}
if (url.hash) {
data['http.fragment'] = url.hash;
}

const span = activeSpan.startChild({
op: 'http.client',
description: `${request.method || 'GET'} ${stripUrlQueryAndFragment(stringUrl)}`,
description: `${method} ${stripUrlQueryAndFragment(stringUrl)}`,
data,
});
request.__sentry__ = span;
Expand Down
2 changes: 1 addition & 1 deletion packages/node/test/integrations/http.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ describe('tracing', () => {
// our span is at index 1 because the transaction itself is at index 0
expect(spans[1].description).toEqual('GET http://dogs.are.great/spaniel');
expect(spans[1].op).toEqual('http.client');
expect(spans[1].data.method).toEqual('GET');
expect(spans[1].data['http.method']).toEqual('GET');
expect(spans[1].data.url).toEqual('http://dogs.are.great/spaniel');
expect(spans[1].data['http.query']).toEqual('tail=wag&cute=true');
expect(spans[1].data['http.fragment']).toEqual('learn-more');
Expand Down
10 changes: 10 additions & 0 deletions packages/node/test/integrations/undici.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ conditionalTest({ min: 16 })('Undici integration', () => {
{
description: 'GET http://localhost:18099/',
op: 'http.client',
data: {
'http.method': 'GET',
},
},
],
[
Expand All @@ -66,6 +69,7 @@ conditionalTest({ min: 16 })('Undici integration', () => {
description: 'GET http://localhost:18099/',
op: 'http.client',
data: {
'http.method': 'GET',
'http.query': '?foo=bar',
},
},
Expand All @@ -76,6 +80,9 @@ conditionalTest({ min: 16 })('Undici integration', () => {
{ method: 'POST' },
{
description: 'POST http://localhost:18099/',
data: {
'http.method': 'POST',
},
},
],
[
Expand All @@ -84,6 +91,9 @@ conditionalTest({ min: 16 })('Undici integration', () => {
{ method: 'POST' },
{
description: 'POST http://localhost:18099/',
data: {
'http.method': 'POST',
},
},
],
[
Expand Down
4 changes: 2 additions & 2 deletions packages/sveltekit/src/client/load.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ function instrumentSvelteKitFetch(originalFetch: SvelteKitFetch): SvelteKitFetch

const requestData: SanitizedRequestData = {
url: getSanitizedUrlString(urlObject),
method,
'http.method': method,
...(urlObject.search && { 'http.query': urlObject.search.substring(1) }),
...(urlObject.hash && { 'http.hash': urlObject.hash.substring(1) }),
};
Expand Down Expand Up @@ -190,7 +190,7 @@ function instrumentSvelteKitFetch(originalFetch: SvelteKitFetch): SvelteKitFetch
if (createSpan) {
fetchPromise = trace(
{
name: `${requestData.method} ${requestData.url}`, // this will become the description of the span
name: `${method} ${requestData.url}`, // this will become the description of the span
op: 'http.client',
data: requestData,
},
Expand Down
3 changes: 3 additions & 0 deletions packages/sveltekit/src/server/load.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@ export function wrapServerLoadWithSentry<T extends (...args: any) => any>(origSe
source: routeId ? 'route' : 'url',
dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext,
},
data: {
'http.method': event.request.method,
},
...traceparentData,
};

Expand Down
8 changes: 4 additions & 4 deletions packages/sveltekit/test/client/load.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ describe('wrapLoadWithSentry', () => {
op: 'http.client',
name: 'GET example.com/api/users/',
data: {
method: 'GET',
'http.method': 'GET',
url: 'example.com/api/users/',
'http.hash': 'testfragment',
'http.query': 'id=123',
Expand All @@ -219,7 +219,7 @@ describe('wrapLoadWithSentry', () => {
op: 'http.client',
name: 'POST example.com/api/users/',
data: {
method: 'POST',
'http.method': 'POST',
url: 'example.com/api/users/',
'http.hash': 'testfragment',
'http.query': 'id=123',
Expand All @@ -233,7 +233,7 @@ describe('wrapLoadWithSentry', () => {
op: 'http.client',
name: 'POST example.com/api/users/',
data: {
method: 'POST',
'http.method': 'POST',
url: 'example.com/api/users/',
'http.hash': 'testfragment',
'http.query': 'id=123',
Expand All @@ -247,7 +247,7 @@ describe('wrapLoadWithSentry', () => {
op: 'http.client',
name: 'GET /api/users',
data: {
method: 'GET',
'http.method': 'GET',
url: '/api/users',
'http.query': 'id=123',
},
Expand Down
15 changes: 15 additions & 0 deletions packages/sveltekit/test/server/load.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ const MOCK_LOAD_NO_ROUTE_ARGS: any = {
const MOCK_SERVER_ONLY_LOAD_ARGS: any = {
...MOCK_LOAD_ARGS,
request: {
method: 'GET',
headers: {
get: (key: string) => {
if (key === 'sentry-trace') {
Expand All @@ -87,6 +88,7 @@ const MOCK_SERVER_ONLY_LOAD_ARGS: any = {
const MOCK_SERVER_ONLY_NO_TRACE_LOAD_ARGS: any = {
...MOCK_LOAD_ARGS,
request: {
method: 'GET',
headers: {
get: (_: string) => {
return null;
Expand All @@ -98,6 +100,7 @@ const MOCK_SERVER_ONLY_NO_TRACE_LOAD_ARGS: any = {
const MOCK_SERVER_ONLY_NO_BAGGAGE_LOAD_ARGS: any = {
...MOCK_LOAD_ARGS,
request: {
method: 'GET',
headers: {
get: (key: string) => {
if (key === 'sentry-trace') {
Expand Down Expand Up @@ -268,6 +271,9 @@ describe('wrapServerLoadWithSentry calls trace', () => {
parentSpanId: '1234567890abcdef',
status: 'ok',
traceId: '1234567890abcdef1234567890abcdef',
data: {
'http.method': 'GET',
},
metadata: {
dynamicSamplingContext: {
environment: 'production',
Expand Down Expand Up @@ -296,6 +302,9 @@ describe('wrapServerLoadWithSentry calls trace', () => {
op: 'function.sveltekit.server.load',
name: '/users/[id]',
status: 'ok',
data: {
'http.method': 'GET',
},
metadata: {
source: 'route',
},
Expand All @@ -318,6 +327,9 @@ describe('wrapServerLoadWithSentry calls trace', () => {
parentSpanId: '1234567890abcdef',
status: 'ok',
traceId: '1234567890abcdef1234567890abcdef',
data: {
'http.method': 'GET',
},
metadata: {
dynamicSamplingContext: {},
source: 'route',
Expand All @@ -343,6 +355,9 @@ describe('wrapServerLoadWithSentry calls trace', () => {
parentSpanId: '1234567890abcdef',
status: 'ok',
traceId: '1234567890abcdef1234567890abcdef',
data: {
'http.method': 'GET',
},
metadata: {
dynamicSamplingContext: {
environment: 'production',
Expand Down
2 changes: 1 addition & 1 deletion packages/types/src/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export type QueryParams = string | { [key: string]: string } | Array<[string, st
*/
export type SanitizedRequestData = {
url: string;
method: string;
'http.method': string;
'http.fragment'?: string;
'http.query'?: string;
};