From 07f0e25eb5aa261d67a71e8c7bd8565e14610212 Mon Sep 17 00:00:00 2001 From: Santosh Kumar Cheler Date: Thu, 4 Jul 2024 18:58:53 -0700 Subject: [PATCH] Enable response hook to add attributes to the span --- examples/web/examples/fetch/index.js | 4 + .../src/fetch.ts | 185 +++++++++--------- .../src/types.ts | 34 ++++ 3 files changed, 129 insertions(+), 94 deletions(-) diff --git a/examples/web/examples/fetch/index.js b/examples/web/examples/fetch/index.js index 75485ace53..a849b8f844 100644 --- a/examples/web/examples/fetch/index.js +++ b/examples/web/examples/fetch/index.js @@ -40,6 +40,10 @@ registerInstrumentations({ propagateTraceHeaderCorsUrls: [ 'http://localhost:8090', ], + responseHook: (span, request, response) => { + span.setAttribute('responseHook.method', request.method); + span.setAttribute('responseHook.status_code', response.status); + }, }), ], tracerProvider: tracerProvider, diff --git a/plugins/web/opentelemetry-instrumentation-fetch2/src/fetch.ts b/plugins/web/opentelemetry-instrumentation-fetch2/src/fetch.ts index a5f4f674a0..f63413c18c 100644 --- a/plugins/web/opentelemetry-instrumentation-fetch2/src/fetch.ts +++ b/plugins/web/opentelemetry-instrumentation-fetch2/src/fetch.ts @@ -18,14 +18,13 @@ import * as api from '@opentelemetry/api'; import { isWrapped, InstrumentationBase, - InstrumentationConfig, - safeExecuteInTheMiddle, + safeExecuteInTheMiddle } from '@opentelemetry/instrumentation'; import * as core from '@opentelemetry/core'; import * as web from '@opentelemetry/sdk-trace-web'; import { AttributeNames } from './enums/AttributeNames'; import { SEMATTRS_HTTP_URL, SEMATTRS_HTTP_HOST, SEMATTRS_HTTP_METHOD, SEMATTRS_HTTP_SCHEME, SEMATTRS_HTTP_STATUS_CODE, SEMATTRS_HTTP_USER_AGENT } from '@opentelemetry/semantic-conventions'; -import { FetchError, FetchResponse, SpanContextData } from './types'; +import { FetchError, FetchInstrumentationConfig, FetchResponse, SpanContextData } from './types'; import { PACKAGE_NAME, PACKAGE_VERSION } from './version'; import { _globalThis } from '@opentelemetry/core'; @@ -33,29 +32,7 @@ const isNode = typeof process === 'object' && process.release?.name === 'node'; const RESOURCE_FETCH_INITIATED = '@opentelemetry/ResourceFetchInitiated'; // TODO: duplicated in resource-timing instrumentation -export interface FetchCustomAttributeFunction { - ( - span: api.Span, - request: Request | RequestInit, - result: Response | FetchError - ): void; -} -/** - * FetchPlugin Config - */ -export interface FetchInstrumentationConfig extends InstrumentationConfig { - // urls which should include trace headers when origin doesn't match - propagateTraceHeaderCorsUrls?: web.PropagateTraceHeaderCorsUrls; - /** - * URLs that partially match any regex in ignoreUrls will not be traced. - * In addition, URLs that are _exact matches_ of strings in ignoreUrls will - * also not be traced. - */ - ignoreUrls?: Array; - /** Function for adding custom attributes on the span */ - applyCustomAttributesOnSpan?: FetchCustomAttributeFunction; -} /** * This class represents a fetch plugin for auto instrumentation; @@ -103,8 +80,7 @@ export class FetchInstrumentation extends InstrumentationBase { }, }); } - - /** + /** * Finish span, add attributes. * @param span * @param endTime @@ -115,14 +91,45 @@ export class FetchInstrumentation extends InstrumentationBase { response: FetchResponse, spanContextData: SpanContextData, endTime: api.HrTime - ) { + ): Promise { + + return new Promise((resolve, reject) => { + try { + spanContextData.endTime = endTime; + document.dispatchEvent(new CustomEvent(RESOURCE_FETCH_INITIATED, { + detail: spanContextData + })); + this._addFinalSpanAttributes(span, response); + span.end(endTime); + } finally { + resolve(); + } + }); + } - spanContextData.endTime = endTime; - document.dispatchEvent(new CustomEvent(RESOURCE_FETCH_INITIATED, { - detail: spanContextData - })); - this._addFinalSpanAttributes(span, response); - span.end(endTime); + private _executeResponseHook( + span: api.Span, + request: Request | RequestInit, + result: Response | FetchError + ) : Promise { + if (!this._config.responseHook) { + return Promise.resolve(); + } + return new Promise((resolve) => { + safeExecuteInTheMiddle( + () => { + this._config.responseHook?.(span, request, result); + }, + err => { + if (err) { + this._diag.error('Error running response hook', err); + } + }, + true + ); + resolve(); + + }); } /** @@ -211,6 +218,7 @@ export class FetchInstrumentation extends InstrumentationBase { const startTime = core.hrTime(); const createdSpan = plugin._createSpan(url, startTime, options); if (!createdSpan) { + // url was ignored and no span was created, hence no need to wrap fetch return original.apply(this, args); } @@ -223,81 +231,92 @@ export class FetchInstrumentation extends InstrumentationBase { spanId: createdSpan.spanContext().spanId } - - - function endSpanOnError(span: api.Span, endTime: api.HrTime, error: FetchError) { - plugin._applyAttributesAfterFetch(span, options, error); - plugin._endSpan(span, { - status: error.status || 0, - statusText: error.message, - url, - }, spanContextData, endTime); + function endSpanOnError(span: api.Span, endTime: api.HrTime, error: FetchError): Promise { + return plugin._executeResponseHook(span, options, error) + .then(() => { + plugin._endSpan(span, { + status: error.status || 0, + statusText: error.message, + url, + }, spanContextData, endTime); + }); } - function endSpanOnSuccess(span: api.Span, endTime: api.HrTime, response: Response) { - plugin._applyAttributesAfterFetch(span, options, response); - if (response.status >= 200 && response.status < 400) { - plugin._endSpan(span, response, spanContextData, endTime); - } else { - plugin._endSpan(span, { - status: response.status, - statusText: response.statusText, - url, - }, spanContextData, endTime); - } + function endSpanOnSuccess(span: api.Span, endTime: api.HrTime, response: Response): Promise { + return plugin._executeResponseHook(span, options, response) + .then(() => { + if (response.status >= 200 && response.status < 400) { + plugin._endSpan(span, response, spanContextData, endTime); + } else { + plugin._endSpan(span, { + status: response.status, + statusText: response.statusText, + url, + }, spanContextData, endTime); + } + }) } function onSuccess( span: api.Span, resolve: (value: Response | PromiseLike) => void, response: Response - ): void { - // For client spans, the span should end at the earliest when the response is received - // and not wait for the body to be read. However, since we need attributes that are based on the - // response, we will proceed to read the body and end the span using a endTime that is now. - const endTime = core.millisToHrTime(Date.now()); + ): Promise { + return new Promise(() => { + // For client spans, the span should end at the earliest when the response is received + // and not wait for the body to be read. However, since we need attributes that are based on the + // response, we will proceed to read the body and end the span using a endTime that is now. + const endTime = core.millisToHrTime(Date.now()); - try { const resClone = response.clone(); const resClone4Hook = response.clone(); const body = resClone.body; if (body) { const reader = body.getReader(); const read = (): void => { + // FIXME: Find out the entire body is ready is read; it will buffer the entire body in memory, + // which might not be desirable for large responses. reader.read().then( ({ done }) => { if (done) { - endSpanOnSuccess(span, endTime, resClone4Hook); + endSpanOnSuccess(span, endTime, resClone4Hook) + .finally(() => { + resolve(response); + }); } else { read(); } }, error => { - endSpanOnError(span, endTime, error); + endSpanOnError(span, endTime, error) + .finally(() => { + resolve(response); + }); } ); }; read(); } else { // some older browsers don't have .body implemented - endSpanOnSuccess(span, endTime, response); + endSpanOnSuccess(span, endTime, response) + .finally(() => { + resolve(response); + }); } - } finally { - resolve(response); - } + }) } + function onError( span: api.Span, reject: (reason?: unknown) => void, error: FetchError - ) { + ): Promise { const endTime = core.millisToHrTime(Date.now()); - try { - endSpanOnError(span, endTime, error); - } finally { - reject(error); - } + return endSpanOnError(span, endTime, error) + .finally(() => { + reject(error); + }); } return new Promise((resolve, reject) => { @@ -323,28 +342,6 @@ export class FetchInstrumentation extends InstrumentationBase { }; } - private _applyAttributesAfterFetch( - span: api.Span, - request: Request | RequestInit, - result: Response | FetchError - ) { - const applyCustomAttributesOnSpan = - this._config.applyCustomAttributesOnSpan; - if (applyCustomAttributesOnSpan) { - safeExecuteInTheMiddle( - () => applyCustomAttributesOnSpan(span, request, result), - error => { - if (!error) { - return; - } - - this._diag.error('applyCustomAttributesOnSpan', error); - }, - true - ); - } - } - /** * implements enable function */ diff --git a/plugins/web/opentelemetry-instrumentation-fetch2/src/types.ts b/plugins/web/opentelemetry-instrumentation-fetch2/src/types.ts index 9d194b48e0..4c4c335355 100644 --- a/plugins/web/opentelemetry-instrumentation-fetch2/src/types.ts +++ b/plugins/web/opentelemetry-instrumentation-fetch2/src/types.ts @@ -15,7 +15,41 @@ */ import * as api from '@opentelemetry/api'; +import { InstrumentationConfig } from '@opentelemetry/instrumentation'; +import * as web from '@opentelemetry/sdk-trace-web'; +/** + * Hook function to be called after fetch is executed and before ending the span. + * FIXME: The hook function doesn't have access to the fetch url if the request + * is a RequestInit object + */ +export interface FetchInstrumentationExecutionResponseHook { + ( + span: api.Span, + request: Request | RequestInit, + result: Response | FetchError + ): void; +} + +/** + * FetchPlugin Config + */ +export interface FetchInstrumentationConfig extends InstrumentationConfig { + // urls which should include trace headers when origin doesn't match + propagateTraceHeaderCorsUrls?: web.PropagateTraceHeaderCorsUrls; + /** + * URLs that partially match any regex in ignoreUrls will not be traced. + * In addition, URLs that are _exact matches_ of strings in ignoreUrls will + * also not be traced. + */ + ignoreUrls?: Array; + + /** Hook function to be called after fetch is executed and before ending the span. + * This is useful for adding custom attributes to the span based on the fetch's + * request and response + */ + responseHook?: FetchInstrumentationExecutionResponseHook; +} /** * Interface used to provide information to finish span on fetch response */