Skip to content

Commit

Permalink
Merge branch 'main' into metrics-ff/doc
Browse files Browse the repository at this point in the history
  • Loading branch information
legendecas committed Aug 17, 2022
2 parents b0de445 + 43f4e5a commit 3be670e
Show file tree
Hide file tree
Showing 5 changed files with 199 additions and 16 deletions.
1 change: 1 addition & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ All notable changes to experimental packages in this project will be documented

### :rocket: (Enhancement)

* fix(add-views-to-node-sdk): added the ability to define meter views in `NodeSDK` [#3066](https://github.com/open-telemetry/opentelemetry-js/pull/3124) @weyert
* feature(add-console-metrics-exporter): add ConsoleMetricExporter [#3120](https://github.com/open-telemetry/opentelemetry-js/pull/3120) @weyert
* feature(prometheus-serialiser): export the unit block when unit is set in metric descriptor [#3066](https://github.com/open-telemetry/opentelemetry-js/pull/3041) @weyert

Expand Down
4 changes: 4 additions & 0 deletions experimental/packages/opentelemetry-sdk-node/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,10 @@ Configure a trace exporter. If an exporter OR span processor is not configured,

Configure tracing parameters. These are the same trace parameters used to [configure a tracer](../../../packages/opentelemetry-sdk-trace-base/src/types.ts#L71).

### views

Configure views of your instruments and accepts an array of [View](../opentelemetry-sdk-metrics-base/src/view/View.ts)-instances. The parameter can be used to configure the explicit bucket sizes of histogram metrics.

### serviceName

Configure the [service name](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/semantic_conventions/README.md#service).
Expand Down
70 changes: 60 additions & 10 deletions experimental/packages/opentelemetry-sdk-node/src/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {
Resource,
ResourceDetectionConfig
} from '@opentelemetry/resources';
import { MeterProvider, MetricReader } from '@opentelemetry/sdk-metrics';
import { MeterProvider, MetricReader, View } from '@opentelemetry/sdk-metrics';
import {
BatchSpanProcessor,
SpanProcessor
Expand All @@ -37,15 +37,29 @@ import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions'
import { NodeSDKConfiguration } from './types';

/** This class represents everything needed to register a fully configured OpenTelemetry Node.js SDK */

export type MeterProviderConfig = {
/**
* Reference to the MetricReader instance by the NodeSDK
*/
reader?: MetricReader
/**
* Lists the views that should be passed when meterProvider
*
* Note: This is only getting used when NodeSDK is responsible for
* instantiated an instance of MeterProvider
*/
views?: View[]
};
export class NodeSDK {
private _tracerProviderConfig?: {
tracerConfig: NodeTracerConfig;
spanProcessor: SpanProcessor;
contextManager?: ContextManager;
textMapPropagator?: TextMapPropagator;
};
private _meterProviderConfig?: MeterProviderConfig;
private _instrumentations: InstrumentationOption[];
private _metricReader?: MetricReader;

private _resource: Resource;

Expand Down Expand Up @@ -87,8 +101,17 @@ export class NodeSDK {
);
}

if (configuration.metricReader) {
this.configureMeterProvider(configuration.metricReader);
if (configuration.metricReader || configuration.views) {
const meterProviderConfig: MeterProviderConfig = {};
if (configuration.metricReader) {
meterProviderConfig.reader = configuration.metricReader;
}

if (configuration.views) {
meterProviderConfig.views = configuration.views;
}

this.configureMeterProvider(meterProviderConfig);
}

let instrumentations: InstrumentationOption[] = [];
Expand All @@ -114,16 +137,40 @@ export class NodeSDK {
}

/** Set configurations needed to register a MeterProvider */
public configureMeterProvider(reader: MetricReader): void {
this._metricReader = reader;
public configureMeterProvider(config: MeterProviderConfig): void {
// nothing is set yet, we can set config and return.
if (this._meterProviderConfig == null) {
this._meterProviderConfig = config;
return;
}

// make sure we do not override existing views with other views.
if (this._meterProviderConfig.views != null && config.views != null) {
throw new Error('Views passed but Views have already been configured.');
}

// set views, but make sure we do not override existing views with null/undefined.
if (config.views != null) {
this._meterProviderConfig.views = config.views;
}

// make sure we do not override existing reader with another reader.
if (this._meterProviderConfig.reader != null && config.reader != null) {
throw new Error('MetricReader passed but MetricReader has already been configured.');
}

// set reader, but make sure we do not override existing reader with null/undefined.
if (config.reader != null) {
this._meterProviderConfig.reader = config.reader;
}
}

/** Detect resource attributes */
public async detectResources(
config?: ResourceDetectionConfig
): Promise<void> {
const internalConfig: ResourceDetectionConfig = {
detectors: [ envDetector, processDetector],
detectors: [envDetector, processDetector],
...config,
};

Expand All @@ -146,7 +193,7 @@ export class NodeSDK {
this._resource = this._serviceName === undefined
? this._resource
: this._resource.merge(new Resource(
{[SemanticResourceAttributes.SERVICE_NAME]: this._serviceName}
{ [SemanticResourceAttributes.SERVICE_NAME]: this._serviceName }
));

if (this._tracerProviderConfig) {
Expand All @@ -164,12 +211,15 @@ export class NodeSDK {
});
}

if (this._metricReader) {
if (this._meterProviderConfig) {
const meterProvider = new MeterProvider({
resource: this._resource,
views: this._meterProviderConfig?.views ?? [],
});

meterProvider.addMetricReader(this._metricReader);
if (this._meterProviderConfig.reader) {
meterProvider.addMetricReader(this._meterProviderConfig.reader);
}

this._meterProvider = meterProvider;

Expand Down
3 changes: 2 additions & 1 deletion experimental/packages/opentelemetry-sdk-node/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import type { ContextManager, SpanAttributes } from '@opentelemetry/api';
import { TextMapPropagator } from '@opentelemetry/api';
import { InstrumentationOption } from '@opentelemetry/instrumentation';
import { Resource } from '@opentelemetry/resources';
import { MetricReader } from '@opentelemetry/sdk-metrics';
import { MetricReader, View } from '@opentelemetry/sdk-metrics';
import {
Sampler,
SpanExporter,
Expand All @@ -32,6 +32,7 @@ export interface NodeSDKConfiguration {
defaultAttributes: SpanAttributes;
textMapPropagator: TextMapPropagator;
metricReader: MetricReader;
views: View[]
instrumentations: InstrumentationOption[];
resource: Resource;
sampler: Sampler;
Expand Down
137 changes: 132 additions & 5 deletions experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {
AsyncLocalStorageContextManager,
} from '@opentelemetry/context-async-hooks';
import { CompositePropagator } from '@opentelemetry/core';
import { ConsoleMetricExporter, MeterProvider, PeriodicExportingMetricReader } from '@opentelemetry/sdk-metrics';
import { AggregationTemporality, ConsoleMetricExporter, InMemoryMetricExporter, InstrumentType, MeterProvider, PeriodicExportingMetricReader, View } from '@opentelemetry/sdk-metrics';
import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node';
import {
assertServiceResource,
Expand Down Expand Up @@ -147,6 +147,133 @@ describe('Node SDK', () => {
});
});

async function waitForNumberOfMetrics(exporter: InMemoryMetricExporter, numberOfMetrics: number): Promise<void> {
if (numberOfMetrics <= 0) {
throw new Error('numberOfMetrics must be greater than or equal to 0');
}

let totalExports = 0;
while (totalExports < numberOfMetrics) {
await new Promise(resolve => setTimeout(resolve, 20));
const exportedMetrics = exporter.getMetrics();
totalExports = exportedMetrics.length;
}
}

it('should register meter views when provided', async () => {
const exporter = new InMemoryMetricExporter(AggregationTemporality.CUMULATIVE);
const metricReader = new PeriodicExportingMetricReader({
exporter: exporter,
exportIntervalMillis: 100,
exportTimeoutMillis: 100
});

const sdk = new NodeSDK({
metricReader: metricReader,
views: [
new View({
name: 'test-view',
instrumentName: 'test_counter',
instrumentType: InstrumentType.COUNTER,
})
],
autoDetectResources: false,
});

await sdk.start();

assert.strictEqual(context['_getContextManager'](), ctxManager, 'context manager should not change');
assert.strictEqual(propagation['_getGlobalPropagator'](), propagator, 'propagator should not change');
assert.strictEqual((trace.getTracerProvider() as ProxyTracerProvider).getDelegate(), delegate, 'tracer provider should not have changed');

const meterProvider = metrics.getMeterProvider() as MeterProvider;
assert.ok(meterProvider);

const meter = meterProvider.getMeter('NodeSDKViews', '1.0.0');
const counter = meter.createCounter('test_counter', {
description: 'a test description',
});
counter.add(10);

await waitForNumberOfMetrics(exporter, 1);
const exportedMetrics = exporter.getMetrics();
const [firstExportedMetric] = exportedMetrics;
assert.ok(firstExportedMetric, 'should have one exported metric');
const [firstScopeMetric] = firstExportedMetric.scopeMetrics;
assert.ok(firstScopeMetric, 'should have one scope metric');
assert.ok(firstScopeMetric.scope.name === 'NodeSDKViews', 'scope should match created view');
assert.ok(firstScopeMetric.metrics.length > 0, 'should have at least one metrics entry');
const [firstMetricRecord] = firstScopeMetric.metrics;
assert.ok(firstMetricRecord.descriptor.name === 'test-view', 'should have renamed counter metric');

await sdk.shutdown();
});

it('should throw error when calling configureMeterProvider when views are already configured', () => {
const exporter = new InMemoryMetricExporter(AggregationTemporality.CUMULATIVE);
const metricReader = new PeriodicExportingMetricReader({
exporter: exporter,
exportIntervalMillis: 100,
exportTimeoutMillis: 100
});

const sdk = new NodeSDK({
metricReader: metricReader,
views: [
new View({
name: 'test-view',
instrumentName: 'test_counter',
instrumentType: InstrumentType.COUNTER,
})
],
autoDetectResources: false,
});

assert.throws(() => {
sdk.configureMeterProvider({
reader: metricReader,
views: [
new View({
name: 'test-view',
instrumentName: 'test_counter',
instrumentType: InstrumentType.COUNTER,
})
]
});
}, (error: Error) => {
return error.message.includes('Views passed but Views have already been configured');
});
});

it('should throw error when calling configureMeterProvider when metricReader is already configured', () => {
const exporter = new InMemoryMetricExporter(AggregationTemporality.CUMULATIVE);
const metricReader = new PeriodicExportingMetricReader({
exporter: exporter,
exportIntervalMillis: 100,
exportTimeoutMillis: 100
});

const sdk = new NodeSDK({
metricReader: metricReader,
views: [
new View({
name: 'test-view',
instrumentName: 'test_counter',
instrumentType: InstrumentType.COUNTER,
})
],
autoDetectResources: false,
});

assert.throws(() => {
sdk.configureMeterProvider({
reader: metricReader,
});
}, (error: Error) => {
return error.message.includes('MetricReader passed but MetricReader has already been configured.');
});
});

describe('detectResources', async () => {
beforeEach(() => {
process.env.OTEL_RESOURCE_ATTRIBUTES =
Expand All @@ -163,12 +290,12 @@ describe('Node SDK', () => {
autoDetectResources: true,
});
await sdk.detectResources({
detectors: [ processDetector, {
detectors: [processDetector, {
detect() {
throw new Error('Buggy detector');
}
},
envDetector ]
envDetector]
});
const resource = sdk['_resource'];

Expand Down Expand Up @@ -277,7 +404,7 @@ describe('Node SDK', () => {
});

it('should configure service name via OTEL_SERVICE_NAME env var', async () => {
process.env.OTEL_SERVICE_NAME='env-set-name';
process.env.OTEL_SERVICE_NAME = 'env-set-name';
const sdk = new NodeSDK();

await sdk.start();
Expand All @@ -290,7 +417,7 @@ describe('Node SDK', () => {
});

it('should favor config set service name over OTEL_SERVICE_NAME env set service name', async () => {
process.env.OTEL_SERVICE_NAME='env-set-name';
process.env.OTEL_SERVICE_NAME = 'env-set-name';
const sdk = new NodeSDK({
serviceName: 'config-set-name',
});
Expand Down

0 comments on commit 3be670e

Please sign in to comment.