Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
12 changes: 7 additions & 5 deletions experimental/packages/opentelemetry-sdk-node/src/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,12 @@ export class NodeSDK {
})
);

// While SDK metrics are unstable, we require an opt-in.
// https://opentelemetry.io/docs/specs/semconv/otel/sdk-metrics/
const sdkMetricsEnabled = getBooleanFromEnv(
'OTEL_NODE_EXPERIMENTAL_SDK_METRICS'
);

if (
this._meterProviderConfig?.readers &&
// only register if there is a reader, otherwise we waste compute/memory.
Expand All @@ -335,6 +341,7 @@ export class NodeSDK {
resource: this._resource,
views: this._meterProviderConfig?.views ?? [],
readers: this._meterProviderConfig.readers,
sdkMetricsEnabled,
});

this._meterProvider = meterProvider;
Expand All @@ -354,11 +361,6 @@ export class NodeSDK {

// Only register if there is a span processor
if (spanProcessors.length > 0) {
// While SDK metrics are unstable, we require an opt-in.
// https://opentelemetry.io/docs/specs/semconv/otel/sdk-metrics/
const sdkMetricsEnabled = getBooleanFromEnv(
'OTEL_NODE_EXPERIMENTAL_SDK_METRICS'
);
this._tracerProvider = new NodeTracerProvider({
...this._configuration,
resource: this._resource,
Expand Down
13 changes: 11 additions & 2 deletions experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ import { OTLPTraceExporter as OTLPGrpcTraceExporter } from '@opentelemetry/expor
import { ZipkinExporter } from '@opentelemetry/exporter-zipkin';

import { ATTR_HOST_NAME, ATTR_PROCESS_PID } from '../src/semconv';
import { NOOP_HISTOGRAM_METRIC } from '../../../../api/src/metrics/NoopMeter';

function assertDefaultContextManagerRegistered() {
assert.ok(
Expand Down Expand Up @@ -420,7 +421,7 @@ describe('Node SDK', () => {
await sdk.shutdown();
});

it('should register a meter provider to the tracer provider if both initialized and metrics enabled', async () => {
it('should configure components for metrics if enabled', async () => {
process.env.OTEL_NODE_EXPERIMENTAL_SDK_METRICS = 'true';
const exporter = new ConsoleMetricExporter();
const metricReader = new PeriodicExportingMetricReader({
Expand Down Expand Up @@ -448,11 +449,15 @@ describe('Node SDK', () => {
);

assert.ok(metrics.getMeterProvider() instanceof MeterProvider);
assert.notDeepEqual(
(metricReader as any)._metrics.collectionDuration,
NOOP_HISTOGRAM_METRIC
);

await sdk.shutdown();
});

it('should not register a meter provider to the tracer provider if both initialized but metrics disabled', async () => {
it('should not configure components for metrics if disabled', async () => {
const exporter = new ConsoleMetricExporter();
const metricReader = new PeriodicExportingMetricReader({
exporter: exporter,
Expand All @@ -477,6 +482,10 @@ describe('Node SDK', () => {
assert.equal((tracerProvider as any)._config.meterProvider, undefined);

assert.ok(metrics.getMeterProvider() instanceof MeterProvider);
assert.deepEqual(
(metricReader as any)._metrics.collectionDuration,
NOOP_HISTOGRAM_METRIC
);

await sdk.shutdown();
});
Expand Down
13 changes: 13 additions & 0 deletions packages/sdk-metrics/src/MeterProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { MeterProviderSharedState } from './state/MeterProviderSharedState';
import { MetricCollector } from './state/MetricCollector';
import { ForceFlushOptions, ShutdownOptions } from './types';
import { View, ViewOptions } from './view/View';
import { PeriodicExportingMetricReader } from './export/PeriodicExportingMetricReader';

/**
* MeterProviderOptions provides an interface for configuring a MeterProvider.
Expand All @@ -36,6 +37,12 @@ export interface MeterProviderOptions {
resource?: Resource;
views?: ViewOptions[];
readers?: IMetricReader[];

/**
* Whether to enable SDK metrics for this meter provider.
* @experimental This option is experimental and is subject to breaking changes in minor releases.
*/
sdkMetricsEnabled?: boolean;
}

/**
Expand All @@ -60,6 +67,12 @@ export class MeterProvider implements IMeterProvider {
const collector = new MetricCollector(this._sharedState, metricReader);
metricReader.setMetricProducer(collector);
this._sharedState.metricCollectors.push(collector);
if (
options.sdkMetricsEnabled &&
metricReader instanceof PeriodicExportingMetricReader
) {
metricReader.setMeterProvider(this);
}
}
}
}
Expand Down
62 changes: 62 additions & 0 deletions packages/sdk-metrics/src/export/MetricReaderMetrics.ts
Comment thread
anuraaga marked this conversation as resolved.
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* Copyright The OpenTelemetry Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { Attributes, Histogram, Meter } from '@opentelemetry/api';
import {
ATTR_ERROR_TYPE,
ATTR_OTEL_COMPONENT_NAME,
ATTR_OTEL_COMPONENT_TYPE,
METRIC_OTEL_SDK_METRIC_READER_COLLECTION_DURATION,
} from '../semconv';

const componentCounter = new Map<string, number>();

/**
* Generates `otel.sdk.metric_reader.*` metrics.
* https://opentelemetry.io/docs/specs/semconv/otel/sdk-metrics/#metric-otelsdkmetric_readercollectionduration
*/
export class MetricReaderMetrics {
private readonly collectionDuration: Histogram;
private readonly standardAttrs: Attributes;

constructor(componentType: string, meter: Meter) {
const counter = componentCounter.get(componentType) ?? 0;
componentCounter.set(componentType, counter + 1);

this.standardAttrs = {
[ATTR_OTEL_COMPONENT_TYPE]: componentType,
[ATTR_OTEL_COMPONENT_NAME]: `${componentType}/${counter}`,
};

this.collectionDuration = meter.createHistogram(
METRIC_OTEL_SDK_METRIC_READER_COLLECTION_DURATION,
{
unit: 's',
description:
'The duration of the collect operation of the metric reader.',
advice: {
explicitBucketBoundaries: [],
},
}
);
}

recordCollection(durationSecs: number, error: string | undefined) {
const attrs = error
? { ...this.standardAttrs, [ATTR_ERROR_TYPE]: error }
: this.standardAttrs;
this.collectionDuration.record(durationSecs, attrs);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,16 @@ import {
internal,
ExportResultCode,
globalErrorHandler,
hrTime,
hrTimeDuration,
} from '@opentelemetry/core';
import { MetricReader } from './MetricReader';
import { PushMetricExporter } from './MetricExporter';
import { callWithTimeout, TimeoutError } from '../utils';
import { MetricProducer } from './MetricProducer';
import { MetricReaderMetrics } from './MetricReaderMetrics';
import { OTEL_COMPONENT_TYPE_VALUE_PERIODIC_METRIC_READER } from '../semconv';
import { VERSION } from '../version';

export type PeriodicExportingMetricReaderOptions = {
/**
Expand Down Expand Up @@ -55,6 +60,7 @@ export type PeriodicExportingMetricReaderOptions = {
export class PeriodicExportingMetricReader extends MetricReader {
private _interval?: ReturnType<typeof setInterval>;
private _exporter: PushMetricExporter;
private _metrics: MetricReaderMetrics;
private readonly _exportInterval: number;
private readonly _exportTimeout: number;

Expand Down Expand Up @@ -98,6 +104,18 @@ export class PeriodicExportingMetricReader extends MetricReader {
this._exportInterval = exportIntervalMillis;
this._exportTimeout = exportTimeoutMillis;
this._exporter = exporter;
this._metrics = new MetricReaderMetrics(
OTEL_COMPONENT_TYPE_VALUE_PERIODIC_METRIC_READER,
api.createNoopMeter()
);
}

setMeterProvider(meterProvider: api.MeterProvider) {
const meter = meterProvider.getMeter('@opentelemetry/sdk-metrics', VERSION);
this._metrics = new MetricReaderMetrics(
OTEL_COMPONENT_TYPE_VALUE_PERIODIC_METRIC_READER,
meter
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about moving the MetricReaderMetrics instance to the MetricReader base class:

  • setMeterProvider moves to class MetricReader (and the method gets added to interface IMetricReader)
  • The setMeterProvider needs a value for otel.component.type, so a class extending MetricReader should be able to pass that in:
    • Add an optional otelComponentType string option to interface MetricReaderOptions
    • Get class PeriodicExportingMetricReader to pass in OTEL_COMPONENT_TYPE_VALUE_PERIODIC_METRIC_READER in its call to super()
    • Get class MetricReader to save that to private readonly _otelComponentType: string | undefined;
    • Get the implementation of setMeterProvider to use this._otelComponentType || this.constructor.name. This will allow a reasonable default value for MetricReader subclasses that don't fit one of the well-known names for otel.component.type. (From the spec: "otel.component.type: If none of the standardized values apply, implementations SHOULD use the language-defined name of the type. E.g. for Java the fully qualified classname SHOULD be used in this case.")
  • Move the handling for this._metrics.recordCollection(...) to MetricReader#collect().
  • Then it would also be good to update "experimental/packages/opentelemetry-exporter-prometheus/src/PrometheusExporter.ts" to pass in otelComponentType: OTEL_COMPONENT_TYPE_VALUE_PROMETHEUS_HTTP_TEXT_METRIC_EXPORTER in its call to super()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea that makes sense - did it and also the prometheus change. One deviation from this comment I resolved _otelComponentType to the automatic value in the constructor instead of setMeterProvider, let me know if I'm missing some JS intricacy that means that shouldn't be done.

}

private async _runOnce(): Promise<void> {
Expand All @@ -117,17 +135,25 @@ export class PeriodicExportingMetricReader extends MetricReader {
}

private async _doRun(): Promise<void> {
const startTime = hrTime();
const { resourceMetrics, errors } = await this.collect({
timeoutMillis: this._exportTimeout,
});
const endTime = hrTime();
let collectError: string | undefined = undefined;

if (errors.length > 0) {
api.diag.error(
'PeriodicExportingMetricReader: metrics collection errors',
...errors
);
collectError = (errors[0] as Error).name ?? 'collect_error';
}

const collectDuration = hrTimeDuration(startTime, endTime);
const collectDurationSecs = collectDuration[0] + collectDuration[1] / 1e9;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know if I missed something, I was surprised to not find hrTimeToSeconds since there are many metrics defined as seconds

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be worthwhile adding hrTimeToSeconds to "packages/opentelemetry-core/src/common/time.ts". That could totally be in a separate PR, though. Or an issue would be welcome.

this._metrics.recordCollection(collectDurationSecs, collectError);

if (resourceMetrics.resource.asyncAttributesPending) {
try {
await resourceMetrics.resource.waitForAsyncAttributes?.();
Expand Down
109 changes: 109 additions & 0 deletions packages/sdk-metrics/src/semconv.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
/*
* Copyright The OpenTelemetry Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/*
* This file contains a copy of unstable semantic convention definitions
* used by this package.
* @see https://github.com/open-telemetry/opentelemetry-js/tree/main/semantic-conventions#unstable-semconv
*/

/**
* A name uniquely identifying the instance of the OpenTelemetry component within its containing SDK instance.
*
* @example otlp_grpc_span_exporter/0
* @example custom-name
*
* @note Implementations **SHOULD** ensure a low cardinality for this attribute, even across application or SDK restarts.
* E.g. implementations **MUST NOT** use UUIDs as values for this attribute.
*
* Implementations **MAY** achieve these goals by following a `<otel.component.type>/<instance-counter>` pattern, e.g. `batching_span_processor/0`.
* Hereby `otel.component.type` refers to the corresponding attribute value of the component.
*
* The value of `instance-counter` **MAY** be automatically assigned by the component and uniqueness within the enclosing SDK instance **MUST** be guaranteed.
* For example, `<instance-counter>` **MAY** be implemented by using a monotonically increasing counter (starting with `0`), which is incremented every time an
* instance of the given component type is started.
*
* With this implementation, for example the first Batching Span Processor would have `batching_span_processor/0`
* as `otel.component.name`, the second one `batching_span_processor/1` and so on.
* These values will therefore be reused in the case of an application restart.
*
* @experimental This attribute is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`.
*/
export const ATTR_OTEL_COMPONENT_NAME = 'otel.component.name' as const;

/**
* A name identifying the type of the OpenTelemetry component.
*
* @example batching_span_processor
* @example com.example.MySpanExporter
*
* @note If none of the standardized values apply, implementations **SHOULD** use the language-defined name of the type.
* E.g. for Java the fully qualified classname **SHOULD** be used in this case.
*
* @experimental This attribute is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`.
*/
export const ATTR_OTEL_COMPONENT_TYPE = 'otel.component.type' as const;

/**
* Enum value "periodic_metric_reader" for attribute {@link ATTR_OTEL_COMPONENT_TYPE}.
*
* The builtin SDK periodically exporting metric reader
*
* @experimental This enum value is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`.
*/
export const OTEL_COMPONENT_TYPE_VALUE_PERIODIC_METRIC_READER =
'periodic_metric_reader' as const;

/**
* The duration of the collect operation of the metric reader.
*
* @note For successful collections, `error.type` **MUST NOT** be set. For failed collections, `error.type` **SHOULD** contain the failure cause.
* It can happen that metrics collection is successful for some MetricProducers, while others fail. In that case `error.type` **SHOULD** be set to any of the failure causes.
*
* @experimental This metric is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`.
*/
export const METRIC_OTEL_SDK_METRIC_READER_COLLECTION_DURATION =
'otel.sdk.metric_reader.collection.duration' as const;

/**
* Describes a class of error the operation ended with.
*
* @example timeout
* @example java.net.UnknownHostException
* @example server_certificate_invalid
* @example 500
*
* @note The `error.type` **SHOULD** be predictable, and **SHOULD** have low cardinality.
*
* When `error.type` is set to a type (e.g., an exception type), its
* canonical class name identifying the type within the artifact **SHOULD** be used.
*
* Instrumentations **SHOULD** document the list of errors they report.
*
* The cardinality of `error.type` within one instrumentation library **SHOULD** be low.
* Telemetry consumers that aggregate data from multiple instrumentation libraries and applications
* should be prepared for `error.type` to have high cardinality at query time when no
* additional filters are applied.
*
* If the operation has completed successfully, instrumentations **SHOULD NOT** set `error.type`.
*
* If a specific domain defines its own set of error identifiers (such as HTTP or RPC status codes),
* it's **RECOMMENDED** to:
*
* - Use a domain-specific attribute
* - Set `error.type` to capture all errors, regardless of whether they are defined within the domain-specific set or not.
*/
export const ATTR_ERROR_TYPE = 'error.type' as const;

@anuraaga anuraaga Feb 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is actually stable but wasn't sure it's worth adding a dep on semconv for it

@trentm trentm Feb 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood. I don't have a strong opinion. If the semconv package was smaller we wouldn't hesistate to add the dep. :)

I imagine this usage will break attempts to use https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/scripts/gen-semconv-ts.js to re-generate this semconv.ts file, but that's not a real concern right now. That script is currently a best-effort helper and not mandated at all.

Loading
Loading