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: HTTP instrumentation: add the option to capture headers as span attributes #2492

Merged
merged 9 commits into from
Sep 27, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,16 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
/** keep track on spans not ended */
private readonly _spanNotEnded: WeakSet<Span> = new WeakSet<Span>();
private readonly _version = process.versions.node;
private _headerCapture;

constructor(config: HttpInstrumentationConfig & InstrumentationConfig = {}) {
super(
'@opentelemetry/instrumentation-http',
VERSION,
Object.assign({}, config)
);

this._headerCapture = this._createHeaderCapture();
}

private _getConfig(): HttpInstrumentationConfig {
Expand All @@ -73,6 +76,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {

override setConfig(config: HttpInstrumentationConfig & InstrumentationConfig = {}): void {
this._config = Object.assign({}, config);
this._headerCapture = this._createHeaderCapture();
}

init(): [InstrumentationNodeModuleDefinition<Https>, InstrumentationNodeModuleDefinition<Http>] {
Expand Down Expand Up @@ -279,6 +283,8 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
this._callRequestHook(span, request);
}

this._headerCapture.client.captureRequestHeaders(span, header => request.getHeader(header));

/*
* User 'response' event listeners can be added before our listener,
* force our listener to be the first, so response emitter is bound
Expand All @@ -296,6 +302,8 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
this._callResponseHook(span, response);
}

this._headerCapture.client.captureResponseHeaders(span, header => response.headers[header]);

context.bind(context.active(), response);
this._diag.debug('outgoingRequest on response()');
response.on('end', () => {
Expand Down Expand Up @@ -424,6 +432,8 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
instrumentation._callResponseHook(span, response);
}

instrumentation._headerCapture.server.captureRequestHeaders(span, header => request.headers[header]);

// Wraps end (inspired by:
// https://github.com/GoogleCloudPlatform/cloud-trace-nodejs/blob/master/src/instrumentations/instrumentation-connect.ts#L75)
const originalEnd = response.end;
Expand All @@ -449,6 +459,8 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
response
);

instrumentation._headerCapture.server.captureResponseHeaders(span, header => response.getHeader(header));

span
.setAttributes(attributes)
.setStatus(utils.parseResponseStatus(response.statusCode));
Expand Down Expand Up @@ -662,4 +674,19 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
);
}
}

private _createHeaderCapture() {
const config = this._getConfig();

return {
client: {
captureRequestHeaders: utils.headerCapture('request', config.headersToSpanAttributes?.client?.requestHeaders ?? []),
captureResponseHeaders: utils.headerCapture('response', config.headersToSpanAttributes?.client?.responseHeaders ?? [])
},
server: {
captureRequestHeaders: utils.headerCapture('request', config.headersToSpanAttributes?.server?.requestHeaders ?? []),
captureResponseHeaders: utils.headerCapture('response', config.headersToSpanAttributes?.server?.responseHeaders ?? []),
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,11 @@ export interface HttpInstrumentationConfig extends InstrumentationConfig {
requireParentforOutgoingSpans?: boolean;
/** Require parent to create span for incoming requests */
requireParentforIncomingSpans?: boolean;
/** Map the following HTTP headers to span attributes. */
headersToSpanAttributes?: {
client?: { requestHeaders?: string[]; responseHeaders?: string[]; },
server?: { requestHeaders?: string[]; responseHeaders?: string[]; },
}
}

export interface Err extends Error {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -495,3 +495,27 @@ export const getIncomingRequestAttributesOnResponse = (
}
return attributes;
};

export function headerCapture(type: 'request' | 'response', headers: string[]) {
const normalizedHeaders = new Map(headers.map(header => [header.toLowerCase(), header.toLowerCase().replace(/-/g, '_')]));

return (span: Span, getHeader: (key: string) => undefined | string | string[] | number) => {
for (const [capturedHeader, normalizedHeader] of normalizedHeaders) {
const value = getHeader(capturedHeader);

if (value === undefined) {
continue;
}

const key = `http.${type}.header.${normalizedHeader}`;

if (typeof value === 'string') {
rauno56 marked this conversation as resolved.
Show resolved Hide resolved
span.setAttribute(key, [value]);
} else if (Array.isArray(value)) {
span.setAttribute(key, value);
} else {
span.setAttribute(key, [value]);
}
}
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -908,4 +908,87 @@ describe('HttpInstrumentation', () => {
});
});
});

describe('capturing headers as span attributes', () => {
beforeEach(() => {
memoryExporter.reset();
});

before(() => {
instrumentation.setConfig({
headersToSpanAttributes: {
client: { requestHeaders: ['X-Client-Header1'], responseHeaders: ['X-Server-Header1'] },
server: { requestHeaders: ['X-Client-Header2'], responseHeaders: ['X-Server-Header2'] },
}
});
instrumentation.enable();
server = http.createServer((request, response) => {
response.setHeader('X-Server-Header1', 'server123');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
response.setHeader('X-Server-Header1', 'server123');
response.setHeader('X-ServEr-hEeader1', 'server123');

or similar to verify that casing doesn't matter

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, added it!

response.setHeader('X-Server-Header2', '123server');
response.end('Test Server Response');
});

server.listen(serverPort);
});

after(() => {
server.close();
instrumentation.disable();
});

it('should convert headers to span attributes', async () => {
await httpRequest.get(
`${protocol}://${hostname}:${serverPort}${pathname}`,
{
headers: {
'X-Client-Header1': 'client123',
'X-Client-Header2': '123client',
seemk marked this conversation as resolved.
Show resolved Hide resolved
}
}
);
const spans = memoryExporter.getFinishedSpans();
const [incomingSpan, outgoingSpan] = spans;

assert.strictEqual(spans.length, 2);

assert.deepStrictEqual(
incomingSpan.attributes['http.request.header.x_client_header2'],
['123client']
);

assert.deepStrictEqual(
incomingSpan.attributes['http.response.header.x_server_header2'],
['123server']
);

assert.strictEqual(
incomingSpan.attributes['http.request.header.x_client_header1'],
undefined
);

assert.strictEqual(
incomingSpan.attributes['http.response.header.x_server_header1'],
undefined
);

assert.deepStrictEqual(
outgoingSpan.attributes['http.request.header.x_client_header1'],
['client123']
);
assert.deepStrictEqual(
outgoingSpan.attributes['http.response.header.x_server_header1'],
['server123']
);

assert.strictEqual(
outgoingSpan.attributes['http.request.header.x_client_header2'],
undefined
);

assert.strictEqual(
outgoingSpan.attributes['http.response.header.x_server_header2'],
undefined
);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -465,4 +465,65 @@ describe('Utility', () => {
verifyValueInAttributes(attributes, undefined, 1200);
});
});

describe('headers to span attributes capture', () => {
let span: Span;

beforeEach(() => {
span = new Span(
new BasicTracerProvider().getTracer('default'),
ROOT_CONTEXT,
'test',
{ spanId: '', traceId: '', traceFlags: TraceFlags.SAMPLED },
SpanKind.INTERNAL
);
});

it('should set attributes for request and response keys', () => {
dyladan marked this conversation as resolved.
Show resolved Hide resolved
utils.headerCapture('request', ['Origin'])(span, () => 'localhost');
utils.headerCapture('response', ['Cookie'])(span, () => 'token=123');
assert.deepStrictEqual(span.attributes['http.request.header.origin'], ['localhost']);
assert.deepStrictEqual(span.attributes['http.response.header.cookie'], ['token=123']);
});

it('should set attributes for multiple values', () => {
utils.headerCapture('request', ['Origin'])(span, () => ['localhost', 'www.example.com']);
assert.deepStrictEqual(span.attributes['http.request.header.origin'], ['localhost', 'www.example.com']);
});

it('sets attributes for multiple headers', () => {
utils.headerCapture('request', ['Origin', 'Foo'])(span, header => {
if (header === 'origin') {
return 'localhost';
}

if (header === 'foo') {
return 42;
}

return undefined;
});

assert.deepStrictEqual(span.attributes['http.request.header.origin'], ['localhost']);
assert.deepStrictEqual(span.attributes['http.request.header.foo'], [42]);
});

it('should normalize header names', () => {
utils.headerCapture('request', ['X-Forwarded-For'])(span, () => 'foo');
assert.deepStrictEqual(span.attributes['http.request.header.x_forwarded_for'], ['foo']);
});

it('ignores non-existent headers', () => {
utils.headerCapture('request', ['Origin', 'Accept'])(span, header => {
if (header === 'origin') {
return 'localhost';
}

return undefined;
});

assert.deepStrictEqual(span.attributes['http.request.header.origin'], ['localhost']);
assert.deepStrictEqual(span.attributes['http.request.header.accept'], undefined);
})
});
});