Skip to content
This repository has been archived by the owner on May 3, 2024. It is now read-only.

Commit

Permalink
feat(tracing): improve span names for HTTP requests
Browse files Browse the repository at this point in the history
  • Loading branch information
10xLaCroixDrinker committed Feb 13, 2024
1 parent 9c20a9a commit d4bdbce
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 67 deletions.
45 changes: 32 additions & 13 deletions __tests__/integration/one-app.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1580,15 +1580,30 @@ describe('Tests that can run against either local Docker setup or remote One App
);
const getSpanByName = (spans, name) => spans.find((span) => span.name === name);

const formatSpansForSnapshot = (spans, includeAttributes = []) => spans.map(
(span) => `${span.name}: ${JSON.stringify(
span.attributes
.filter((attribute) => (includeAttributes.length === 0
? true
: includeAttributes.includes(attribute.key))
)
.reduce(
(acc, attr) => ({
...acc,
[attr.key]: attr.value.stringValue,
}),
{}
)
)}`
);

const { traceId, spanId: parentSpanId } = getSpanByAttribute(
getSpans('@opentelemetry/instrumentation-http'),
{ key: 'http.target', value: target }
);

const httpSpans = getSpans('@opentelemetry/instrumentation-http', traceId);
expect(httpSpans.length).toBe(2);
const dnsSpans = getSpans('@opentelemetry/instrumentation-dns', traceId);
expect(dnsSpans.length).toBe(1);
const spans = getSpans('@autotelic/fastify-opentelemetry', traceId);
expect(spans.length).toBe(14);

Expand All @@ -1601,19 +1616,23 @@ describe('Tests that can run against either local Docker setup or remote One App
'createRequestHtmlFragment -> loadModuleData'
);

expect(dnsSpans[0].parentSpanId).toBe(dataFetchSpan.spanId);
expect(dataFetchSpan.parentSpanId).toBe(loadModuleDataSpan.spanId);

expect(
spans.map(
(span) => `${span.name}: ${JSON.stringify(
span.attributes.reduce(
(acc, attr) => ({ ...acc, [attr.key]: attr.value.stringValue }),
{}
)
)}`
)
).toMatchInlineSnapshot(`
expect(formatSpansForSnapshot(httpSpans, ['direction'])).toMatchInlineSnapshot(`
[
"GET https://fast.api.frank/posts: {"direction":"out"}",
"GET /healthy-frank/ssr-frank: {"direction":"in"}",
]
`);

expect(httpSpans.map((span) => span.name)).toMatchInlineSnapshot(`
[
"GET https://fast.api.frank/posts",
"GET /healthy-frank/ssr-frank",
]
`);

expect(formatSpansForSnapshot(spans)).toMatchInlineSnapshot(`
[
"addSecurityHeaders: {"req.method":"GET","req.url":"/healthy-frank/ssr-frank"}",
"addFrameOptionsHeader: {"req.method":"GET","req.url":"/healthy-frank/ssr-frank"}",
Expand Down
68 changes: 36 additions & 32 deletions __tests__/server/utils/tracer.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import { HttpInstrumentation } from '@opentelemetry/instrumentation-http';
import { PinoInstrumentation } from '@opentelemetry/instrumentation-pino';
// eslint-disable-next-line no-unused-vars -- required for mocking
import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node';
import { DnsInstrumentation } from '@opentelemetry/instrumentation-dns';
import {
BatchSpanProcessor,
SimpleSpanProcessor,
Expand All @@ -32,22 +31,22 @@ import { OTLPTraceExporter } from '@opentelemetry/exporter-trace-otlp-grpc';
jest.mock('@opentelemetry/instrumentation');
jest.mock('@opentelemetry/instrumentation-http');
jest.mock('@opentelemetry/instrumentation-pino');
jest.mock('@opentelemetry/sdk-trace-base');
jest.mock('@opentelemetry/exporter-trace-otlp-grpc');

jest.mock('@opentelemetry/sdk-trace-node', () => {
const { NodeTracerProvider: ActualNodeTracerProvider } = jest.requireActual(
'@opentelemetry/sdk-trace-node'
);
return {
NodeTracerProvider: jest.fn(() => {
const tracerProvider = new ActualNodeTracerProvider();
tracerProvider.register = jest.fn();
tracerProvider.addSpanProcessor = jest.fn();
jest.spyOn(tracerProvider, 'register');
jest.spyOn(tracerProvider, 'addSpanProcessor');
return tracerProvider;
}),
};
});
jest.mock('@opentelemetry/instrumentation-dns');
jest.mock('@opentelemetry/sdk-trace-base');
jest.mock('@opentelemetry/exporter-trace-otlp-grpc');

jest.mock('../../../src/server/utils/getOtelResourceAttributes', () => () => ({
'test-resource-attribute': 'test-resource-attribute-value',
Expand Down Expand Up @@ -124,36 +123,11 @@ describe('tracer', () => {
expect(registerInstrumentations).toHaveBeenCalledTimes(1);
expect(registerInstrumentations).toHaveBeenCalledWith({
tracerProvider: _tracerProvider,
instrumentations: [
expect.any(HttpInstrumentation),
expect.any(DnsInstrumentation),
expect.any(PinoInstrumentation),
],
instrumentations: [expect.any(HttpInstrumentation), expect.any(PinoInstrumentation)],
});
expect(_tracerProvider.register).toHaveBeenCalledTimes(1);
});

it('should ignore DNS requests to itself', () => {
setup({});
expect(DnsInstrumentation.mock.calls[0][0]).toMatchInlineSnapshot(`
{
"ignoreHostnames": [
"0.0.0.0",
"localhost",
],
}
`);
});

it('should not ignore DNS requests to itself when tracing all requests', () => {
setup({ traceAllRequests: true });
expect(DnsInstrumentation.mock.calls[0][0]).toMatchInlineSnapshot(`
{
"ignoreHostnames": undefined,
}
`);
});

it('should not trace outgoing requests that do not have a parent span', () => {
setup({});
expect(HttpInstrumentation.mock.calls[0][0].requireParentforOutgoingSpans).toBe(true);
Expand Down Expand Up @@ -210,6 +184,36 @@ describe('tracer', () => {
`);
});

it('should update span names for incoming and outgoing HTTP requests', () => {
setup({});
const startSpan = (spanName, { attributes }) => {
const span = { name: spanName, attributes };
span.updateName = (updatedName) => {
span.name = updatedName;
};
return span;
};
const { requestHook } = HttpInstrumentation.mock.calls[0][0];
const incomingSpan = startSpan('mock-incoming-span', {
attributes: {
direction: 'in',
'http.method': 'GET',
'http.target': '/mock-route',
},
});
requestHook(incomingSpan);
expect(incomingSpan.name).toMatchInlineSnapshot('"GET /mock-route"');
const outgoingSpan = startSpan('mock-outgoing-span', {
attributes: {
direction: 'out',
'http.method': 'POST',
'http.url': 'http://localhost/mock-route',
},
});
requestHook(outgoingSpan);
expect(outgoingSpan.name).toMatchInlineSnapshot('"POST http://localhost/mock-route"');
});

it('should register a batch span processor with OTLP exporter when traces endpoint is set', () => {
const { _tracerProvider } = setup({ tracesEndpoint: 'http://localhost:4317' });
expect(BatchSpanProcessor).toHaveBeenCalledTimes(1);
Expand Down
17 changes: 0 additions & 17 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@
"@opentelemetry/api": "^1.7.0",
"@opentelemetry/exporter-trace-otlp-grpc": "^0.48.0",
"@opentelemetry/instrumentation": "^0.48.0",
"@opentelemetry/instrumentation-dns": "^0.33.0",
"@opentelemetry/instrumentation-http": "^0.48.0",
"@opentelemetry/instrumentation-pino": "^0.35.0",
"@opentelemetry/sdk-trace-base": "^1.21.0",
Expand Down
12 changes: 8 additions & 4 deletions src/server/utils/tracer.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import { registerInstrumentations } from '@opentelemetry/instrumentation';
import { HttpInstrumentation } from '@opentelemetry/instrumentation-http';
import { PinoInstrumentation } from '@opentelemetry/instrumentation-pino';
import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node';
import { DnsInstrumentation } from '@opentelemetry/instrumentation-dns';
import {
BatchSpanProcessor,
SimpleSpanProcessor,
Expand Down Expand Up @@ -48,12 +47,17 @@ registerInstrumentations({
? (request) => request.url.startsWith('/_/') || request.headers.host.endsWith(`:${devCdnPort}`)
: undefined,
requireParentforOutgoingSpans: !traceAllRequests,
requestHook(span) {
if (span?.attributes?.direction === 'in') {
span.updateName(`${span.attributes['http.method']} ${span.attributes['http.target']}`);
}
if (span?.attributes?.direction === 'out') {
span.updateName(`${span.attributes['http.method']} ${span.attributes['http.url']}`);
}
},
startIncomingSpanHook: () => ({ direction: 'in' }),
startOutgoingSpanHook: () => ({ direction: 'out' }),
}),
new DnsInstrumentation({
ignoreHostnames: !traceAllRequests ? ['0.0.0.0', 'localhost'] : undefined,
}),
new PinoInstrumentation(),
],
});
Expand Down

0 comments on commit d4bdbce

Please sign in to comment.