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

Allow LoggerProvider to be specified in Instrumentations #4314

Merged
merged 21 commits into from
Feb 6, 2024
Merged
Show file tree
Hide file tree
Changes from 14 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
2 changes: 2 additions & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ All notable changes to experimental packages in this project will be documented

### :rocket: (Enhancement)

* feat(instrumentation): allow LoggerProvider to be specified in Instrumentations [#4314](https://github.com/open-telemetry/opentelemetry-js/pull/4314) @hectorhdzg

### :bug: (Bug Fix)

### :books: (Refine Doc)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,13 @@
"shimmer": "^1.2.1"
},
"peerDependencies": {
"@opentelemetry/api": "^1.3.0"
"@opentelemetry/api": "^1.3.0",
"@opentelemetry/api-logs": "^0.46.0"
},
"devDependencies": {
"@babel/core": "7.23.6",
"@opentelemetry/api": "1.7.0",
"@opentelemetry/api-logs": "0.47.0",
"@opentelemetry/sdk-metrics": "1.20.0",
"@types/mocha": "10.0.6",
"@types/node": "18.6.5",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

import { trace, metrics } from '@opentelemetry/api';
import { logs } from '@opentelemetry/api-logs';
import {
disableInstrumentations,
enableInstrumentations,
Expand All @@ -36,8 +37,14 @@ export function registerInstrumentations(
);
const tracerProvider = options.tracerProvider || trace.getTracerProvider();
const meterProvider = options.meterProvider || metrics.getMeterProvider();
const loggerProvider = options.loggerProvider || logs.getLoggerProvider();

enableInstrumentations(instrumentations, tracerProvider, meterProvider);
enableInstrumentations(
instrumentations,
tracerProvider,
meterProvider,
loggerProvider
);

return () => {
disableInstrumentations(instrumentations);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import { TracerProvider, MeterProvider } from '@opentelemetry/api';
import { Instrumentation } from './types';
import { AutoLoaderResult, InstrumentationOption } from './types_internal';
import { LoggerProvider } from '@opentelemetry/api-logs';

/**
* Parses the options and returns instrumentations, node plugins and
Expand Down Expand Up @@ -52,7 +53,8 @@ export function parseInstrumentationOptions(
export function enableInstrumentations(
instrumentations: Instrumentation[],
tracerProvider?: TracerProvider,
meterProvider?: MeterProvider
meterProvider?: MeterProvider,
loggerProvider?: LoggerProvider
): void {
for (let i = 0, j = instrumentations.length; i < j; i++) {
const instrumentation = instrumentations[i];
Expand All @@ -62,6 +64,9 @@ export function enableInstrumentations(
if (meterProvider) {
instrumentation.setMeterProvider(meterProvider);
}
if (loggerProvider) {
instrumentation.setLoggerProvider(loggerProvider);
}
// instrumentations have been already enabled during creation
// so enable only if user prevented that by setting enabled to false
// this is to prevent double enabling but when calling register all
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
Tracer,
TracerProvider,
} from '@opentelemetry/api';
import { Logger, LoggerProvider, logs } from '@opentelemetry/api-logs';
import * as shimmer from 'shimmer';
import {
InstrumentationModuleDefinition,
Expand All @@ -41,6 +42,7 @@

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

constructor(
Expand All @@ -58,8 +60,8 @@
});

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

this._meter = metrics.getMeter(instrumentationName, instrumentationVersion);
this._logger = logs.getLogger(instrumentationName, instrumentationVersion);
this._updateMetricInstruments();
}

Expand Down Expand Up @@ -90,6 +92,22 @@
this._updateMetricInstruments();
}

/* Returns logger */
protected get logger(): Logger {
return this._logger;

Check warning on line 97 in experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts

View check run for this annotation

Codecov / codecov/patch

experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts#L96-L97

Added lines #L96 - L97 were not covered by tests
}

/**
* Sets LoggerProvider to this plugin
* @param loggerProvider
*/
public setLoggerProvider(loggerProvider: LoggerProvider): void {
this._logger = loggerProvider.getLogger(
this.instrumentationName,
this.instrumentationVersion
);
}

/**
* Sets the new metric instruments with the current Meter.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

import { TracerProvider, MeterProvider } from '@opentelemetry/api';
import { LoggerProvider } from '@opentelemetry/api-logs';

/** Interface Instrumentation to apply patch. */
export interface Instrumentation {
Expand Down Expand Up @@ -43,6 +44,9 @@ export interface Instrumentation {
/** Method to set meter provider */
setMeterProvider(meterProvider: MeterProvider): void;

/** Method to set logger provider */
setLoggerProvider(loggerProvider: LoggerProvider): void;
hectorhdzg marked this conversation as resolved.
Show resolved Hide resolved

/** Method to set instrumentation config */
setConfig(config: InstrumentationConfig): void;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import { TracerProvider, MeterProvider } from '@opentelemetry/api';
import { InstrumentationBase } from './platform';
import { Instrumentation } from './types';
import { LoggerProvider } from '@opentelemetry/api-logs';

export type InstrumentationOption =
| typeof InstrumentationBase
Expand All @@ -32,4 +33,5 @@ export interface AutoLoaderOptions {
instrumentations?: InstrumentationOption[];
tracerProvider?: TracerProvider;
meterProvider?: MeterProvider;
loggerProvider?: LoggerProvider;
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
} from '../../src';

import { MeterProvider } from '@opentelemetry/sdk-metrics';
import { LoggerProvider } from '@opentelemetry/sdk-logs';

interface TestInstrumentationConfig extends InstrumentationConfig {
isActive?: boolean;
Expand Down Expand Up @@ -90,6 +91,21 @@ describe('BaseInstrumentation', () => {
});
});

describe('setLoggerProvider', () => {
it('should get a logger from provider', () => {
let called = true;
class TestLoggerProvider extends LoggerProvider {
override getLogger(name: any, version?: any, options?: any) {
called = true;
return super.getLogger(name, version, options);
}
}
instrumentation = new TestInstrumentation();
instrumentation.setLoggerProvider(new TestLoggerProvider());
assert.strictEqual(called, true);
});
});

describe('getConfig', () => {
it('should return instrumentation config', () => {
const instrumentation: Instrumentation = new TestInstrumentation({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
import * as assert from 'assert';
import * as sinon from 'sinon';
import { InstrumentationBase, registerInstrumentations } from '../../src';
import { Logger, LoggerOptions, LoggerProvider } from '@opentelemetry/api-logs';

class DummyTracerProvider implements TracerProvider {
getTracer(name: string, version?: string): Tracer {
Expand All @@ -37,6 +38,12 @@ class DummyMeterProvider implements MeterProvider {
}
}

class DummyLoggerProvider implements LoggerProvider {
getLogger(name: string, version?: string, options?: LoggerOptions): Logger {
throw new Error('not implemented');
}
}

class FooInstrumentation extends InstrumentationBase {
init() {
return [];
Expand All @@ -63,17 +70,21 @@ describe('autoLoader', () => {
let enableSpy: sinon.SinonSpy;
let setTracerProviderSpy: sinon.SinonSpy;
let setMeterProviderSpy: sinon.SinonSpy;
let setLoggerProviderSpy: sinon.SinonSpy;
const tracerProvider = new DummyTracerProvider();
const meterProvider = new DummyMeterProvider();
const loggerProvider = new DummyLoggerProvider();
beforeEach(() => {
instrumentation = new FooInstrumentation('foo', '1', {});
enableSpy = sinon.spy(instrumentation, 'enable');
setTracerProviderSpy = sinon.stub(instrumentation, 'setTracerProvider');
setMeterProviderSpy = sinon.stub(instrumentation, 'setMeterProvider');
setLoggerProviderSpy = sinon.stub(instrumentation, 'setLoggerProvider');
unload = registerInstrumentations({
instrumentations: [instrumentation],
tracerProvider,
meterProvider,
loggerProvider,
});
});

Expand All @@ -96,10 +107,12 @@ describe('autoLoader', () => {
enableSpy = sinon.spy(instrumentation, 'enable');
setTracerProviderSpy = sinon.stub(instrumentation, 'setTracerProvider');
setMeterProviderSpy = sinon.stub(instrumentation, 'setMeterProvider');
setLoggerProviderSpy = sinon.stub(instrumentation, 'setLoggerProvider');
unload = registerInstrumentations({
instrumentations: [instrumentation],
tracerProvider,
meterProvider,
loggerProvider,
});
assert.strictEqual(enableSpy.callCount, 1);
});
Expand All @@ -119,6 +132,12 @@ describe('autoLoader', () => {
assert.ok(setMeterProviderSpy.lastCall.args[0] === meterProvider);
assert.strictEqual(setMeterProviderSpy.lastCall.args.length, 1);
});

it('should set LoggerProvider', () => {
assert.strictEqual(setLoggerProviderSpy.callCount, 1);
assert.ok(setLoggerProviderSpy.lastCall.args[0] === loggerProvider);
assert.strictEqual(setLoggerProviderSpy.lastCall.args.length, 1);
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
},
{
"path": "../../../packages/sdk-metrics"
},
{
"path": "../api-logs"
}
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
},
{
"path": "../../../packages/sdk-metrics"
},
{
"path": "../api-logs"
}
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
},
{
"path": "../../../packages/sdk-metrics"
},
{
"path": "../api-logs"
}
]
}
6 changes: 5 additions & 1 deletion package-lock.json

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