Skip to content

Commit

Permalink
Merge branch 'main' into fix/recording-NaN-values
Browse files Browse the repository at this point in the history
  • Loading branch information
pichlermarc authored Feb 6, 2024
2 parents d975333 + 588d8ad commit 1414982
Show file tree
Hide file tree
Showing 18 changed files with 167 additions and 17 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/

### :rocket: (Enhancement)

* feat(instrumentation): Make `init()` method public [#4418](https://github.com/open-telemetry/opentelemetry-js/pull/4418)

### :bug: (Bug Fix)

* fix(sdk-metrics): ignore `NaN` value recordings in Histograms [#4455](https://github.com/open-telemetry/opentelemetry-js/pull/4455) @pichlermarc
Expand Down
3 changes: 3 additions & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ 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
* feat(instrumentation): Make `init()` method public [#4418](https://github.com/open-telemetry/opentelemetry-js/pull/4418)
* feat(exporter-metrics-otlp-http): add option to set the exporter aggregation preference [#4409](https://github.com/open-telemetry/opentelemetry-js/pull/4409) @AkselAllas
* feat(node-sdk): add spanProcessors option [#4454](https://github.com/open-telemetry/opentelemetry-js/pull/4454) @naseemkullah

### :bug: (Bug Fix)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,14 @@
"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",
"@babel/preset-env": "7.22.20",
"@opentelemetry/api": "1.7.0",
"@opentelemetry/api-logs": "0.47.0",
"@opentelemetry/sdk-metrics": "1.21.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) {
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 @@ import {
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 @@ export abstract class InstrumentationAbstract<T = any>

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

constructor(
Expand All @@ -58,8 +60,8 @@ export abstract class InstrumentationAbstract<T = any>
});

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 @@ export abstract class InstrumentationAbstract<T = any>
this._updateMetricInstruments();
}

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

/**
* 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 Expand Up @@ -134,9 +152,12 @@ export abstract class InstrumentationAbstract<T = any>

/**
* Init method in which plugin should define _modules and patches for
* methods
* methods.
* Use `enable()` if you are trying to turn on this plugin. This method
* will return objects to patch specific modules with the appropriate
* instrumentation (or not return anything).
*/
protected abstract init():
abstract init():
| InstrumentationModuleDefinition<T>
| InstrumentationModuleDefinition<T>[]
| void;
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;

/** 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,23 @@ 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();
if (instrumentation.setLoggerProvider) {
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: 6 additions & 0 deletions experimental/packages/opentelemetry-sdk-node/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,12 @@ Configure a custom sampler. By default, all traces will be sampled.

### spanProcessor

Deprecated, please use [spanProcessors](#spanprocessors) instead.

### spanProcessors

An array of span processors to register to the tracer provider.

### traceExporter

Configure a trace exporter. If an exporter is configured, it will be used with a [BatchSpanProcessor](../../../packages/opentelemetry-sdk-trace-base/src/platform/node/export/BatchSpanProcessor.ts). If an exporter OR span processor is not configured programatically, this package will auto setup the default `otlp` exporter with `http/protobuf` protocol with a `BatchSpanProcessor`.
Expand Down
26 changes: 20 additions & 6 deletions experimental/packages/opentelemetry-sdk-node/src/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export type LoggerProviderConfig = {
export class NodeSDK {
private _tracerProviderConfig?: {
tracerConfig: NodeTracerConfig;
spanProcessor: SpanProcessor;
spanProcessors: SpanProcessor[];
contextManager?: ContextManager;
textMapPropagator?: TextMapPropagator;
};
Expand Down Expand Up @@ -130,7 +130,11 @@ export class NodeSDK {
this._autoDetectResources = configuration.autoDetectResources ?? true;

// If a tracer provider can be created from manual configuration, create it
if (configuration.traceExporter || configuration.spanProcessor) {
if (
configuration.traceExporter ||
configuration.spanProcessor ||
configuration.spanProcessors
) {
const tracerProviderConfig: NodeTracerConfig = {};

if (configuration.sampler) {
Expand All @@ -143,13 +147,21 @@ export class NodeSDK {
tracerProviderConfig.idGenerator = configuration.idGenerator;
}

if (configuration.spanProcessor) {
diag.warn(
"The 'spanProcessor' option is deprecated. Please use 'spanProcessors' instead."
);
}

const spanProcessor =
configuration.spanProcessor ??
new BatchSpanProcessor(configuration.traceExporter!);

const spanProcessors = configuration.spanProcessors ?? [spanProcessor];

this.configureTracerProvider(
tracerProviderConfig,
spanProcessor,
spanProcessors,
configuration.contextManager,
configuration.textMapPropagator
);
Expand Down Expand Up @@ -192,13 +204,13 @@ export class NodeSDK {
*/
public configureTracerProvider(
tracerConfig: NodeTracerConfig,
spanProcessor: SpanProcessor,
spanProcessors: SpanProcessor[],
contextManager?: ContextManager,
textMapPropagator?: TextMapPropagator
): void {
this._tracerProviderConfig = {
tracerConfig,
spanProcessor,
spanProcessors,
contextManager,
textMapPropagator,
};
Expand Down Expand Up @@ -334,7 +346,9 @@ export class NodeSDK {
this._tracerProvider = tracerProvider;

if (this._tracerProviderConfig) {
tracerProvider.addSpanProcessor(this._tracerProviderConfig.spanProcessor);
for (const spanProcessor of this._tracerProviderConfig.spanProcessors) {
tracerProvider.addSpanProcessor(spanProcessor);
}
}

tracerProvider.register({
Expand Down
Loading

0 comments on commit 1414982

Please sign in to comment.