From 1e5895bdf05d5dfb088672d8e90296a887c7ee45 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Thu, 14 Nov 2024 18:25:09 +0100 Subject: [PATCH 1/2] feat(otlp-transformer)!: accept ResourceMetrics instead of ResoruceMetrics[] in metrics serializers --- experimental/CHANGELOG.md | 3 + .../src/OTLPMetricExporterBase.ts | 36 +++--- .../common/OTLPMetricExporterBase.test.ts | 105 ------------------ .../otlp-transformer/src/json/serializers.ts | 6 +- .../src/protobuf/serializers.ts | 6 +- .../otlp-transformer/test/metrics.test.ts | 12 +- 6 files changed, 28 insertions(+), 140 deletions(-) delete mode 100644 experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/common/OTLPMetricExporterBase.test.ts diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index a439362565f..7cbd5bd50a9 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -62,6 +62,9 @@ All notable changes to experimental packages in this project will be documented * This type is scheduled for removal in a future version of this package, please treat all exporters as `SpanExporter`, `PushMetricExporter` or `LogRecordExporter`, based on their respective type. * feat(otlp-grpc-exporter-base)!: collapse base classes into one [#5031](https://github.com/open-telemetry/opentelemetry-js/pull/5031) * `OTLPGRPCExporterNodeBase` has been removed in favor of a platform-agnostic implementation (`OTLPExporterBase` from `@opentelemetry/otlp-exporter-base`) +* feat(otlp-transformer)!: accept `ResourceMetrics` in serializers instead of `ResourceMetrics[]` + * (user-facing): `ProtobufMetricsSerializer` now only accepts `ResourceMetrics` instead of `ResourceMetrics[]` to align with `PushMetricExporter` requirements + * (user-facing): `JsonMetricsSerializer` now only accepts `ResourceMetrics` instead of `ResourceMetrics[]` to align with `PushMetricExporter` requirements ### :rocket: (Enhancement) diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterBase.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterBase.ts index 8e66fa430ed..33880a4fdaa 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterBase.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/src/OTLPMetricExporterBase.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { ExportResult, getEnv } from '@opentelemetry/core'; +import { getEnv } from '@opentelemetry/core'; import { AggregationTemporality, AggregationTemporalitySelector, @@ -28,7 +28,10 @@ import { AggregationTemporalityPreference, OTLPMetricExporterOptions, } from './OTLPMetricExporterOptions'; -import { IOtlpExportDelegate } from '@opentelemetry/otlp-exporter-base'; +import { + IOtlpExportDelegate, + OTLPExporterBase, +} from '@opentelemetry/otlp-exporter-base'; import { diag } from '@opentelemetry/api'; export const CumulativeTemporalitySelector: AggregationTemporalitySelector = @@ -117,37 +120,24 @@ function chooseAggregationSelector( } } -export class OTLPMetricExporterBase implements PushMetricExporter { - public _delegate: IOtlpExportDelegate; - private _aggregationTemporalitySelector: AggregationTemporalitySelector; - private _aggregationSelector: AggregationSelector; +export class OTLPMetricExporterBase + extends OTLPExporterBase + implements PushMetricExporter +{ + private readonly _aggregationTemporalitySelector: AggregationTemporalitySelector; + private readonly _aggregationSelector: AggregationSelector; constructor( - delegate: IOtlpExportDelegate, + delegate: IOtlpExportDelegate, config?: OTLPMetricExporterOptions ) { - this._delegate = delegate; + super(delegate); this._aggregationSelector = chooseAggregationSelector(config); this._aggregationTemporalitySelector = chooseTemporalitySelector( config?.temporalityPreference ); } - export( - metrics: ResourceMetrics, - resultCallback: (result: ExportResult) => void - ): void { - this._delegate.export([metrics], resultCallback); - } - - async shutdown(): Promise { - await this._delegate.shutdown(); - } - - forceFlush(): Promise { - return this._delegate.forceFlush(); - } - selectAggregation(instrumentType: InstrumentType): Aggregation { return this._aggregationSelector(instrumentType); } diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/common/OTLPMetricExporterBase.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/common/OTLPMetricExporterBase.test.ts deleted file mode 100644 index 073446ef70b..00000000000 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/common/OTLPMetricExporterBase.test.ts +++ /dev/null @@ -1,105 +0,0 @@ -/* - * 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 * as sinon from 'sinon'; -import { IOtlpExportDelegate } from '@opentelemetry/otlp-exporter-base'; -import { OTLPMetricExporterBase } from '../../src/OTLPMetricExporterBase'; -import { ResourceMetrics } from '@opentelemetry/sdk-metrics'; -import { Resource } from '@opentelemetry/resources'; - -describe('OTLPMetricExporterBase', function () { - describe('shutdown', function () { - it('calls delegate shutdown', async function () { - // arrange - const exportStub = sinon.stub(); - const forceFlushStub = sinon.stub(); - const shutdownStub = sinon.stub(); - const delegateStubs: IOtlpExportDelegate = { - export: exportStub, - forceFlush: forceFlushStub, - shutdown: shutdownStub, - }; - const exporterBase = new OTLPMetricExporterBase(delegateStubs); - - // act - await exporterBase.shutdown(); - - // assert - sinon.assert.calledOnce(shutdownStub); - // any extra calls on delegate should be handled by the delegate - sinon.assert.notCalled(exportStub); - sinon.assert.notCalled(forceFlushStub); - }); - }); - - describe('forceFlush', function () { - it('calls delegate forceFlush', async function () { - // arrange - const exportStub = sinon.stub(); - const forceFlushStub = sinon.stub(); - const shutdownStub = sinon.stub(); - const delegateStubs: IOtlpExportDelegate = { - export: exportStub, - forceFlush: forceFlushStub, - shutdown: shutdownStub, - }; - const exporterBase = new OTLPMetricExporterBase(delegateStubs); - - // act - await exporterBase.forceFlush(); - - // assert - sinon.assert.calledOnce(forceFlushStub); - // any extra calls on delegate should be handled by the delegate - sinon.assert.notCalled(exportStub); - sinon.assert.notCalled(shutdownStub); - }); - }); - - describe('export', function () { - it('calls delegate export', function () { - // arrange - const exportStub = sinon.stub(); - const forceFlushStub = sinon.stub(); - const shutdownStub = sinon.stub(); - const delegateStubs: IOtlpExportDelegate = { - export: exportStub, - forceFlush: forceFlushStub, - shutdown: shutdownStub, - }; - const exporterBase = new OTLPMetricExporterBase(delegateStubs); - const expectedExportItem: ResourceMetrics = { - resource: Resource.empty(), - scopeMetrics: [], - }; - const expectedCallback = sinon.stub(); - - // act - exporterBase.export(expectedExportItem, expectedCallback); - - // assert - sinon.assert.calledOnceWithExactly( - exportStub, - [expectedExportItem], - expectedCallback - ); - // should not do anything with the callback, any interaction with it should happen on the delegate - sinon.assert.notCalled(expectedCallback); - // any extra calls on delegate should be handled by the delegate - sinon.assert.notCalled(forceFlushStub); - sinon.assert.notCalled(shutdownStub); - }); - }); -}); diff --git a/experimental/packages/otlp-transformer/src/json/serializers.ts b/experimental/packages/otlp-transformer/src/json/serializers.ts index 5d360faf47f..d21070d98ce 100644 --- a/experimental/packages/otlp-transformer/src/json/serializers.ts +++ b/experimental/packages/otlp-transformer/src/json/serializers.ts @@ -43,11 +43,11 @@ export const JsonTraceSerializer: ISerializer< }; export const JsonMetricsSerializer: ISerializer< - ResourceMetrics[], + ResourceMetrics, IExportMetricsServiceResponse > = { - serializeRequest: (arg: ResourceMetrics[]) => { - const request = createExportMetricsServiceRequest(arg, { + serializeRequest: (arg: ResourceMetrics) => { + const request = createExportMetricsServiceRequest([arg], { useLongBits: false, }); const encoder = new TextEncoder(); diff --git a/experimental/packages/otlp-transformer/src/protobuf/serializers.ts b/experimental/packages/otlp-transformer/src/protobuf/serializers.ts index ce2d0d94098..ff73ab67f22 100644 --- a/experimental/packages/otlp-transformer/src/protobuf/serializers.ts +++ b/experimental/packages/otlp-transformer/src/protobuf/serializers.ts @@ -68,11 +68,11 @@ export const ProtobufLogsSerializer: ISerializer< }; export const ProtobufMetricsSerializer: ISerializer< - ResourceMetrics[], + ResourceMetrics, IExportMetricsServiceResponse > = { - serializeRequest: (arg: ResourceMetrics[]) => { - const request = createExportMetricsServiceRequest(arg); + serializeRequest: (arg: ResourceMetrics) => { + const request = createExportMetricsServiceRequest([arg]); return metricsRequestType.encode(request).finish(); }, deserializeResponse: (arg: Uint8Array) => { diff --git a/experimental/packages/otlp-transformer/test/metrics.test.ts b/experimental/packages/otlp-transformer/test/metrics.test.ts index 54d67cde86f..06d357102ef 100644 --- a/experimental/packages/otlp-transformer/test/metrics.test.ts +++ b/experimental/packages/otlp-transformer/test/metrics.test.ts @@ -788,11 +788,11 @@ describe('Metrics', () => { describe('ProtobufMetricsSerializer', function () { it('serializes an export request', () => { - const serialized = ProtobufMetricsSerializer.serializeRequest([ + const serialized = ProtobufMetricsSerializer.serializeRequest( createResourceMetrics([ createCounterData(10, AggregationTemporality.DELTA), - ]), - ]); + ]) + ); assert.ok(serialized, 'serialized response is undefined'); const decoded = root.opentelemetry.proto.collector.metrics.v1.ExportMetricsServiceRequest.decode( @@ -873,11 +873,11 @@ describe('Metrics', () => { describe('JsonMetricsSerializer', function () { it('serializes an export request', () => { - const serialized = JsonMetricsSerializer.serializeRequest([ + const serialized = JsonMetricsSerializer.serializeRequest( createResourceMetrics([ createCounterData(10, AggregationTemporality.DELTA), - ]), - ]); + ]) + ); const decoder = new TextDecoder(); const expected = { From 3215551757e9a7f47f860db20cc99d11e8ed304f Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Tue, 19 Nov 2024 15:14:42 +0100 Subject: [PATCH 2/2] fixup! feat(otlp-transformer)!: accept ResourceMetrics instead of ResoruceMetrics[] in metrics serializers --- .../test/browser/index-webpack.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/browser/index-webpack.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/browser/index-webpack.ts index 99100a0f6ee..ae7d4b5a9dd 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/browser/index-webpack.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-http/test/browser/index-webpack.ts @@ -16,8 +16,5 @@ const testsContext = require.context('../browser', true, /test$/); testsContext.keys().forEach(testsContext); -const testsContextCommon = require.context('../common', true, /test$/); -testsContextCommon.keys().forEach(testsContextCommon); - const srcContext = require.context('.', true, /src$/); srcContext.keys().forEach(srcContext);