-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(sdk-node): pass OTLP HTTP metric config #6666
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
Changes from 3 commits
ac5a078
2ab245d
7191c49
507b9e8
a9a1683
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ import { | |
| getNumberFromEnv, | ||
| getStringFromEnv, | ||
| getStringListFromEnv, | ||
| parseKeyPairsIntoRecord, | ||
| W3CBaggagePropagator, | ||
| W3CTraceContextPropagator, | ||
| } from '@opentelemetry/core'; | ||
|
|
@@ -71,6 +72,7 @@ import type { | |
| } from '@opentelemetry/configuration'; | ||
| import type { | ||
| AggregationOption, | ||
| AggregationSelector, | ||
| IAttributesProcessor, | ||
| IMetricReader, | ||
| PushMetricExporter, | ||
|
|
@@ -85,7 +87,10 @@ import { | |
| PeriodicExportingMetricReader, | ||
| } from '@opentelemetry/sdk-metrics'; | ||
| import { OTLPMetricExporter as OTLPGrpcMetricExporter } from '@opentelemetry/exporter-metrics-otlp-grpc'; | ||
| import { OTLPMetricExporter as OTLPHttpMetricExporter } from '@opentelemetry/exporter-metrics-otlp-http'; | ||
| import { | ||
| AggregationTemporalityPreference, | ||
| OTLPMetricExporter as OTLPHttpMetricExporter, | ||
| } from '@opentelemetry/exporter-metrics-otlp-http'; | ||
| import { OTLPMetricExporter as OTLPProtoMetricExporter } from '@opentelemetry/exporter-metrics-otlp-proto'; | ||
| import type { | ||
| BufferConfig, | ||
|
|
@@ -519,37 +524,90 @@ export function getOtlpMetricExporterFromEnv(): PushMetricExporter { | |
| return new OTLPProtoMetricExporter(); | ||
| } | ||
|
|
||
| type OtlpHttpMetricExporterConfigModel = NonNullable< | ||
| PeriodicMetricReaderConfigModel['exporter']['otlp_http'] | ||
| >; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: This type could be exported from the |
||
|
|
||
| function getMetricExporterCompression( | ||
| compression: string | null | undefined | ||
| ): CompressionAlgorithm { | ||
| return compression === 'gzip' | ||
| ? CompressionAlgorithm.GZIP | ||
| : CompressionAlgorithm.NONE; | ||
| } | ||
|
|
||
| function getMetricExporterTemporalityPreference( | ||
| temporalityPreference: OtlpHttpMetricExporterConfigModel['temporality_preference'] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Likewise, we could add an ExporterTemporalityPreferenceConfigModel export from the |
||
| ): AggregationTemporalityPreference | undefined { | ||
| switch (temporalityPreference) { | ||
| case 'delta': | ||
| return AggregationTemporalityPreference.DELTA; | ||
| case 'low_memory': | ||
| return AggregationTemporalityPreference.LOWMEMORY; | ||
| case 'cumulative': | ||
| return AggregationTemporalityPreference.CUMULATIVE; | ||
| default: | ||
| return undefined; | ||
| } | ||
| } | ||
|
|
||
| function getMetricExporterDefaultHistogramAggregation( | ||
| defaultHistogramAggregation: OtlpHttpMetricExporterConfigModel['default_histogram_aggregation'] | ||
| ): AggregationSelector | undefined { | ||
| if (defaultHistogramAggregation !== 'base2_exponential_bucket_histogram') { | ||
| return undefined; | ||
| } | ||
|
|
||
| return instrumentType => { | ||
| if (instrumentType === InstrumentType.HISTOGRAM) { | ||
| return { type: AggregationType.EXPONENTIAL_HISTOGRAM }; | ||
| } | ||
| return { type: AggregationType.DEFAULT }; | ||
| }; | ||
| } | ||
|
|
||
| function getOtlpHttpMetricExporterConfig( | ||
| config: OtlpHttpMetricExporterConfigModel | ||
| ) { | ||
| return { | ||
| compression: getMetricExporterCompression(config.compression), | ||
| url: config.endpoint ?? undefined, | ||
| headers: getHeadersFromConfiguration( | ||
| config.headers, | ||
| config.headers_list ?? undefined | ||
| ), | ||
| timeoutMillis: config.timeout ?? undefined, | ||
| httpAgentOptions: getHttpAgentOptionsFromTls(config.tls), | ||
| temporalityPreference: getMetricExporterTemporalityPreference( | ||
| config.temporality_preference | ||
| ), | ||
| aggregationPreference: getMetricExporterDefaultHistogramAggregation( | ||
| config.default_histogram_aggregation | ||
| ), | ||
| }; | ||
| } | ||
|
|
||
| export function getPeriodicMetricReaderFromConfiguration( | ||
| periodic: PeriodicMetricReaderConfigModel | ||
| ): IMetricReader | undefined { | ||
| if (periodic.exporter) { | ||
| let exporter; | ||
| if (periodic.exporter.otlp_http !== undefined) { | ||
| const encoding = periodic.exporter.otlp_http?.encoding ?? 'protobuf'; | ||
| if (periodic.exporter.otlp_http) { | ||
| const config = periodic.exporter.otlp_http; | ||
| const encoding = config.encoding ?? 'protobuf'; | ||
| const exporterConfig = getOtlpHttpMetricExporterConfig(config); | ||
| if (encoding === 'json') { | ||
| exporter = new OTLPHttpMetricExporter({ | ||
| compression: | ||
| periodic.exporter.otlp_http?.compression === 'gzip' | ||
| ? CompressionAlgorithm.GZIP | ||
| : CompressionAlgorithm.NONE, | ||
| }); | ||
| exporter = new OTLPHttpMetricExporter(exporterConfig); | ||
| } else if (encoding === 'protobuf') { | ||
| exporter = new OTLPProtoMetricExporter({ | ||
| compression: | ||
| periodic.exporter.otlp_http?.compression === 'gzip' | ||
| ? CompressionAlgorithm.GZIP | ||
| : CompressionAlgorithm.NONE, | ||
| }); | ||
| exporter = new OTLPProtoMetricExporter(exporterConfig); | ||
| } else { | ||
| diag.warn(`Unsupported OTLP metrics encoding: ${encoding}.`); | ||
| } | ||
| } | ||
| if (periodic.exporter.otlp_grpc !== undefined) { | ||
| if (periodic.exporter.otlp_grpc) { | ||
| const config = periodic.exporter.otlp_grpc; | ||
| exporter = new OTLPGrpcMetricExporter({ | ||
| compression: | ||
| periodic.exporter.otlp_grpc?.compression === 'gzip' | ||
| ? CompressionAlgorithm.GZIP | ||
| : CompressionAlgorithm.NONE, | ||
| compression: getMetricExporterCompression(config.compression), | ||
| }); | ||
| } | ||
|
|
||
|
|
@@ -693,13 +751,14 @@ export function getLogRecordProcessorsFromConfiguration( | |
| } | ||
|
|
||
| export function getHeadersFromConfiguration( | ||
| headers: NameStringValuePairConfigModel[] | undefined | ||
| headers: NameStringValuePairConfigModel[] | undefined, | ||
| headersList?: string | ||
| ): Record<string, string> | undefined { | ||
| if (!headers) { | ||
| if (!headers && headersList === undefined) { | ||
| return undefined; | ||
| } | ||
| const result: Record<string, string> = {}; | ||
| headers.forEach(header => { | ||
| const result: Record<string, string> = parseKeyPairsIntoRecord(headersList); | ||
| headers?.forEach(header => { | ||
| if (header.value !== null) { | ||
| result[header.name] = header.value; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -34,9 +34,58 @@ import { | |||||
| serviceInstanceIdDetector, | ||||||
| } from '@opentelemetry/resources'; | ||||||
| import type { LoggerProviderOptions } from '@opentelemetry/sdk-logs'; | ||||||
| import { AggregationType, InstrumentType } from '@opentelemetry/sdk-metrics'; | ||||||
| import type { AggregationOption } from '@opentelemetry/sdk-metrics'; | ||||||
| import { | ||||||
| AggregationTemporality, | ||||||
| AggregationType, | ||||||
| InstrumentType, | ||||||
| } from '@opentelemetry/sdk-metrics'; | ||||||
| import type { SpanLimits } from '@opentelemetry/sdk-trace-node'; | ||||||
|
|
||||||
| interface OtlpHttpTransportParameters { | ||||||
| url: string; | ||||||
| compression: string; | ||||||
| headers: () => Promise<Record<string, string>>; | ||||||
| agentFactory: (protocol: string) => Promise<{ | ||||||
| options: { | ||||||
| ca?: Buffer; | ||||||
| cert?: Buffer; | ||||||
| key?: Buffer; | ||||||
| }; | ||||||
| }>; | ||||||
| } | ||||||
|
|
||||||
| interface OtlpMetricExporterInternals { | ||||||
| _delegate: { | ||||||
| _timeout: number; | ||||||
| _transport: { | ||||||
| _transport: { | ||||||
| _parameters: OtlpHttpTransportParameters; | ||||||
| }; | ||||||
| }; | ||||||
| }; | ||||||
| selectAggregation(instrumentType: InstrumentType): AggregationOption; | ||||||
| selectAggregationTemporality( | ||||||
| instrumentType: InstrumentType | ||||||
| ): AggregationTemporality; | ||||||
| } | ||||||
|
|
||||||
| interface PeriodicMetricReaderInternals { | ||||||
| _exporter: OtlpMetricExporterInternals; | ||||||
| } | ||||||
|
|
||||||
| function getMetricExporterInternals( | ||||||
| reader: unknown | ||||||
| ): OtlpMetricExporterInternals { | ||||||
| return (reader as PeriodicMetricReaderInternals)._exporter; | ||||||
| } | ||||||
|
|
||||||
| function getHttpTransportParameters( | ||||||
| exporter: OtlpMetricExporterInternals | ||||||
| ): OtlpHttpTransportParameters { | ||||||
| return exporter._delegate._transport._transport._parameters; | ||||||
| } | ||||||
|
|
||||||
| describe('getPropagatorFromEnv', function () { | ||||||
| afterEach(() => { | ||||||
| delete process.env.OTEL_PROPAGATORS; | ||||||
|
|
@@ -433,6 +482,78 @@ describe('getBatchLogRecordProcessorConfigFromEnv', function () { | |||||
| ); | ||||||
| }); | ||||||
|
|
||||||
| it('passes OTLP HTTP metric exporter connection options from configuration', async function () { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Move this |
||||||
| const reader = getPeriodicMetricReaderFromConfiguration({ | ||||||
| interval: 7000, | ||||||
| timeout: 5000, | ||||||
| exporter: { | ||||||
| otlp_http: { | ||||||
| endpoint: 'https://collector.example/v1/metrics', | ||||||
| tls: { | ||||||
| ca_file: 'test/fixtures/ca.pem', | ||||||
| key_file: 'test/fixtures/ca-key.pem', | ||||||
| cert_file: 'test/fixtures/cert.pem', | ||||||
| }, | ||||||
| headers_list: 'x-list=list-value,x-overridden=list-value', | ||||||
| headers: [ | ||||||
| { name: 'x-test-header', value: 'test-value' }, | ||||||
| { name: 'x-overridden', value: 'header-value' }, | ||||||
| ], | ||||||
| compression: 'gzip', | ||||||
| timeout: 1234, | ||||||
| }, | ||||||
| }, | ||||||
| }); | ||||||
|
|
||||||
| assert.ok(reader); | ||||||
| const exporter = getMetricExporterInternals(reader); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could the same thing be accomplished with a type assertion to any? E.g.:
Suggested change
I guess I'm expressing a personal preference to avoid defining |
||||||
| const parameters = getHttpTransportParameters(exporter); | ||||||
|
|
||||||
| assert.strictEqual(parameters.url, 'https://collector.example/v1/metrics'); | ||||||
| assert.strictEqual(parameters.compression, 'gzip'); | ||||||
| assert.strictEqual(exporter._delegate._timeout, 1234); | ||||||
| assert.deepStrictEqual(await parameters.headers(), { | ||||||
| 'x-list': 'list-value', | ||||||
| 'x-overridden': 'header-value', | ||||||
| 'x-test-header': 'test-value', | ||||||
| 'Content-Type': 'application/x-protobuf', | ||||||
| }); | ||||||
|
|
||||||
| const agent = await parameters.agentFactory('https:'); | ||||||
| assert.ok(agent.options.ca); | ||||||
| assert.ok(agent.options.key); | ||||||
| assert.ok(agent.options.cert); | ||||||
| }); | ||||||
|
|
||||||
| it('passes OTLP HTTP metric exporter aggregation options from configuration', function () { | ||||||
| const reader = getPeriodicMetricReaderFromConfiguration({ | ||||||
| exporter: { | ||||||
| otlp_http: { | ||||||
| encoding: 'json', | ||||||
| temporality_preference: 'delta', | ||||||
| default_histogram_aggregation: 'base2_exponential_bucket_histogram', | ||||||
| }, | ||||||
| }, | ||||||
| }); | ||||||
|
|
||||||
| assert.ok(reader); | ||||||
| const exporter = getMetricExporterInternals(reader); | ||||||
|
|
||||||
| assert.strictEqual( | ||||||
| exporter.selectAggregationTemporality(InstrumentType.COUNTER), | ||||||
| AggregationTemporality.DELTA | ||||||
| ); | ||||||
| assert.deepStrictEqual( | ||||||
| exporter.selectAggregation(InstrumentType.HISTOGRAM), | ||||||
| { | ||||||
| type: AggregationType.EXPONENTIAL_HISTOGRAM, | ||||||
| } | ||||||
| ); | ||||||
| assert.deepStrictEqual(exporter.selectAggregation(InstrumentType.COUNTER), { | ||||||
| type: AggregationType.DEFAULT, | ||||||
| }); | ||||||
| }); | ||||||
|
|
||||||
| it('should return values for getInstrumentType', function () { | ||||||
| assert.deepStrictEqual( | ||||||
| getInstrumentType('counter' as InstrumentTypeConfigModel), | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This now needs to move up to the unreleased section.