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

Adding ComponentLogger into instrumentations #2261

Merged
merged 6 commits into from
Jun 9, 2021
Merged
Show file tree
Hide file tree
Changes from 5 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
8 changes: 4 additions & 4 deletions packages/opentelemetry-instrumentation-fetch/src/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ export class FetchInstrumentation extends InstrumentationBase<
const headers: Partial<Record<string, unknown>> = {};
api.propagation.inject(api.context.active(), headers);
if (Object.keys(headers).length > 0) {
api.diag.debug('headers inject skipped due to CORS policy');
this._diag.debug('headers inject skipped due to CORS policy');
}
return;
}
Expand Down Expand Up @@ -198,7 +198,7 @@ export class FetchInstrumentation extends InstrumentationBase<
options: Partial<Request | RequestInit> = {}
): api.Span | undefined {
if (core.isUrlIgnored(url, this._getConfig().ignoreUrls)) {
api.diag.debug('ignoring span as url matches ignored url');
this._diag.debug('ignoring span as url matches ignored url');
return;
}
const method = (options.method || 'GET').toUpperCase();
Expand Down Expand Up @@ -417,7 +417,7 @@ export class FetchInstrumentation extends InstrumentationBase<
return;
}

api.diag.error('applyCustomAttributesOnSpan', error);
this._diag.error('applyCustomAttributesOnSpan', error);
},
true
);
Expand Down Expand Up @@ -461,7 +461,7 @@ export class FetchInstrumentation extends InstrumentationBase<
override enable() {
if (isWrapped(window.fetch)) {
this._unwrap(window, 'fetch');
api.diag.debug('removing previous patch for constructor');
this._diag.debug('removing previous patch for constructor');
}
this._wrap(window, 'fetch', this._patchConstructor());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ describe('fetch', () => {
});
it('should debug info that injecting headers was skipped', () => {
assert.strictEqual(
spyDebug.lastCall.args[0],
spyDebug.lastCall.args[1],
'headers inject skipped due to CORS policy'
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import {
} from './types';
import {
context,
diag,
propagation,
ROOT_CONTEXT,
SpanOptions,
Expand Down Expand Up @@ -76,7 +75,7 @@ export class GrpcJsInstrumentation extends InstrumentationBase {
'@grpc/grpc-js',
['1.*'],
(moduleExports, version) => {
diag.debug(`Applying patch for @grpc/grpc-js@${version}`);
this._diag.debug(`Applying patch for @grpc/grpc-js@${version}`);
if (isWrapped(moduleExports.Server.prototype.register)) {
this._unwrap(moduleExports.Server.prototype, 'register');
}
Expand Down Expand Up @@ -115,7 +114,7 @@ export class GrpcJsInstrumentation extends InstrumentationBase {
},
(moduleExports, version) => {
if (moduleExports === undefined) return;
diag.debug(`Removing patch for @grpc/grpc-js@${version}`);
this._diag.debug(`Removing patch for @grpc/grpc-js@${version}`);

this._unwrap(moduleExports.Server.prototype, 'register');
this._unwrap(moduleExports, 'makeClientConstructor');
Expand All @@ -136,7 +135,7 @@ export class GrpcJsInstrumentation extends InstrumentationBase {
const instrumentation = this;
return (originalRegister: ServerRegisterFunction) => {
const config = this._config;
diag.debug('patched gRPC server');
instrumentation._diag.debug('patched gRPC server');
return function register<RequestType, ResponseType>(
this: grpcJs.Server,
name: string,
Expand Down Expand Up @@ -186,7 +185,7 @@ export class GrpcJsInstrumentation extends InstrumentationBase {
kind: SpanKind.SERVER,
};

diag.debug('patch func: %s', JSON.stringify(spanOptions));
instrumentation._diag.debug('patch func: %s', JSON.stringify(spanOptions));

context.with(
propagation.extract(ROOT_CONTEXT, call.metadata, {
Expand Down Expand Up @@ -231,7 +230,7 @@ export class GrpcJsInstrumentation extends InstrumentationBase {
) => MakeClientConstructorFunction {
const instrumentation = this;
return (original: MakeClientConstructorFunction) => {
diag.debug('patching client');
instrumentation._diag.debug('patching client');
return function makeClientConstructor(
this: typeof grpcJs.Client,
methods: grpcJs.ServiceDefinition,
Expand All @@ -255,7 +254,7 @@ export class GrpcJsInstrumentation extends InstrumentationBase {
*/
private _patchLoadPackageDefinition(grpcClient: typeof grpcJs) {
const instrumentation = this;
diag.debug('patching loadPackageDefinition');
instrumentation._diag.debug('patching loadPackageDefinition');
return (original: typeof grpcJs.loadPackageDefinition) => {
return function patchedLoadPackageDefinition(
this: null,
Expand All @@ -279,7 +278,7 @@ export class GrpcJsInstrumentation extends InstrumentationBase {
): (original: GrpcClientFunc) => () => EventEmitter {
const instrumentation = this;
return (original: GrpcClientFunc) => {
diag.debug('patch all client methods');
instrumentation._diag.debug('patch all client methods');
return function clientMethodTrace(this: grpcJs.Client) {
const name = `grpc.${original.path.replace('/', '')}`;
const args = [...arguments];
Expand Down
17 changes: 8 additions & 9 deletions packages/opentelemetry-instrumentation-grpc/src/grpc/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import {
import { GrpcInstrumentationConfig } from '../types';
import {
context,
diag,
propagation,
SpanOptions,
SpanKind,
Expand Down Expand Up @@ -75,7 +74,7 @@ export class GrpcNativeInstrumentation extends InstrumentationBase<
'grpc',
['1.*'],
(moduleExports, version) => {
diag.debug(`Applying patch for grpc@${version}`);
this._diag.debug(`Applying patch for grpc@${version}`);
grpcClient = moduleExports;

if (isWrapped(moduleExports.Server.prototype.register)) {
Expand All @@ -99,7 +98,7 @@ export class GrpcNativeInstrumentation extends InstrumentationBase<
},
(moduleExports, version) => {
if (moduleExports === undefined) return;
diag.debug(`Removing patch for grpc@${version}`);
this._diag.debug(`Removing patch for grpc@${version}`);

this._unwrap(moduleExports.Server.prototype, 'register');
},
Expand All @@ -113,7 +112,7 @@ export class GrpcNativeInstrumentation extends InstrumentationBase<
moduleExports: GrpcInternalClientTypes,
version?: string
) => {
diag.debug(`Applying internal patch for grpc@${version}`);
this._diag.debug(`Applying internal patch for grpc@${version}`);
if (isWrapped(moduleExports.makeClientConstructor)) {
this._unwrap(moduleExports, 'makeClientConstructor');
}
Expand All @@ -125,7 +124,7 @@ export class GrpcNativeInstrumentation extends InstrumentationBase<
version?: string
) => {
if (moduleExports === undefined) return;
diag.debug(`Removing internal patch for grpc@${version}`);
this._diag.debug(`Removing internal patch for grpc@${version}`);
this._unwrap(moduleExports, 'makeClientConstructor');
};
return [
Expand All @@ -147,7 +146,7 @@ export class GrpcNativeInstrumentation extends InstrumentationBase<
private _patchServer(grpcModule: typeof grpcTypes) {
const instrumentation = this;
return (originalRegister: typeof grpcTypes.Server.prototype.register) => {
diag.debug('patched gRPC server');
instrumentation._diag.debug('patched gRPC server');

return function register<RequestType, ResponseType>(
this: grpcTypes.Server & { handlers: any },
Expand Down Expand Up @@ -191,7 +190,7 @@ export class GrpcNativeInstrumentation extends InstrumentationBase<
kind: SpanKind.SERVER,
};

diag.debug('patch func: %s', JSON.stringify(spanOptions));
instrumentation._diag.debug('patch func: %s', JSON.stringify(spanOptions));

context.with(
propagation.extract(context.active(), call.metadata, {
Expand Down Expand Up @@ -243,7 +242,7 @@ export class GrpcNativeInstrumentation extends InstrumentationBase<
private _patchClient() {
const instrumentation = this;
return (original: typeof grpcTypes.makeGenericClientConstructor): never => {
diag.debug('patching client');
instrumentation._diag.debug('patching client');
return function makeClientConstructor(
this: typeof grpcTypes.Client,
methods: { [key: string]: { originalName?: string } },
Expand Down Expand Up @@ -288,7 +287,7 @@ export class GrpcNativeInstrumentation extends InstrumentationBase<
private _getPatchedClientMethods() {
const instrumentation = this;
return (original: GrpcClientFunc) => {
diag.debug('patch all client methods');
instrumentation._diag.debug('patch all client methods');
return function clientMethodTrace(this: grpcTypes.Client) {
const name = `grpc.${(original.path as string | undefined)?.replace(
'/',
Expand Down
23 changes: 11 additions & 12 deletions packages/opentelemetry-instrumentation-http/src/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/
import {
context,
diag,
INVALID_SPAN_CONTEXT,
propagation,
ROOT_CONTEXT,
Expand Down Expand Up @@ -86,7 +85,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
'http',
['*'],
moduleExports => {
diag.debug(`Applying patch for http@${this._version}`);
this._diag.debug(`Applying patch for http@${this._version}`);
if (isWrapped(moduleExports.request)) {
this._unwrap(moduleExports, 'request');
}
Expand Down Expand Up @@ -115,7 +114,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
},
moduleExports => {
if (moduleExports === undefined) return;
diag.debug(`Removing patch for http@${this._version}`);
this._diag.debug(`Removing patch for http@${this._version}`);

this._unwrap(moduleExports, 'request');
this._unwrap(moduleExports, 'get');
Expand All @@ -129,7 +128,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
'https',
['*'],
moduleExports => {
diag.debug(`Applying patch for https@${this._version}`);
this._diag.debug(`Applying patch for https@${this._version}`);
if (isWrapped(moduleExports.request)) {
this._unwrap(moduleExports, 'request');
}
Expand Down Expand Up @@ -158,7 +157,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
},
moduleExports => {
if (moduleExports === undefined) return;
diag.debug(`Removing patch for https@${this._version}`);
this._diag.debug(`Removing patch for https@${this._version}`);

this._unwrap(moduleExports, 'request');
this._unwrap(moduleExports, 'get');
Expand Down Expand Up @@ -309,9 +308,9 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
}

context.bind(context.active(), response);
diag.debug('outgoingRequest on response()');
this._diag.debug('outgoingRequest on response()');
response.on('end', () => {
diag.debug('outgoingRequest on end()');
this._diag.debug('outgoingRequest on end()');
let status: SpanStatus;

if (response.aborted && !response.complete) {
Expand Down Expand Up @@ -353,7 +352,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
this._closeHttpSpan(span);
});

diag.debug('http.ClientRequest return request');
this._diag.debug('http.ClientRequest return request');
return request;
}

Expand All @@ -379,13 +378,13 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
: '/';
const method = request.method || 'GET';

diag.debug('%s instrumentation incomingRequest', component);
instrumentation._diag.debug('%s instrumentation incomingRequest', component);

if (
utils.isIgnored(
pathname,
instrumentation._getConfig().ignoreIncomingPaths,
(e: Error) => diag.error('caught ignoreIncomingPaths error: ', e)
(e: Error) => instrumentation._diag.error('caught ignoreIncomingPaths error: ', e)
)
) {
return context.with(suppressTracing(context.active()), () => {
Expand Down Expand Up @@ -529,7 +528,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
utils.isIgnored(
origin + pathname,
instrumentation._getConfig().ignoreOutgoingUrls,
(e: Error) => diag.error('caught ignoreOutgoingUrls error: ', e)
(e: Error) => instrumentation._diag.error('caught ignoreOutgoingUrls error: ', e)
)
) {
return original.apply(this, [optionsParsed, ...args]);
Expand Down Expand Up @@ -570,7 +569,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
}
);

diag.debug('%s instrumentation outgoingRequest', component);
instrumentation._diag.debug('%s instrumentation outgoingRequest', component);
context.bind(parentContext, request);
return instrumentation._traceClientRequest(
component,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase<XMLHttpRe
const headers: Partial<Record<string, unknown>> = {};
api.propagation.inject(api.context.active(), headers);
if (Object.keys(headers).length > 0) {
api.diag.debug('headers inject skipped due to CORS policy');
this._diag.debug('headers inject skipped due to CORS policy');
}
return;
}
Expand Down Expand Up @@ -190,7 +190,7 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase<XMLHttpRe
return;
}

api.diag.error('applyCustomAttributesOnSpan', error);
this._diag.error('applyCustomAttributesOnSpan', error);
},
true
);
Expand Down Expand Up @@ -327,7 +327,7 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase<XMLHttpRe
method: string
): api.Span | undefined {
if (isUrlIgnored(url, this._getConfig().ignoreUrls)) {
api.diag.debug('ignoring span as url matches ignored url');
this._diag.debug('ignoring span as url matches ignored url');
return;
}
const spanName = `HTTP ${method.toUpperCase()}`;
Expand Down Expand Up @@ -511,16 +511,16 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase<XMLHttpRe
* implements enable function
*/
override enable() {
api.diag.debug('applying patch to', this.moduleName, this.version);
this._diag.debug('applying patch to', this.moduleName, this.version);

if (isWrapped(XMLHttpRequest.prototype.open)) {
this._unwrap(XMLHttpRequest.prototype, 'open');
api.diag.debug('removing previous patch from method open');
this._diag.debug('removing previous patch from method open');
}

if (isWrapped(XMLHttpRequest.prototype.send)) {
this._unwrap(XMLHttpRequest.prototype, 'send');
api.diag.debug('removing previous patch from method send');
this._diag.debug('removing previous patch from method send');
}

this._wrap(XMLHttpRequest.prototype, 'open', this._patchOpen());
Expand All @@ -531,7 +531,7 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase<XMLHttpRe
* implements disable function
*/
override disable() {
api.diag.debug('removing patch from', this.moduleName, this.version);
this._diag.debug('removing patch from', this.moduleName, this.version);

this._unwrap(XMLHttpRequest.prototype, 'open');
this._unwrap(XMLHttpRequest.prototype, 'send');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ describe('xhr', () => {

it('should debug info that injecting headers was skipped', () => {
assert.strictEqual(
spyDebug.lastCall.args[0],
spyDebug.lastCall.args[1],
'headers inject skipped due to CORS policy'
);
});
Expand Down
13 changes: 12 additions & 1 deletion packages/opentelemetry-instrumentation/src/instrumentation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,13 @@
* limitations under the License.
*/

import { TracerProvider, Tracer, trace } from '@opentelemetry/api';
import {
diag,
DiagLogger,
trace,
Tracer,
TracerProvider,
} from '@opentelemetry/api';
import { Meter, MeterProvider, metrics } from '@opentelemetry/api-metrics';
import * as shimmer from 'shimmer';
import { InstrumentationModuleDefinition } from './platform/node';
Expand All @@ -29,6 +35,7 @@ export abstract class InstrumentationAbstract<T = any>

private _tracer: Tracer;
private _meter: Meter;
protected _diag: DiagLogger;

constructor(
public readonly instrumentationName: string,
Expand All @@ -40,6 +47,10 @@ export abstract class InstrumentationAbstract<T = any>
...config,
};

this._diag = diag.createComponentLogger({
namespace: instrumentationName,
});

this._tracer = trace.getTracer(instrumentationName, instrumentationVersion);

this._meter = metrics.getMeter(instrumentationName, instrumentationVersion);
Expand Down