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.

Sign up for GitHub

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(@opentelemetry-instrumentation-http): support adding custom attributes before a span is started #2332

Merged
merged 3 commits into from
Jul 27, 2021
Merged

feat(@opentelemetry-instrumentation-http): support adding custom attributes before a span is started #2332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1c5221a
feat(@opentelemetry-instrumentation-http): support adding custom attr…
echoontheway Jul 26, 2021
4817b4e
Merge branch 'main' into feature/echo_addStartSpanHook
dyladan Jul 26, 2021
c637254
Merge branch 'main' into feature/echo_addStartSpanHook
dyladan Jul 27, 2021
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
18 changes: 10 additions & 8 deletions packages/opentelemetry-instrumentation-http/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,16 @@ Http instrumentation has few options available to choose from. You can set the f

| Options | Type | Description |
| ------- | ---- | ----------- |
| [`applyCustomAttributesOnSpan`](https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-instrumentation-http/src/types.ts#L79) | `HttpCustomAttributeFunction` | Function for adding custom attributes |
| [`requestHook`](https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-instrumentation-http/src/types.ts#81) | `HttpRequestCustomAttributeFunction` | Function for adding custom attributes before request is handled |
| [`responseHook`](https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-instrumentation-http/src/types.ts#L83) | `HttpResponseCustomAttributeFunction` | Function for adding custom attributes before response is handled |
| [`ignoreIncomingPaths`](https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-instrumentation-http/src/types.ts#L75) | `IgnoreMatcher[]` | Http instrumentation will not trace all incoming requests that match paths |
| [`ignoreOutgoingUrls`](https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-instrumentation-http/src/types.ts#L77) | `IgnoreMatcher[]` | Http instrumentation will not trace all outgoing requests that match urls |
| [`serverName`](https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-instrumentation-http/src/types.ts#L85) | `string` | The primary server name of the matched virtual host. |
| `requireParentforOutgoingSpans` | Boolean | Require that is a parent span to create new span for outgoing requests. |
| `requireParentforIncomingSpans` | Boolean | Require that is a parent span to create new span for incoming requests. |
| [`applyCustomAttributesOnSpan`](https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-instrumentation-http/src/types.ts#L91) | `HttpCustomAttributeFunction` | Function for adding custom attributes |
| [`requestHook`](https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-instrumentation-http/src/types.ts#93) | `HttpRequestCustomAttributeFunction` | Function for adding custom attributes before request is handled |
| [`responseHook`](https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-instrumentation-http/src/types.ts#L95) | `HttpResponseCustomAttributeFunction` | Function for adding custom attributes before response is handled |
| [`startIncomingSpanHook`](https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-instrumentation-http/src/types.ts#L97) | `StartIncomingSpanCustomAttributeFunction` | Function for adding custom attributes before a span is started in incomingRequest |
| [`startOutgoingSpanHook`](https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-instrumentation-http/src/types.ts#L99) | `StartOutgoingSpanCustomAttributeFunction` | Function for adding custom attributes before a span is started in outgoingRequest |
| [`ignoreIncomingPaths`](https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-instrumentation-http/src/types.ts#L87) | `IgnoreMatcher[]` | Http instrumentation will not trace all incoming requests that match paths |
| [`ignoreOutgoingUrls`](https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-instrumentation-http/src/types.ts#L89) | `IgnoreMatcher[]` | Http instrumentation will not trace all outgoing requests that match urls |
| [`serverName`](https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-instrumentation-http/src/types.ts#L101) | `string` | The primary server name of the matched virtual host. |
| [`requireParentforOutgoingSpans`](https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-instrumentation-http/src/types.ts#L103) | Boolean | Require that is a parent span to create new span for outgoing requests. |
| [`requireParentforIncomingSpans`](https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-instrumentation-http/src/types.ts#L105) | Boolean | Require that is a parent span to create new span for incoming requests. |

## Useful links

Expand Down
21 changes: 21 additions & 0 deletions packages/opentelemetry-instrumentation-http/src/http.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,10 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
attributes: utils.getIncomingRequestAttributes(request, {
component: component,
serverName: instrumentation._getConfig().serverName,
hookAttributes: instrumentation._callStartSpanHook(
request,
instrumentation._getConfig().startIncomingSpanHook
),
}),
};

Expand Down Expand Up @@ -532,6 +536,10 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
const attributes = utils.getOutgoingRequestAttributes(optionsParsed, {
component,
hostname,
hookAttributes: instrumentation._callStartSpanHook(
optionsParsed,
instrumentation._getConfig().startOutgoingSpanHook
),
});

const spanOptions: SpanOptions = {
Expand Down Expand Up @@ -638,4 +646,17 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
true
);
}

private _callStartSpanHook(
request: http.IncomingMessage | http.RequestOptions,
hookFunc: Function | undefined,
) {
if(typeof hookFunc === 'function'){
return safeExecuteInTheMiddle(
() => hookFunc(request),
() => { },
true
);
}
}
}
18 changes: 17 additions & 1 deletion packages/opentelemetry-instrumentation-http/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { Span } from '@opentelemetry/api';
import {
Span,
SpanAttributes,
} from '@opentelemetry/api';
import type * as http from 'http';
import type * as https from 'https';
import {
Expand All @@ -22,6 +25,7 @@ import {
IncomingMessage,
request,
ServerResponse,
RequestOptions,
} from 'http';
import * as url from 'url';
import { InstrumentationConfig } from '@opentelemetry/instrumentation';
Expand Down Expand Up @@ -67,6 +71,14 @@ export interface HttpResponseCustomAttributeFunction {
(span: Span, response: IncomingMessage | ServerResponse): void;
}

export interface StartIncomingSpanCustomAttributeFunction {
(request: IncomingMessage ): SpanAttributes;
}

export interface StartOutgoingSpanCustomAttributeFunction {
(request: RequestOptions ): SpanAttributes;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

In the specs meeting it looks like there will be a different way to doing this (if I'm reading and listening correctly)

https://github.com/open-telemetry/oteps/blob/8335c9a53cdf3398075dd9523d3752522c0ece28/text/0165-instrumentation-api.md

Copy link
Member

Choose a reason for hiding this comment

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

I believe i read it the same as you, this is welcome as a lot of instrumentation had some kind of system like this already. I believe this should be handled in the base instrumentation class though. I think it make sense to create a dedicated issue for this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍
Before it's handled in the base instrumentation, will these startHooks for http be accepted? For I wanna use some custom fields to make the sample decision in my project. :)

Copy link
Member

Choose a reason for hiding this comment

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

I think so

/**
* Options available for the HTTP instrumentation (see [documentation](https://github.com/open-telemetry/opentelemetry-js/tree/main/packages/opentelemetry-instrumentation-http#http-instrumentation-options))
*/
Expand All @@ -81,6 +93,10 @@ export interface HttpInstrumentationConfig extends InstrumentationConfig {
requestHook?: HttpRequestCustomAttributeFunction;
/** Function for adding custom attributes before response is handled */
responseHook?: HttpResponseCustomAttributeFunction;
/** Function for adding custom attributes before a span is started in incomingRequest */
startIncomingSpanHook?: StartIncomingSpanCustomAttributeFunction;
/** Function for adding custom attributes before a span is started in outgoingRequest */
startOutgoingSpanHook?: StartOutgoingSpanCustomAttributeFunction;
/** The primary server name of the matched virtual host. */
serverName?: string;
/** Require parent to create span for outgoing requests */
Expand Down
12 changes: 6 additions & 6 deletions packages/opentelemetry-instrumentation-http/src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,11 +334,11 @@ export const isValidOptionsType = (options: unknown): boolean => {
/**
* Returns outgoing request attributes scoped to the options passed to the request
* @param {ParsedRequestOptions} requestOptions the same options used to make the request
* @param {{ component: string, hostname: string }} options used to pass data needed to create attributes
* @param {{ component: string, hostname: string, hookAttributes?: SpanAttributes }} options used to pass data needed to create attributes
*/
export const getOutgoingRequestAttributes = (
requestOptions: ParsedRequestOptions,
options: { component: string; hostname: string }
options: { component: string; hostname: string; hookAttributes?: SpanAttributes }
): SpanAttributes => {
const host = requestOptions.host;
const hostname =
Expand All @@ -363,7 +363,7 @@ export const getOutgoingRequestAttributes = (
if (userAgent !== undefined) {
attributes[SemanticAttributes.HTTP_USER_AGENT] = userAgent;
}
return attributes;
return Object.assign(attributes, options.hookAttributes);
};

/**
Expand Down Expand Up @@ -415,11 +415,11 @@ export const getOutgoingRequestAttributesOnResponse = (
/**
* Returns incoming request attributes scoped to the request data
* @param {IncomingMessage} request the request object
* @param {{ component: string, serverName?: string }} options used to pass data needed to create attributes
* @param {{ component: string, serverName?: string, hookAttributes?: SpanAttributes }} options used to pass data needed to create attributes
*/
export const getIncomingRequestAttributes = (
request: IncomingMessage,
options: { component: string; serverName?: string }
options: { component: string; serverName?: string; hookAttributes?: SpanAttributes }
): SpanAttributes => {
const headers = request.headers;
const userAgent = headers['user-agent'];
Expand Down Expand Up @@ -463,7 +463,7 @@ export const getIncomingRequestAttributes = (
setRequestContentLengthAttribute(request, attributes);

const httpKindAttributes = getAttributesFromHttpKind(httpVersion);
return Object.assign(attributes, httpKindAttributes);
return Object.assign(attributes, httpKindAttributes, options.hookAttributes);
};

/**
Expand Down
31 changes: 28 additions & 3 deletions packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ import {
context,
propagation,
Span as ISpan,
SpanKind, trace,
SpanKind,
trace,
SpanAttributes,
} from '@opentelemetry/api';
import { NodeTracerProvider } from '@opentelemetry/node';
import {
Expand All @@ -39,7 +41,7 @@ import { DummyPropagation } from '../utils/DummyPropagation';
import { httpRequest } from '../utils/httpRequest';
import { ContextManager } from '@opentelemetry/api';
import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks';
import type { ClientRequest, IncomingMessage, ServerResponse } from 'http';
import type { ClientRequest, IncomingMessage, ServerResponse, RequestOptions } from 'http';
import { isWrapped } from '@opentelemetry/instrumentation';
import { getRPCMetadata, RPCType } from '@opentelemetry/core';

Expand Down Expand Up @@ -95,6 +97,18 @@ export const responseHookFunction = (
span.setAttribute('custom response hook attribute', 'response');
};

export const startIncomingSpanHookFunction = (
request: IncomingMessage
): SpanAttributes => {
return {guid: request.headers?.guid}
};

export const startOutgoingSpanHookFunction = (
request: RequestOptions
): SpanAttributes => {
return {guid: request.headers?.guid}
};

describe('HttpInstrumentation', () => {
let contextManager: ContextManager;

Expand Down Expand Up @@ -201,6 +215,8 @@ describe('HttpInstrumentation', () => {
applyCustomAttributesOnSpan: customAttributeFunction,
requestHook: requestHookFunction,
responseHook: responseHookFunction,
startIncomingSpanHook: startIncomingSpanHookFunction,
startOutgoingSpanHook: startOutgoingSpanHookFunction,
serverName,
});
instrumentation.enable();
Expand Down Expand Up @@ -686,7 +702,8 @@ describe('HttpInstrumentation', () => {

it('custom attributes should show up on client and server spans', async () => {
await httpRequest.get(
`${protocol}://${hostname}:${serverPort}${pathname}`
`${protocol}://${hostname}:${serverPort}${pathname}`,
{headers: {guid: 'user_guid'}}
);
const spans = memoryExporter.getFinishedSpans();
const [incomingSpan, outgoingSpan] = spans;
Expand All @@ -699,6 +716,10 @@ describe('HttpInstrumentation', () => {
incomingSpan.attributes['custom response hook attribute'],
'response'
);
assert.strictEqual(
incomingSpan.attributes['guid'],
'user_guid'
);
assert.strictEqual(
incomingSpan.attributes['span kind'],
SpanKind.CLIENT
Expand All @@ -712,6 +733,10 @@ describe('HttpInstrumentation', () => {
outgoingSpan.attributes['custom response hook attribute'],
'response'
);
assert.strictEqual(
outgoingSpan.attributes['guid'],
'user_guid'
);
assert.strictEqual(
outgoingSpan.attributes['span kind'],
SpanKind.CLIENT
Expand Down