From 462cad61a93cd0f4118ad727445ff08ea92f98da Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Thu, 18 Jan 2024 16:50:39 +0100 Subject: [PATCH 01/13] fix(exporter-*-otlp-grpc)!: lazy load gRPC --- .../src/OTLPLogExporter.ts | 32 ++-- .../test/OTLPLogExporter.test.ts | 33 ++-- .../src/OTLPTraceExporter.ts | 32 ++-- .../test/OTLPTraceExporter.test.ts | 37 ++-- .../src/OTLPMetricExporter.ts | 30 ++- .../test/OTLPMetricExporter.test.ts | 58 +++--- .../src/LogsExportServiceClient.ts | 53 ------ .../src/MetricsExportServiceClient.ts | 56 ------ .../src/OTLPGRPCExporterNodeBase.ts | 152 +++++++++------ .../src/TraceExportServiceClient.ts | 56 ------ .../src/create-service-client-constructor.ts | 51 +++++ .../src/grpc-exporter-transport.ts | 166 +++++++++++++++++ .../otlp-grpc-exporter-base/src/index.ts | 14 +- .../src/serializers.ts | 88 +++++++++ .../otlp-grpc-exporter-base/src/types.ts | 27 +-- .../otlp-grpc-exporter-base/src/util.ts | 176 ++++-------------- .../test/OTLPGRPCExporterNodeBase.test.ts | 143 -------------- .../otlp-grpc-exporter-base/test/util.test.ts | 39 ++-- 18 files changed, 580 insertions(+), 663 deletions(-) delete mode 100644 experimental/packages/otlp-grpc-exporter-base/src/LogsExportServiceClient.ts delete mode 100644 experimental/packages/otlp-grpc-exporter-base/src/MetricsExportServiceClient.ts delete mode 100644 experimental/packages/otlp-grpc-exporter-base/src/TraceExportServiceClient.ts create mode 100644 experimental/packages/otlp-grpc-exporter-base/src/create-service-client-constructor.ts create mode 100644 experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts create mode 100644 experimental/packages/otlp-grpc-exporter-base/src/serializers.ts diff --git a/experimental/packages/exporter-logs-otlp-grpc/src/OTLPLogExporter.ts b/experimental/packages/exporter-logs-otlp-grpc/src/OTLPLogExporter.ts index 675ce83db0a..b134619043f 100644 --- a/experimental/packages/exporter-logs-otlp-grpc/src/OTLPLogExporter.ts +++ b/experimental/packages/exporter-logs-otlp-grpc/src/OTLPLogExporter.ts @@ -16,35 +16,41 @@ import { LogRecordExporter, ReadableLogRecord } from '@opentelemetry/sdk-logs'; import { baggageUtils, getEnv } from '@opentelemetry/core'; -import { Metadata } from '@grpc/grpc-js'; import { OTLPGRPCExporterConfigNode, OTLPGRPCExporterNodeBase, - ServiceClientType, validateAndNormalizeUrl, DEFAULT_COLLECTOR_URL, + LogsSerializer, } from '@opentelemetry/otlp-grpc-exporter-base'; import { createExportLogsServiceRequest, IExportLogsServiceRequest, + IExportLogsServiceResponse, } from '@opentelemetry/otlp-transformer'; /** * OTLP Logs Exporter for Node */ export class OTLPLogExporter - extends OTLPGRPCExporterNodeBase + extends OTLPGRPCExporterNodeBase< + ReadableLogRecord, + IExportLogsServiceRequest, + IExportLogsServiceResponse + > implements LogRecordExporter { constructor(config: OTLPGRPCExporterConfigNode = {}) { - super(config); - const headers = baggageUtils.parseKeyPairsIntoRecord( + const signalSpecificMetadata = baggageUtils.parseKeyPairsIntoRecord( getEnv().OTEL_EXPORTER_OTLP_LOGS_HEADERS ); - this.metadata ||= new Metadata(); - for (const [k, v] of Object.entries(headers)) { - this.metadata.set(k, v); - } + super( + config, + signalSpecificMetadata, + 'LogsExportService', + '/opentelemetry.proto.collector.logs.v1.LogsService/Export', + LogsSerializer + ); } convert(logRecords: ReadableLogRecord[]): IExportLogsServiceRequest { @@ -55,14 +61,6 @@ export class OTLPLogExporter return validateAndNormalizeUrl(this.getUrlFromConfig(config)); } - getServiceClientType() { - return ServiceClientType.LOGS; - } - - getServiceProtoPath(): string { - return 'opentelemetry/proto/collector/logs/v1/logs_service.proto'; - } - getUrlFromConfig(config: OTLPGRPCExporterConfigNode): string { if (typeof config.url === 'string') { return config.url; diff --git a/experimental/packages/exporter-logs-otlp-grpc/test/OTLPLogExporter.test.ts b/experimental/packages/exporter-logs-otlp-grpc/test/OTLPLogExporter.test.ts index 9bda910924c..0368356555e 100644 --- a/experimental/packages/exporter-logs-otlp-grpc/test/OTLPLogExporter.test.ts +++ b/experimental/packages/exporter-logs-otlp-grpc/test/OTLPLogExporter.test.ts @@ -32,7 +32,6 @@ import { } from './logsHelper'; import * as core from '@opentelemetry/core'; import { CompressionAlgorithm } from '@opentelemetry/otlp-exporter-base'; -import { GrpcCompressionAlgorithm } from '@opentelemetry/otlp-grpc-exporter-base'; import { IExportLogsServiceRequest, IResourceLogs, @@ -291,7 +290,7 @@ const testCollectorExporter = (params: TestParams) => { }); assert.strictEqual( collectorExporter.compression, - GrpcCompressionAlgorithm.GZIP + CompressionAlgorithm.GZIP ); delete envSource.OTEL_EXPORTER_OTLP_COMPRESSION; }); @@ -319,38 +318,46 @@ describe('OTLPLogExporter - node (getDefaultUrl)', () => { describe('when configuring via environment', () => { const envSource = process.env; + + afterEach(function () { + // Ensure we don't pollute other tests if assertions fail + delete envSource.OTEL_EXPORTER_OTLP_ENDPOINT; + delete envSource.OTEL_EXPORTER_OTLP_LOGS_ENDPOINT; + delete envSource.OTEL_EXPORTER_OTLP_HEADERS; + delete envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS; + sinon.restore(); + }); + it('should use url defined in env', () => { envSource.OTEL_EXPORTER_OTLP_ENDPOINT = 'http://foo.bar'; const collectorExporter = new OTLPLogExporter(); assert.strictEqual(collectorExporter.url, 'foo.bar'); - envSource.OTEL_EXPORTER_OTLP_ENDPOINT = ''; }); it('should override global exporter url with signal url defined in env', () => { envSource.OTEL_EXPORTER_OTLP_ENDPOINT = 'http://foo.bar'; envSource.OTEL_EXPORTER_OTLP_LOGS_ENDPOINT = 'http://foo.logs'; const collectorExporter = new OTLPLogExporter(); assert.strictEqual(collectorExporter.url, 'foo.logs'); - envSource.OTEL_EXPORTER_OTLP_ENDPOINT = ''; - envSource.OTEL_EXPORTER_OTLP_LOGS_ENDPOINT = ''; }); it('should use headers defined via env', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar'; const collectorExporter = new OTLPLogExporter(); - assert.deepStrictEqual(collectorExporter.metadata?.get('foo'), ['bar']); - envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; + const actualMetadata = + collectorExporter['_transport']['_parameters'].metadata(); + assert.deepStrictEqual(actualMetadata.get('foo'), ['bar']); }); - it('should override global headers config with signal headers defined via env', () => { + it('should not override hard-coded headers config with headers defined via env', () => { const metadata = new grpc.Metadata(); metadata.set('foo', 'bar'); metadata.set('goo', 'lol'); envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=jar,bar=foo'; envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS = 'foo=boo'; const collectorExporter = new OTLPLogExporter({ metadata }); - assert.deepStrictEqual(collectorExporter.metadata?.get('foo'), ['boo']); - assert.deepStrictEqual(collectorExporter.metadata?.get('bar'), ['foo']); - assert.deepStrictEqual(collectorExporter.metadata?.get('goo'), ['lol']); - envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS = ''; - envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; + const actualMetadata = + collectorExporter['_transport']['_parameters'].metadata(); + assert.deepStrictEqual(actualMetadata.get('foo'), ['bar']); + assert.deepStrictEqual(actualMetadata.get('goo'), ['lol']); + assert.deepStrictEqual(actualMetadata.get('bar'), ['foo']); }); }); diff --git a/experimental/packages/exporter-trace-otlp-grpc/src/OTLPTraceExporter.ts b/experimental/packages/exporter-trace-otlp-grpc/src/OTLPTraceExporter.ts index c99826a176f..88e55734e6e 100644 --- a/experimental/packages/exporter-trace-otlp-grpc/src/OTLPTraceExporter.ts +++ b/experimental/packages/exporter-trace-otlp-grpc/src/OTLPTraceExporter.ts @@ -16,17 +16,17 @@ import { ReadableSpan, SpanExporter } from '@opentelemetry/sdk-trace-base'; import { baggageUtils, getEnv } from '@opentelemetry/core'; -import { Metadata } from '@grpc/grpc-js'; import { OTLPGRPCExporterConfigNode, OTLPGRPCExporterNodeBase, - ServiceClientType, validateAndNormalizeUrl, DEFAULT_COLLECTOR_URL, + TraceSerializer, } from '@opentelemetry/otlp-grpc-exporter-base'; import { createExportTraceServiceRequest, IExportTraceServiceRequest, + IExportTraceServiceResponse, } from '@opentelemetry/otlp-transformer'; import { VERSION } from './version'; @@ -38,21 +38,27 @@ const USER_AGENT = { * OTLP Trace Exporter for Node */ export class OTLPTraceExporter - extends OTLPGRPCExporterNodeBase + extends OTLPGRPCExporterNodeBase< + ReadableSpan, + IExportTraceServiceRequest, + IExportTraceServiceResponse + > implements SpanExporter { constructor(config: OTLPGRPCExporterConfigNode = {}) { - super(config); - const headers = { + const signalSpecificMetadata = { ...USER_AGENT, ...baggageUtils.parseKeyPairsIntoRecord( getEnv().OTEL_EXPORTER_OTLP_TRACES_HEADERS ), }; - this.metadata ||= new Metadata(); - for (const [k, v] of Object.entries(headers)) { - this.metadata.set(k, v); - } + super( + config, + signalSpecificMetadata, + 'TraceExportService', + '/opentelemetry.proto.collector.trace.v1.TraceService/Export', + TraceSerializer + ); } convert(spans: ReadableSpan[]): IExportTraceServiceRequest { @@ -63,14 +69,6 @@ export class OTLPTraceExporter return validateAndNormalizeUrl(this.getUrlFromConfig(config)); } - getServiceClientType() { - return ServiceClientType.SPANS; - } - - getServiceProtoPath(): string { - return 'opentelemetry/proto/collector/trace/v1/trace_service.proto'; - } - getUrlFromConfig(config: OTLPGRPCExporterConfigNode): string { if (typeof config.url === 'string') { return config.url; diff --git a/experimental/packages/exporter-trace-otlp-grpc/test/OTLPTraceExporter.test.ts b/experimental/packages/exporter-trace-otlp-grpc/test/OTLPTraceExporter.test.ts index f22fc95d2ad..9a3a13ce322 100644 --- a/experimental/packages/exporter-trace-otlp-grpc/test/OTLPTraceExporter.test.ts +++ b/experimental/packages/exporter-trace-otlp-grpc/test/OTLPTraceExporter.test.ts @@ -37,7 +37,6 @@ import { } from './traceHelper'; import * as core from '@opentelemetry/core'; import { CompressionAlgorithm } from '@opentelemetry/otlp-exporter-base'; -import { GrpcCompressionAlgorithm } from '@opentelemetry/otlp-grpc-exporter-base'; import { IExportTraceServiceRequest, IResourceSpans, @@ -302,7 +301,7 @@ const testCollectorExporter = (params: TestParams) => { }); assert.strictEqual( collectorExporter.compression, - GrpcCompressionAlgorithm.GZIP + CompressionAlgorithm.GZIP ); delete envSource.OTEL_EXPORTER_OTLP_COMPRESSION; }); @@ -330,44 +329,54 @@ describe('OTLPTraceExporter - node (getDefaultUrl)', () => { describe('when configuring via environment', () => { const envSource = process.env; + + afterEach(function () { + // Ensure we don't pollute other tests if assertions fail + delete envSource.OTEL_EXPORTER_OTLP_ENDPOINT; + delete envSource.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT; + delete envSource.OTEL_EXPORTER_OTLP_HEADERS; + delete envSource.OTEL_EXPORTER_OTLP_TRACES_HEADERS; + sinon.restore(); + }); + it('should use url defined in env', () => { envSource.OTEL_EXPORTER_OTLP_ENDPOINT = 'http://foo.bar'; const collectorExporter = new OTLPTraceExporter(); assert.strictEqual(collectorExporter.url, 'foo.bar'); - envSource.OTEL_EXPORTER_OTLP_ENDPOINT = ''; }); it('should override global exporter url with signal url defined in env', () => { envSource.OTEL_EXPORTER_OTLP_ENDPOINT = 'http://foo.bar'; envSource.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT = 'http://foo.traces'; const collectorExporter = new OTLPTraceExporter(); assert.strictEqual(collectorExporter.url, 'foo.traces'); - envSource.OTEL_EXPORTER_OTLP_ENDPOINT = ''; - envSource.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT = ''; }); it('should use headers defined via env', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar'; const collectorExporter = new OTLPTraceExporter(); - assert.deepStrictEqual(collectorExporter.metadata?.get('foo'), ['bar']); - envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; + const actualMetadata = + collectorExporter['_transport']['_parameters'].metadata(); + assert.deepStrictEqual(actualMetadata.get('foo'), ['bar']); }); it('should include user agent in header', () => { const collectorExporter = new OTLPTraceExporter(); - assert.deepStrictEqual(collectorExporter.metadata?.get('User-Agent'), [ + const actualMetadata = + collectorExporter['_transport']['_parameters'].metadata(); + assert.deepStrictEqual(actualMetadata.get('User-Agent'), [ `OTel-OTLP-Exporter-JavaScript/${VERSION}`, ]); }); - it('should override global headers config with signal headers defined via env', () => { + it('should not override hard-coded headers config with headers defined via env', () => { const metadata = new grpc.Metadata(); metadata.set('foo', 'bar'); metadata.set('goo', 'lol'); envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=jar,bar=foo'; envSource.OTEL_EXPORTER_OTLP_TRACES_HEADERS = 'foo=boo'; const collectorExporter = new OTLPTraceExporter({ metadata }); - assert.deepStrictEqual(collectorExporter.metadata?.get('foo'), ['boo']); - assert.deepStrictEqual(collectorExporter.metadata?.get('bar'), ['foo']); - assert.deepStrictEqual(collectorExporter.metadata?.get('goo'), ['lol']); - envSource.OTEL_EXPORTER_OTLP_TRACES_HEADERS = ''; - envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; + const actualMetadata = + collectorExporter['_transport']['_parameters'].metadata(); + assert.deepStrictEqual(actualMetadata.get('foo'), ['bar']); + assert.deepStrictEqual(actualMetadata.get('goo'), ['lol']); + assert.deepStrictEqual(actualMetadata.get('bar'), ['foo']); }); }); diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/src/OTLPMetricExporter.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/src/OTLPMetricExporter.ts index b1b12272750..77d68d60a67 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/src/OTLPMetricExporter.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/src/OTLPMetricExporter.ts @@ -22,15 +22,15 @@ import { ResourceMetrics } from '@opentelemetry/sdk-metrics'; import { OTLPGRPCExporterConfigNode, OTLPGRPCExporterNodeBase, - ServiceClientType, validateAndNormalizeUrl, DEFAULT_COLLECTOR_URL, + MetricsSerializer, } from '@opentelemetry/otlp-grpc-exporter-base'; import { baggageUtils, getEnv } from '@opentelemetry/core'; -import { Metadata } from '@grpc/grpc-js'; import { createExportMetricsServiceRequest, IExportMetricsServiceRequest, + IExportMetricsServiceResponse, } from '@opentelemetry/otlp-transformer'; import { VERSION } from './version'; @@ -40,30 +40,24 @@ const USER_AGENT = { class OTLPMetricExporterProxy extends OTLPGRPCExporterNodeBase< ResourceMetrics, - IExportMetricsServiceRequest + IExportMetricsServiceRequest, + IExportMetricsServiceResponse > { constructor(config?: OTLPGRPCExporterConfigNode & OTLPMetricExporterOptions) { - super(config); - const headers = { + const signalSpecificMetadata = { ...USER_AGENT, ...baggageUtils.parseKeyPairsIntoRecord( getEnv().OTEL_EXPORTER_OTLP_METRICS_HEADERS ), ...config?.headers, }; - - this.metadata ||= new Metadata(); - for (const [k, v] of Object.entries(headers)) { - this.metadata.set(k, v); - } - } - - getServiceProtoPath(): string { - return 'opentelemetry/proto/collector/metrics/v1/metrics_service.proto'; - } - - getServiceClientType(): ServiceClientType { - return ServiceClientType.METRICS; + super( + config, + signalSpecificMetadata, + 'MetricsExportService', + '/opentelemetry.proto.collector.metrics.v1.MetricsService/Export', + MetricsSerializer + ); } getDefaultUrl(config: OTLPGRPCExporterConfigNode): string { diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/OTLPMetricExporter.test.ts b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/OTLPMetricExporter.test.ts index 79a6125a095..3ed034bde89 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/OTLPMetricExporter.test.ts +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/test/OTLPMetricExporter.test.ts @@ -307,6 +307,15 @@ describe('OTLPMetricExporter - node (getDefaultUrl)', () => { }); describe('when configuring via environment', () => { + afterEach(function () { + // Ensure we don't pollute other tests if assertions fail + delete envSource.OTEL_EXPORTER_OTLP_ENDPOINT; + delete envSource.OTEL_EXPORTER_OTLP_METRICS_ENDPOINT; + delete envSource.OTEL_EXPORTER_OTLP_HEADERS; + delete envSource.OTEL_EXPORTER_OTLP_METRICS_HEADERS; + sinon.restore(); + }); + const envSource = process.env; it('should use url defined in env', () => { envSource.OTEL_EXPORTER_OTLP_ENDPOINT = 'http://foo.bar'; @@ -334,20 +343,20 @@ describe('when configuring via environment', () => { it('should use headers defined via env', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar'; const collectorExporter = new OTLPMetricExporter(); - assert.deepStrictEqual( - collectorExporter._otlpExporter.metadata?.get('foo'), - ['bar'] - ); + const actualMetadata = + collectorExporter._otlpExporter['_transport']['_parameters'].metadata(); + assert.deepStrictEqual(actualMetadata.get('foo'), ['bar']); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); it('should include user agent in header', () => { const collectorExporter = new OTLPMetricExporter(); - assert.deepStrictEqual( - collectorExporter._otlpExporter.metadata?.get('User-Agent'), - [`OTel-OTLP-Exporter-JavaScript/${VERSION}`] - ); + const actualMetadata = + collectorExporter._otlpExporter['_transport']['_parameters'].metadata(); + assert.deepStrictEqual(actualMetadata.get('User-Agent'), [ + `OTel-OTLP-Exporter-JavaScript/${VERSION}`, + ]); }); - it('should override global headers config with signal headers defined via env', () => { + it('should not override hard-coded headers config with headers defined via env', () => { const metadata = new grpc.Metadata(); metadata.set('foo', 'bar'); metadata.set('goo', 'lol'); @@ -357,21 +366,15 @@ describe('when configuring via environment', () => { metadata, temporalityPreference: AggregationTemporalityPreference.CUMULATIVE, }); - assert.deepStrictEqual( - collectorExporter._otlpExporter.metadata?.get('foo'), - ['boo'] - ); - assert.deepStrictEqual( - collectorExporter._otlpExporter.metadata?.get('bar'), - ['foo'] - ); - assert.deepStrictEqual( - collectorExporter._otlpExporter.metadata?.get('goo'), - ['lol'] - ); + const actualMetadata = + collectorExporter._otlpExporter['_transport']['_parameters'].metadata(); + assert.deepStrictEqual(actualMetadata.get('foo'), ['bar']); + assert.deepStrictEqual(actualMetadata.get('bar'), ['foo']); + assert.deepStrictEqual(actualMetadata.get('goo'), ['lol']); envSource.OTEL_EXPORTER_OTLP_METRICS_HEADERS = ''; envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); + it('should override headers defined via env with headers defined in constructor', () => { envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar,bar=foo'; const collectorExporter = new OTLPMetricExporter({ @@ -379,14 +382,11 @@ describe('when configuring via environment', () => { foo: 'constructor', }, }); - assert.deepStrictEqual( - collectorExporter._otlpExporter.metadata?.get('foo'), - ['constructor'] - ); - assert.deepStrictEqual( - collectorExporter._otlpExporter.metadata?.get('bar'), - ['foo'] - ); + + const actualMetadata = + collectorExporter._otlpExporter['_transport']['_parameters'].metadata(); + assert.deepStrictEqual(actualMetadata.get('foo'), ['constructor']); + assert.deepStrictEqual(actualMetadata.get('bar'), ['foo']); envSource.OTEL_EXPORTER_OTLP_HEADERS = ''; }); }); diff --git a/experimental/packages/otlp-grpc-exporter-base/src/LogsExportServiceClient.ts b/experimental/packages/otlp-grpc-exporter-base/src/LogsExportServiceClient.ts deleted file mode 100644 index b867743cea7..00000000000 --- a/experimental/packages/otlp-grpc-exporter-base/src/LogsExportServiceClient.ts +++ /dev/null @@ -1,53 +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 root from './generated/root'; -import * as grpc from '@grpc/grpc-js'; -import { - IExportLogsServiceRequest, - IExportLogsServiceResponse, -} from '@opentelemetry/otlp-transformer'; -import { ExportType } from './internal-types'; - -const responseType = root.opentelemetry.proto.collector.logs.v1 - .ExportLogsServiceResponse as ExportType; - -const requestType = root.opentelemetry.proto.collector.logs.v1 - .ExportLogsServiceRequest as ExportType; - -const logsServiceDefinition = { - export: { - path: '/opentelemetry.proto.collector.logs.v1.LogsService/Export', - requestStream: false, - responseStream: false, - requestSerialize: (arg: IExportLogsServiceRequest) => { - return Buffer.from(requestType.encode(arg).finish()); - }, - requestDeserialize: (arg: Buffer) => { - return requestType.decode(arg); - }, - responseSerialize: (arg: IExportLogsServiceResponse) => { - return Buffer.from(responseType.encode(arg).finish()); - }, - responseDeserialize: (arg: Buffer) => { - return responseType.decode(arg); - }, - }, -}; - -// Creates a new instance of a gRPC service client for OTLP logs -export const LogsExportServiceClient: grpc.ServiceClientConstructor = - grpc.makeGenericClientConstructor(logsServiceDefinition, 'LogsExportService'); diff --git a/experimental/packages/otlp-grpc-exporter-base/src/MetricsExportServiceClient.ts b/experimental/packages/otlp-grpc-exporter-base/src/MetricsExportServiceClient.ts deleted file mode 100644 index 7f81be6087d..00000000000 --- a/experimental/packages/otlp-grpc-exporter-base/src/MetricsExportServiceClient.ts +++ /dev/null @@ -1,56 +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 root from './generated/root'; -import * as grpc from '@grpc/grpc-js'; -import { - IExportMetricsServiceRequest, - IExportMetricsServiceResponse, -} from '@opentelemetry/otlp-transformer'; -import { ExportType } from './internal-types'; - -const responseType = root.opentelemetry.proto.collector.metrics.v1 - .ExportMetricsServiceResponse as ExportType; - -const requestType = root.opentelemetry.proto.collector.metrics.v1 - .ExportMetricsServiceRequest as ExportType; - -const metricsServiceDefinition = { - export: { - path: '/opentelemetry.proto.collector.metrics.v1.MetricsService/Export', - requestStream: false, - responseStream: false, - requestSerialize: (arg: IExportMetricsServiceRequest) => { - return Buffer.from(requestType.encode(arg).finish()); - }, - requestDeserialize: (arg: Buffer) => { - return requestType.decode(arg); - }, - responseSerialize: (arg: IExportMetricsServiceResponse) => { - return Buffer.from(responseType.encode(arg).finish()); - }, - responseDeserialize: (arg: Buffer) => { - return responseType.decode(arg); - }, - }, -}; - -// Creates a new instance of a gRPC service client for OTLP metrics -export const MetricExportServiceClient: grpc.ServiceClientConstructor = - grpc.makeGenericClientConstructor( - metricsServiceDefinition, - 'MetricsExportService' - ); diff --git a/experimental/packages/otlp-grpc-exporter-base/src/OTLPGRPCExporterNodeBase.ts b/experimental/packages/otlp-grpc-exporter-base/src/OTLPGRPCExporterNodeBase.ts index b4f06472c61..f5f4c2144fd 100644 --- a/experimental/packages/otlp-grpc-exporter-base/src/OTLPGRPCExporterNodeBase.ts +++ b/experimental/packages/otlp-grpc-exporter-base/src/OTLPGRPCExporterNodeBase.ts @@ -15,19 +15,19 @@ */ import { diag } from '@opentelemetry/api'; -import { Metadata } from '@grpc/grpc-js'; -import { - OTLPGRPCExporterConfigNode, - GRPCQueueItem, - ServiceClientType, -} from './types'; -import { ServiceClient } from './types'; -import { getEnv, baggageUtils } from '@opentelemetry/core'; -import { configureCompression, GrpcCompressionAlgorithm } from './util'; +import { GRPCQueueItem, OTLPGRPCExporterConfigNode } from './types'; +import { baggageUtils, getEnv } from '@opentelemetry/core'; import { + CompressionAlgorithm, OTLPExporterBase, OTLPExporterError, } from '@opentelemetry/otlp-exporter-base'; +import { + createEmptyMetadata, + GrpcExporterTransport, +} from './grpc-exporter-transport'; +import { configureCompression, configureCredentials } from './util'; +import { ISerializer } from './serializers'; /** * OTLP Exporter abstract base class @@ -35,59 +35,84 @@ import { export abstract class OTLPGRPCExporterNodeBase< ExportItem, ServiceRequest, + ServiceResponse, > extends OTLPExporterBase< OTLPGRPCExporterConfigNode, ExportItem, ServiceRequest > { grpcQueue: GRPCQueueItem[] = []; - metadata?: Metadata; - serviceClient?: ServiceClient = undefined; - private _send!: Function; - compression: GrpcCompressionAlgorithm; + compression: CompressionAlgorithm; + private _transport: GrpcExporterTransport; + private _serializer: ISerializer; - constructor(config: OTLPGRPCExporterConfigNode = {}) { + constructor( + config: OTLPGRPCExporterConfigNode = {}, + signalSpecificMetadata: Record, + grpcName: string, + grpcPath: string, + serializer: ISerializer + ) { super(config); + this._serializer = serializer; if (config.headers) { diag.warn('Headers cannot be set when using grpc'); } - const headers = baggageUtils.parseKeyPairsIntoRecord( + const nonSignalSpecificMetadata = baggageUtils.parseKeyPairsIntoRecord( getEnv().OTEL_EXPORTER_OTLP_HEADERS ); - this.metadata = config.metadata || new Metadata(); - for (const [k, v] of Object.entries(headers)) { - this.metadata.set(k, v); + const rawMetadata = Object.assign( + {}, + nonSignalSpecificMetadata, + signalSpecificMetadata + ); + + let credentialProvider = () => { + return configureCredentials(undefined, this.getUrlFromConfig(config)); + }; + + if (config.credentials != null) { + const credentials = config.credentials; + credentialProvider = () => { + return credentials; + }; } - this.compression = configureCompression(config.compression); - } - private _sendPromise( - objects: ExportItem[], - onSuccess: () => void, - onError: (error: OTLPExporterError) => void - ): void { - const promise = new Promise((resolve, reject) => { - this._send(this, objects, resolve, reject); - }).then(onSuccess, onError); + // Ensure we don't modify the original. + const configMetadata = config.metadata?.clone(); + const metadataProvider = () => { + const metadata = configMetadata ?? createEmptyMetadata(); + for (const [key, value] of Object.entries(rawMetadata)) { + // only override with env var data if the key has no values. + // not using Metadata.merge() as it will keep both values. + if (metadata.get(key).length < 1) { + metadata.set(key, value); + } + } - this._sendingPromises.push(promise); - const popPromise = () => { - const index = this._sendingPromises.indexOf(promise); - this._sendingPromises.splice(index, 1); + return metadata; }; - promise.then(popPromise, popPromise); - } - onInit(config: OTLPGRPCExporterConfigNode): void { - // defer to next tick and lazy load to avoid loading grpc too early - // and making this impossible to be instrumented - setImmediate(() => { - // eslint-disable-next-line @typescript-eslint/no-var-requires - const { onInit } = require('./util'); - onInit(this, config); + this.compression = configureCompression(config.compression); + this._transport = new GrpcExporterTransport({ + address: this.getDefaultUrl(config), + compression: this.compression, + credentials: credentialProvider, + grpcName: grpcName, + grpcPath: grpcPath, + metadata: metadataProvider, + timeoutMillis: this.timeoutMillis, }); } + onInit() { + // Intentionally left empty; nothing to do. + } + + override onShutdown() { + this._transport.shutdown(); + } + send( objects: ExportItem[], onSuccess: () => void, @@ -97,28 +122,35 @@ export abstract class OTLPGRPCExporterNodeBase< diag.debug('Shutdown already started. Cannot send objects'); return; } - if (!this._send) { - // defer to next tick and lazy load to avoid loading grpc too early - // and making this impossible to be instrumented - setImmediate(() => { - // eslint-disable-next-line @typescript-eslint/no-var-requires - const { send } = require('./util'); - this._send = send; - - this._sendPromise(objects, onSuccess, onError); - }); - } else { - this._sendPromise(objects, onSuccess, onError); - } - } - onShutdown(): void { - if (this.serviceClient) { - this.serviceClient.close(); + const converted = this.convert(objects); + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore TODO: no time for type stuff, it will work, trust me (TM) - that's what TM stands for lol. + const data = this._serializer.serializeRequest(converted); + + if (data == null) { + onError(new Error('Could not serialize message')); + return; } + + const promise = this._transport.send(data).then(response => { + if (response.status === 'success') { + onSuccess(); + return; + } + if (response.status === 'failure' && response.error) { + onError(response.error); + } + onError(new OTLPExporterError('Export failed with unknown error')); + }, onError); + + this._sendingPromises.push(promise); + const popPromise = () => { + const index = this._sendingPromises.indexOf(promise); + this._sendingPromises.splice(index, 1); + }; + promise.then(popPromise, popPromise); } - abstract getServiceProtoPath(): string; - abstract getServiceClientType(): ServiceClientType; abstract getUrlFromConfig(config: OTLPGRPCExporterConfigNode): string; } diff --git a/experimental/packages/otlp-grpc-exporter-base/src/TraceExportServiceClient.ts b/experimental/packages/otlp-grpc-exporter-base/src/TraceExportServiceClient.ts deleted file mode 100644 index d332e4f4daa..00000000000 --- a/experimental/packages/otlp-grpc-exporter-base/src/TraceExportServiceClient.ts +++ /dev/null @@ -1,56 +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 root from './generated/root'; -import * as grpc from '@grpc/grpc-js'; -import { - IExportTraceServiceRequest, - IExportTraceServiceResponse, -} from '@opentelemetry/otlp-transformer'; -import { ExportType } from './internal-types'; - -const responseType = root.opentelemetry.proto.collector.trace.v1 - .ExportTraceServiceResponse as ExportType; - -const requestType = root.opentelemetry.proto.collector.trace.v1 - .ExportTraceServiceRequest as ExportType; - -const traceServiceDefinition = { - export: { - path: '/opentelemetry.proto.collector.trace.v1.TraceService/Export', - requestStream: false, - responseStream: false, - requestSerialize: (arg: IExportTraceServiceRequest) => { - return Buffer.from(requestType.encode(arg).finish()); - }, - requestDeserialize: (arg: Buffer) => { - return requestType.decode(arg); - }, - responseSerialize: (arg: IExportTraceServiceResponse) => { - return Buffer.from(responseType.encode(arg).finish()); - }, - responseDeserialize: (arg: Buffer) => { - return responseType.decode(arg); - }, - }, -}; - -// Creates a new instance of a gRPC service client for exporting OTLP traces -export const TraceExportServiceClient: grpc.ServiceClientConstructor = - grpc.makeGenericClientConstructor( - traceServiceDefinition, - 'TraceExportService' - ); diff --git a/experimental/packages/otlp-grpc-exporter-base/src/create-service-client-constructor.ts b/experimental/packages/otlp-grpc-exporter-base/src/create-service-client-constructor.ts new file mode 100644 index 00000000000..9447e7b786b --- /dev/null +++ b/experimental/packages/otlp-grpc-exporter-base/src/create-service-client-constructor.ts @@ -0,0 +1,51 @@ +/* + * 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 grpc from '@grpc/grpc-js'; + +/** + * Creates a unary service client constructor that, when instantiated, does not serialize/deserialize anything. + * Allows for passing in {@link Buffer} directly, serialization can be handled via protobufjs or custom implementations. + * + * @param path service path + * @param name service name + */ +export function createServiceClientConstructor( + path: string, + name: string +): grpc.ServiceClientConstructor { + const serviceDefinition = { + export: { + path: path, + requestStream: false, + responseStream: false, + requestSerialize: (arg: Buffer) => { + return arg; + }, + requestDeserialize: (arg: Buffer) => { + return arg; + }, + responseSerialize: (arg: Buffer) => { + return arg; + }, + responseDeserialize: (arg: Buffer) => { + return arg; + }, + }, + }; + + return grpc.makeGenericClientConstructor(serviceDefinition, name); +} diff --git a/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts b/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts new file mode 100644 index 00000000000..0cd0b5712f7 --- /dev/null +++ b/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts @@ -0,0 +1,166 @@ +/* + * 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. + */ + +// NOTE: do not change these type imports to actual imports. Doing so WILL break `@opentelemetry/instrumentation-http`, +// as they'd be imported before the http/https modules can be wrapped. +import type { Metadata, ServiceError, ChannelCredentials } from '@grpc/grpc-js'; + +export interface ExportResponse { + status: 'success' | 'failure'; + data?: Uint8Array; + retryInMillis?: number; + error?: Error; +} + +const GRPC_COMPRESSION_NONE = 0; +const GRPC_COMPRESSION_GZIP = 2; + +function toGrpcCompression(compression: 'gzip' | 'none'): number { + if (compression === 'none') { + return GRPC_COMPRESSION_NONE; + } else if (compression === 'gzip') { + return GRPC_COMPRESSION_GZIP; + } + return GRPC_COMPRESSION_NONE; +} + +export function createInsecureCredentials(): ChannelCredentials { + const { + credentials, + // eslint-disable-next-line @typescript-eslint/no-var-requires + } = require('@grpc/grpc-js'); + return credentials.createInsecure(); +} + +export function createSslCredentials( + rootCert?: Buffer, + privateKey?: Buffer, + certChain?: Buffer +): ChannelCredentials { + const { + credentials, + // eslint-disable-next-line @typescript-eslint/no-var-requires + } = require('@grpc/grpc-js'); + return credentials.createSsl(rootCert, privateKey, certChain); +} + +export function createEmptyMetadata(): Metadata { + const { + Metadata, + // eslint-disable-next-line @typescript-eslint/no-var-requires + } = require('@grpc/grpc-js'); + return new Metadata(); +} + +export class GrpcExporterTransport { + private _client?: any; + private _metadata?: Metadata; + + constructor( + private _parameters: { + grpcPath: string; + grpcName: string; + address: string; + /** + * NOTE: Ensure that you're only importing/requiring gRPC inside the function providing the channel credentials, + * otherwise, gRPC and http/https instrumentations may break. + * + * For common cases, you can avoid to import/require gRPC your function by using + * - {@link createSslCredentials} + * - {@link createInsecureCredentials} + */ + credentials: () => ChannelCredentials; + /** + * NOTE: Ensure that you're only importing/requiring gRPC inside the function providing the metadata, + * otherwise, gRPC and http/https instrumentations may break. + * + * To avoid having to import/require gRPC from your function to create a new Metadata object, + * use {@link createEmptyMetadata} + */ + metadata: () => Metadata; + compression: 'gzip' | 'none'; + timeoutMillis: number; + } + ) {} + + shutdown() { + this._client.shutdown(); + } + + send(data: Uint8Array): Promise { + // We need to make a for gRPC + const buffer = Buffer.from(data); + + if (this._client == null) { + // Lazy require to ensure that grpc is not loaded before instrumentations can wrap it + const { + createServiceClientConstructor, + // eslint-disable-next-line @typescript-eslint/no-var-requires + } = require('./create-service-client-constructor'); + + const channelCredentials = this._parameters.credentials(); + this._metadata = this._parameters.metadata(); + + const clientConstructor = createServiceClientConstructor( + this._parameters.grpcPath, + this._parameters.grpcName + ); + + this._client = new clientConstructor( + this._parameters.address, + channelCredentials, + { + 'grpc.default_compression_algorithm': toGrpcCompression( + this._parameters.compression + ), + } + ); + } + + return new Promise(resolve => { + // this will always be defined + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const deadline = Date.now() + this._parameters.timeoutMillis; + + // this should never happen + if (this._metadata == null) { + return resolve({ + error: new Error('could not get metadata'), + status: 'failure', + }); + } + + this._client.export( + buffer, + this._metadata, + { deadline: deadline }, + (err: ServiceError, response: Buffer) => { + if (err) { + resolve({ + status: 'failure', + error: err, + }); + } else { + resolve({ + data: response, + status: 'success', + }); + } + } + ); + }); + } +} diff --git a/experimental/packages/otlp-grpc-exporter-base/src/index.ts b/experimental/packages/otlp-grpc-exporter-base/src/index.ts index 2669033a460..0e7f487e2be 100644 --- a/experimental/packages/otlp-grpc-exporter-base/src/index.ts +++ b/experimental/packages/otlp-grpc-exporter-base/src/index.ts @@ -14,10 +14,12 @@ * limitations under the License. */ -export * from './OTLPGRPCExporterNodeBase'; -export { ServiceClientType, OTLPGRPCExporterConfigNode } from './types'; +export { OTLPGRPCExporterNodeBase } from './OTLPGRPCExporterNodeBase'; +export { OTLPGRPCExporterConfigNode } from './types'; +export { DEFAULT_COLLECTOR_URL, validateAndNormalizeUrl } from './util'; export { - DEFAULT_COLLECTOR_URL, - validateAndNormalizeUrl, - GrpcCompressionAlgorithm, -} from './util'; + MetricsSerializer, + TraceSerializer, + LogsSerializer, + ISerializer, +} from './serializers'; diff --git a/experimental/packages/otlp-grpc-exporter-base/src/serializers.ts b/experimental/packages/otlp-grpc-exporter-base/src/serializers.ts new file mode 100644 index 00000000000..3e8b22f6053 --- /dev/null +++ b/experimental/packages/otlp-grpc-exporter-base/src/serializers.ts @@ -0,0 +1,88 @@ +/* + * 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 root from './generated/root'; +import { + IExportLogsServiceRequest, + IExportLogsServiceResponse, + IExportMetricsServiceRequest, + IExportMetricsServiceResponse, + IExportTraceServiceRequest, + IExportTraceServiceResponse, +} from '@opentelemetry/otlp-transformer'; +import { ExportType } from './internal-types'; + +const logsResponseType = root.opentelemetry.proto.collector.logs.v1 + .ExportLogsServiceResponse as ExportType; + +const logsRequestType = root.opentelemetry.proto.collector.logs.v1 + .ExportLogsServiceRequest as ExportType; + +const metricsResponseType = root.opentelemetry.proto.collector.metrics.v1 + .ExportMetricsServiceResponse as ExportType; + +const metricsRequestType = root.opentelemetry.proto.collector.metrics.v1 + .ExportMetricsServiceRequest as ExportType; + +const traceResponseType = root.opentelemetry.proto.collector.trace.v1 + .ExportTraceServiceResponse as ExportType; + +const traceRequestType = root.opentelemetry.proto.collector.trace.v1 + .ExportTraceServiceRequest as ExportType; + +/** + * Serializes and deserializes the OTLP request/response to and from {@link Uint8Array} + */ +export interface ISerializer { + serializeRequest(request: Request): Uint8Array | undefined; + deserializeResponse(data: Uint8Array): Response; +} + +export const LogsSerializer: ISerializer< + IExportLogsServiceRequest, + IExportLogsServiceResponse +> = { + serializeRequest: (arg: IExportLogsServiceRequest) => { + return Buffer.from(logsRequestType.encode(arg).finish()); + }, + deserializeResponse: (arg: Buffer) => { + return logsResponseType.decode(arg); + }, +}; + +export const TraceSerializer: ISerializer< + IExportTraceServiceRequest, + IExportTraceServiceResponse +> = { + serializeRequest: (arg: IExportTraceServiceRequest) => { + return Buffer.from(traceRequestType.encode(arg).finish()); + }, + deserializeResponse: (arg: Buffer) => { + return traceResponseType.decode(arg); + }, +}; + +export const MetricsSerializer: ISerializer< + IExportMetricsServiceRequest, + IExportMetricsServiceResponse +> = { + serializeRequest: (arg: IExportMetricsServiceRequest) => { + return Buffer.from(metricsRequestType.encode(arg).finish()); + }, + deserializeResponse: (arg: Buffer) => { + return metricsResponseType.decode(arg); + }, +}; diff --git a/experimental/packages/otlp-grpc-exporter-base/src/types.ts b/experimental/packages/otlp-grpc-exporter-base/src/types.ts index 7ecedff1baf..43caad1371a 100644 --- a/experimental/packages/otlp-grpc-exporter-base/src/types.ts +++ b/experimental/packages/otlp-grpc-exporter-base/src/types.ts @@ -14,7 +14,8 @@ * limitations under the License. */ -import * as grpc from '@grpc/grpc-js'; +import type { ChannelCredentials, Metadata } from '@grpc/grpc-js'; + import { CompressionAlgorithm, OTLPExporterConfigBase, @@ -31,31 +32,11 @@ export interface GRPCQueueItem { onError: (error: OTLPExporterError) => void; } -/** - * Service Client for sending spans/metrics/logs - */ -export interface ServiceClient { - export: ( - request: any, - metadata: grpc.Metadata, - options: grpc.CallOptions, - callback: Function - ) => {}; - - close(): void; -} - /** * OTLP Exporter Config for Node */ export interface OTLPGRPCExporterConfigNode extends OTLPExporterConfigBase { - credentials?: grpc.ChannelCredentials; - metadata?: grpc.Metadata; + credentials?: ChannelCredentials; + metadata?: Metadata; compression?: CompressionAlgorithm; } - -export enum ServiceClientType { - SPANS, - METRICS, - LOGS, -} diff --git a/experimental/packages/otlp-grpc-exporter-base/src/util.ts b/experimental/packages/otlp-grpc-exporter-base/src/util.ts index 78809466c96..4386b341694 100644 --- a/experimental/packages/otlp-grpc-exporter-base/src/util.ts +++ b/experimental/packages/otlp-grpc-exporter-base/src/util.ts @@ -14,111 +14,23 @@ * limitations under the License. */ -import * as grpc from '@grpc/grpc-js'; import { diag } from '@opentelemetry/api'; -import { getEnv, globalErrorHandler } from '@opentelemetry/core'; +import { getEnv } from '@opentelemetry/core'; import * as path from 'path'; -import { OTLPGRPCExporterNodeBase } from './OTLPGRPCExporterNodeBase'; import { URL } from 'url'; import * as fs from 'fs'; +import { CompressionAlgorithm } from '@opentelemetry/otlp-exporter-base'; import { - GRPCQueueItem, - OTLPGRPCExporterConfigNode, - ServiceClientType, -} from './types'; -import { - ExportServiceError, - OTLPExporterError, - CompressionAlgorithm, -} from '@opentelemetry/otlp-exporter-base'; + createInsecureCredentials, + createSslCredentials, +} from './grpc-exporter-transport'; -import { MetricExportServiceClient } from './MetricsExportServiceClient'; -import { TraceExportServiceClient } from './TraceExportServiceClient'; -import { LogsExportServiceClient } from './LogsExportServiceClient'; +// NOTE: do not change these type imports to actual imports. Doing so WILL break `@opentelemetry/instrumentation-http`, +// as they'd be imported before the http/https modules can be wrapped. +import type { ChannelCredentials } from '@grpc/grpc-js'; export const DEFAULT_COLLECTOR_URL = 'http://localhost:4317'; -export function onInit( - collector: OTLPGRPCExporterNodeBase, - config: OTLPGRPCExporterConfigNode -): void { - collector.grpcQueue = []; - - const credentials: grpc.ChannelCredentials = configureSecurity( - config.credentials, - collector.getUrlFromConfig(config) - ); - - try { - if (collector.getServiceClientType() === ServiceClientType.SPANS) { - const client = new TraceExportServiceClient(collector.url, credentials, { - 'grpc.default_compression_algorithm': collector.compression.valueOf(), - }); - - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - collector.serviceClient = client; - } else if (collector.getServiceClientType() === ServiceClientType.METRICS) { - const client = new MetricExportServiceClient(collector.url, credentials, { - 'grpc.default_compression_algorithm': collector.compression.valueOf(), - }); - - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - collector.serviceClient = client; - } else if (collector.getServiceClientType() === ServiceClientType.LOGS) { - const client = new LogsExportServiceClient(collector.url, credentials, { - 'grpc.default_compression_algorithm': collector.compression.valueOf(), - }); - - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - collector.serviceClient = client; - } - } catch (err) { - globalErrorHandler(err); - } - if (collector.grpcQueue.length > 0) { - const queue = collector.grpcQueue.splice(0); - queue.forEach((item: GRPCQueueItem) => { - collector.send(item.objects, item.onSuccess, item.onError); - }); - } -} - -export function send( - collector: OTLPGRPCExporterNodeBase, - objects: ExportItem[], - onSuccess: () => void, - onError: (error: OTLPExporterError) => void -): void { - if (collector.serviceClient) { - const serviceRequest = collector.convert(objects); - const deadline = Date.now() + collector.timeoutMillis; - - collector.serviceClient.export( - serviceRequest, - collector.metadata || new grpc.Metadata(), - { deadline: deadline }, - (err: ExportServiceError) => { - if (err) { - diag.error('Service request', serviceRequest); - onError(err); - } else { - diag.debug('Objects sent'); - onSuccess(); - } - } - ); - } else { - collector.grpcQueue.push({ - objects, - onSuccess, - onError, - }); - } -} - export function validateAndNormalizeUrl(url: string): string { const hasProtocol = url.match(/^([\w]{1,8}):\/\//); if (!hasProtocol) { @@ -139,10 +51,10 @@ export function validateAndNormalizeUrl(url: string): string { return target.host; } -export function configureSecurity( - credentials: grpc.ChannelCredentials | undefined, +export function configureCredentials( + credentials: ChannelCredentials | undefined, endpoint: string -): grpc.ChannelCredentials { +): ChannelCredentials { let insecure: boolean; if (credentials) { @@ -159,9 +71,9 @@ export function configureSecurity( } if (insecure) { - return grpc.credentials.createInsecure(); + return createInsecureCredentials(); } else { - return useSecureConnection(); + return getCredentialsFromEnvironment(); } } @@ -177,16 +89,15 @@ function getSecurityFromEnv(): boolean { } } -export function useSecureConnection(): grpc.ChannelCredentials { - const rootCertPath = retrieveRootCert(); - const privateKeyPath = retrievePrivateKey(); - const certChainPath = retrieveCertChain(); +/** + * Exported for testing + */ +export function getCredentialsFromEnvironment(): ChannelCredentials { + const rootCert = retrieveRootCert(); + const privateKey = retrievePrivateKey(); + const certChain = retrieveCertChain(); - return grpc.credentials.createSsl( - rootCertPath, - privateKeyPath, - certChainPath - ); + return createSslCredentials(rootCert, privateKey, certChain); } function retrieveRootCert(): Buffer | undefined { @@ -240,36 +151,25 @@ function retrieveCertChain(): Buffer | undefined { } } -function toGrpcCompression( - compression: CompressionAlgorithm -): GrpcCompressionAlgorithm { - if (compression === CompressionAlgorithm.NONE) - return GrpcCompressionAlgorithm.NONE; - else if (compression === CompressionAlgorithm.GZIP) - return GrpcCompressionAlgorithm.GZIP; - return GrpcCompressionAlgorithm.NONE; -} - -/** - * These values are defined by grpc client - */ -export enum GrpcCompressionAlgorithm { - NONE = 0, - GZIP = 2, -} - export function configureCompression( compression: CompressionAlgorithm | undefined -): GrpcCompressionAlgorithm { - if (compression) { - return toGrpcCompression(compression); - } else { - const definedCompression = - getEnv().OTEL_EXPORTER_OTLP_TRACES_COMPRESSION || - getEnv().OTEL_EXPORTER_OTLP_COMPRESSION; +): CompressionAlgorithm { + if (compression != null) { + return compression; + } + + const envCompression = + getEnv().OTEL_EXPORTER_OTLP_TRACES_COMPRESSION || + getEnv().OTEL_EXPORTER_OTLP_COMPRESSION; - return definedCompression === 'gzip' - ? GrpcCompressionAlgorithm.GZIP - : GrpcCompressionAlgorithm.NONE; + if (envCompression === 'gzip') { + return CompressionAlgorithm.GZIP; + } else if (envCompression === 'none') { + return CompressionAlgorithm.NONE; } + + diag.warn( + 'Unknown compression "' + envCompression + '", falling back to "none"' + ); + return CompressionAlgorithm.NONE; } diff --git a/experimental/packages/otlp-grpc-exporter-base/test/OTLPGRPCExporterNodeBase.test.ts b/experimental/packages/otlp-grpc-exporter-base/test/OTLPGRPCExporterNodeBase.test.ts index ec28b53011d..11f5618a98b 100644 --- a/experimental/packages/otlp-grpc-exporter-base/test/OTLPGRPCExporterNodeBase.test.ts +++ b/experimental/packages/otlp-grpc-exporter-base/test/OTLPGRPCExporterNodeBase.test.ts @@ -14,146 +14,3 @@ * limitations under the License. */ -import { ReadableSpan } from '@opentelemetry/sdk-trace-base'; -import * as assert from 'assert'; -import { OTLPGRPCExporterNodeBase } from '../src/OTLPGRPCExporterNodeBase'; -import { OTLPGRPCExporterConfigNode, ServiceClientType } from '../src/types'; -import { mockedReadableSpan } from './traceHelper'; -import { OTLPExporterError } from '@opentelemetry/otlp-exporter-base'; - -class MockCollectorExporter extends OTLPGRPCExporterNodeBase< - ReadableSpan, - ReadableSpan[] -> { - /** - * Callbacks passed to _send() - */ - sendCallbacks: { - onSuccess: () => void; - onError: (error: OTLPExporterError) => void; - }[] = []; - - getDefaultUrl(config: OTLPGRPCExporterConfigNode): string { - return ''; - } - - getDefaultServiceName(config: OTLPGRPCExporterConfigNode): string { - return ''; - } - - convert(spans: ReadableSpan[]): ReadableSpan[] { - return spans; - } - - getServiceClientType() { - return ServiceClientType.SPANS; - } - - getServiceProtoPath(): string { - return 'opentelemetry/proto/collector/trace/v1/trace_service.proto'; - } - - getUrlFromConfig(config: OTLPGRPCExporterConfigNode): string { - return ''; - } -} - -// Mocked _send which just saves the callbacks for later -MockCollectorExporter.prototype['_send'] = function _sendMock( - self: MockCollectorExporter, - objects: ReadableSpan[], - onSuccess: () => void, - onError: (error: OTLPExporterError) => void -): void { - self.sendCallbacks.push({ onSuccess, onError }); -}; - -describe('OTLPGRPCExporterNodeBase', () => { - let exporter: MockCollectorExporter; - const concurrencyLimit = 5; - - beforeEach(done => { - exporter = new MockCollectorExporter({ concurrencyLimit }); - done(); - }); - - describe('export', () => { - it('should export requests concurrently', async () => { - const spans = [Object.assign({}, mockedReadableSpan)]; - const numToExport = concurrencyLimit; - - for (let i = 0; i < numToExport; ++i) { - exporter.export(spans, () => {}); - } - - assert.strictEqual(exporter['_sendingPromises'].length, numToExport); - const promisesAllDone = Promise.all(exporter['_sendingPromises']); - // Mock that all requests finish sending - exporter.sendCallbacks.forEach(({ onSuccess }) => onSuccess()); - - // All finished promises should be popped off - await promisesAllDone; - assert.strictEqual(exporter['_sendingPromises'].length, 0); - }); - - it('should drop new export requests when already sending at concurrencyLimit', async () => { - const spans = [Object.assign({}, mockedReadableSpan)]; - const numToExport = concurrencyLimit + 5; - - for (let i = 0; i < numToExport; ++i) { - exporter.export(spans, () => {}); - } - - assert.strictEqual(exporter['_sendingPromises'].length, concurrencyLimit); - const promisesAllDone = Promise.all(exporter['_sendingPromises']); - // Mock that all requests finish sending - exporter.sendCallbacks.forEach(({ onSuccess }) => onSuccess()); - - // All finished promises should be popped off - await promisesAllDone; - assert.strictEqual(exporter['_sendingPromises'].length, 0); - }); - - it('should pop export request promises even if they failed', async () => { - const spans = [Object.assign({}, mockedReadableSpan)]; - - exporter.export(spans, () => {}); - assert.strictEqual(exporter['_sendingPromises'].length, 1); - const promisesAllDone = Promise.all(exporter['_sendingPromises']); - // Mock that all requests fail sending - exporter.sendCallbacks.forEach(({ onError }) => - onError(new Error('Failed to send!!')) - ); - - // All finished promises should be popped off - await promisesAllDone; - assert.strictEqual(exporter['_sendingPromises'].length, 0); - }); - - it('should pop export request promises even if success callback throws error', async () => { - const spans = [Object.assign({}, mockedReadableSpan)]; - - exporter['_sendPromise']( - spans, - () => { - throw new Error('Oops'); - }, - () => {} - ); - - assert.strictEqual(exporter['_sendingPromises'].length, 1); - const promisesAllDone = Promise.all(exporter['_sendingPromises']) - // catch expected unhandled exception - .catch(() => {}); - - // Mock that the request finishes sending - exporter.sendCallbacks.forEach(({ onSuccess }) => { - onSuccess(); - }); - - // All finished promises should be popped off - await promisesAllDone; - assert.strictEqual(exporter['_sendingPromises'].length, 0); - }); - }); -}); diff --git a/experimental/packages/otlp-grpc-exporter-base/test/util.test.ts b/experimental/packages/otlp-grpc-exporter-base/test/util.test.ts index 967ece67169..1da067b559c 100644 --- a/experimental/packages/otlp-grpc-exporter-base/test/util.test.ts +++ b/experimental/packages/otlp-grpc-exporter-base/test/util.test.ts @@ -22,9 +22,8 @@ import * as grpc from '@grpc/grpc-js'; import { validateAndNormalizeUrl, configureCompression, - GrpcCompressionAlgorithm, - configureSecurity, - useSecureConnection, + configureCredentials, + getCredentialsFromEnvironment, DEFAULT_COLLECTOR_URL, } from '../src/util'; import { CompressionAlgorithm } from '@opentelemetry/otlp-exporter-base'; @@ -94,15 +93,15 @@ describe('validateAndNormalizeUrl()', () => { }); }); -describe('utils - configureSecurity', () => { +describe('utils - configureCredentials', () => { const envSource = process.env; it('should return insecure channel when using all defaults', () => { - const credentials = configureSecurity(undefined, DEFAULT_COLLECTOR_URL); + const credentials = configureCredentials(undefined, DEFAULT_COLLECTOR_URL); assert.ok(credentials._isSecure() === false); }); it('should return user defined channel credentials', () => { const userDefinedCredentials = grpc.credentials.createSsl(); - const credentials = configureSecurity( + const credentials = configureCredentials( userDefinedCredentials, 'http://foo.bar' ); @@ -112,40 +111,40 @@ describe('utils - configureSecurity', () => { }); it('should return secure channel when endpoint contains https scheme - no matter insecure env settings,', () => { envSource.OTEL_EXPORTER_OTLP_TRACES_INSECURE = 'true'; - const credentials = configureSecurity(undefined, 'https://foo.bar'); + const credentials = configureCredentials(undefined, 'https://foo.bar'); assert.ok(credentials._isSecure() === true); delete envSource.OTEL_EXPORTER_OTLP_TRACES_INSECURE; }); it('should return insecure channel when endpoint contains http scheme and no insecure env settings', () => { - const credentials = configureSecurity(undefined, 'http://foo.bar'); + const credentials = configureCredentials(undefined, 'http://foo.bar'); assert.ok(credentials._isSecure() === false); }); it('should return secure channel when endpoint does not contain scheme and no insecure env settings', () => { - const credentials = configureSecurity(undefined, 'foo.bar'); + const credentials = configureCredentials(undefined, 'foo.bar'); assert.ok(credentials._isSecure() === true); }); it('should return insecure channel when endpoint contains http scheme and insecure env set to false', () => { envSource.OTEL_EXPORTER_OTLP_TRACES_INSECURE = 'false'; - const credentials = configureSecurity(undefined, 'http://foo.bar'); + const credentials = configureCredentials(undefined, 'http://foo.bar'); assert.ok(credentials._isSecure() === false); delete envSource.OTEL_EXPORTER_OTLP_TRACES_INSECURE; }); it('should return insecure channel when endpoint contains http scheme and insecure env set to true', () => { envSource.OTEL_EXPORTER_OTLP_INSECURE = 'true'; - const credentials = configureSecurity(undefined, 'http://localhost'); + const credentials = configureCredentials(undefined, 'http://localhost'); assert.ok(credentials._isSecure() === false); delete envSource.OTEL_EXPORTER_OTLP_INSECURE; }); it('should return secure channel when endpoint does not contain scheme and insecure env set to false', () => { envSource.OTEL_EXPORTER_OTLP_TRACES_INSECURE = 'false'; - const credentials = configureSecurity(undefined, 'foo.bar'); + const credentials = configureCredentials(undefined, 'foo.bar'); assert.ok(credentials._isSecure() === true); delete envSource.OTEL_EXPORTER_OTLP_TRACES_INSECURE; }); it('should return insecure channel when endpoint does not contain scheme and insecure env set to true', () => { envSource.OTEL_EXPORTER_OTLP_INSECURE = 'true'; - const credentials = configureSecurity(undefined, 'foo.bar'); + const credentials = configureCredentials(undefined, 'foo.bar'); assert.ok(credentials._isSecure() === false); delete envSource.OTEL_EXPORTER_OTLP_INSECURE; }); @@ -159,7 +158,7 @@ describe('useSecureConnection', () => { envSource.OTEL_EXPORTER_OTLP_TRACES_CLIENT_CERTIFICATE = './test/certs/client.crt'; - const credentials = useSecureConnection(); + const credentials = getCredentialsFromEnvironment(); assert.ok(credentials._isSecure() === true); delete envSource.OTEL_EXPORTER_OTLP_CERTIFICATE; @@ -168,14 +167,14 @@ describe('useSecureConnection', () => { }); it('should return secure connection using only root certificate', () => { envSource.OTEL_EXPORTER_OTLP_CERTIFICATE = './test/certs/ca.crt'; - const credentials = useSecureConnection(); + const credentials = getCredentialsFromEnvironment(); assert.ok(credentials._isSecure() === true); delete envSource.OTEL_EXPORTER_OTLP_CERTIFICATE; }); it('should warn user when file cannot be read and use default root certificate', () => { envSource.OTEL_EXPORTER_OTLP_CERTIFICATE = './wrongpath/test/certs/ca.crt'; const diagWarn = sinon.stub(diag, 'warn'); - const credentials = useSecureConnection(); + const credentials = getCredentialsFromEnvironment(); const args = diagWarn.args[0]; assert.strictEqual(args[0], 'Failed to read root certificate file'); @@ -193,14 +192,14 @@ describe('configureCompression', () => { const compression = CompressionAlgorithm.NONE; assert.strictEqual( configureCompression(compression), - GrpcCompressionAlgorithm.NONE + CompressionAlgorithm.NONE ); }); it('should return gzip compression defined via env', () => { envSource.OTEL_EXPORTER_OTLP_TRACES_COMPRESSION = 'gzip'; assert.strictEqual( configureCompression(undefined), - GrpcCompressionAlgorithm.GZIP + CompressionAlgorithm.GZIP ); delete envSource.OTEL_EXPORTER_OTLP_TRACES_COMPRESSION; }); @@ -208,14 +207,14 @@ describe('configureCompression', () => { envSource.OTEL_EXPORTER_OTLP_TRACES_COMPRESSION = 'none'; assert.strictEqual( configureCompression(undefined), - GrpcCompressionAlgorithm.NONE + CompressionAlgorithm.NONE ); delete envSource.OTEL_EXPORTER_OTLP_TRACES_COMPRESSION; }); it('should return none for compression when no compression is set', () => { assert.strictEqual( configureCompression(undefined), - GrpcCompressionAlgorithm.NONE + CompressionAlgorithm.NONE ); }); }); From 9f12ea650e95e6b6f3396212798186fa83801afe Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Thu, 25 Jan 2024 14:56:34 +0100 Subject: [PATCH 02/13] fix(otlp-grpc-exporter-base): add back tests --- .../src/OTLPGRPCExporterNodeBase.ts | 3 +- .../src/export-response.ts | 22 ++ .../src/exporter-transport.ts | 22 ++ .../src/grpc-exporter-transport.ts | 11 +- .../test/OTLPGRPCExporterNodeBase.test.ts | 194 ++++++++++++++++++ 5 files changed, 243 insertions(+), 9 deletions(-) create mode 100644 experimental/packages/otlp-grpc-exporter-base/src/export-response.ts create mode 100644 experimental/packages/otlp-grpc-exporter-base/src/exporter-transport.ts diff --git a/experimental/packages/otlp-grpc-exporter-base/src/OTLPGRPCExporterNodeBase.ts b/experimental/packages/otlp-grpc-exporter-base/src/OTLPGRPCExporterNodeBase.ts index f5f4c2144fd..d0b741c1a13 100644 --- a/experimental/packages/otlp-grpc-exporter-base/src/OTLPGRPCExporterNodeBase.ts +++ b/experimental/packages/otlp-grpc-exporter-base/src/OTLPGRPCExporterNodeBase.ts @@ -28,6 +28,7 @@ import { } from './grpc-exporter-transport'; import { configureCompression, configureCredentials } from './util'; import { ISerializer } from './serializers'; +import { IExporterTransport } from './exporter-transport'; /** * OTLP Exporter abstract base class @@ -43,7 +44,7 @@ export abstract class OTLPGRPCExporterNodeBase< > { grpcQueue: GRPCQueueItem[] = []; compression: CompressionAlgorithm; - private _transport: GrpcExporterTransport; + private _transport: IExporterTransport; private _serializer: ISerializer; constructor( diff --git a/experimental/packages/otlp-grpc-exporter-base/src/export-response.ts b/experimental/packages/otlp-grpc-exporter-base/src/export-response.ts new file mode 100644 index 00000000000..623c4532925 --- /dev/null +++ b/experimental/packages/otlp-grpc-exporter-base/src/export-response.ts @@ -0,0 +1,22 @@ +/* + * 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. + */ + +export interface ExportResponse { + status: 'success' | 'failure'; + data?: Uint8Array; + retryInMillis?: number; + error?: Error; +} diff --git a/experimental/packages/otlp-grpc-exporter-base/src/exporter-transport.ts b/experimental/packages/otlp-grpc-exporter-base/src/exporter-transport.ts new file mode 100644 index 00000000000..bb9deac834d --- /dev/null +++ b/experimental/packages/otlp-grpc-exporter-base/src/exporter-transport.ts @@ -0,0 +1,22 @@ +/* + * 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 { ExportResponse } from './export-response'; + +export interface IExporterTransport { + send(data: Uint8Array): Promise; + shutdown(): void; +} diff --git a/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts b/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts index 0cd0b5712f7..e714b843ab1 100644 --- a/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts +++ b/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts @@ -17,13 +17,8 @@ // NOTE: do not change these type imports to actual imports. Doing so WILL break `@opentelemetry/instrumentation-http`, // as they'd be imported before the http/https modules can be wrapped. import type { Metadata, ServiceError, ChannelCredentials } from '@grpc/grpc-js'; - -export interface ExportResponse { - status: 'success' | 'failure'; - data?: Uint8Array; - retryInMillis?: number; - error?: Error; -} +import { ExportResponse } from './export-response'; +import {IExporterTransport} from "./exporter-transport"; const GRPC_COMPRESSION_NONE = 0; const GRPC_COMPRESSION_GZIP = 2; @@ -65,7 +60,7 @@ export function createEmptyMetadata(): Metadata { return new Metadata(); } -export class GrpcExporterTransport { +export class GrpcExporterTransport implements IExporterTransport { private _client?: any; private _metadata?: Metadata; diff --git a/experimental/packages/otlp-grpc-exporter-base/test/OTLPGRPCExporterNodeBase.test.ts b/experimental/packages/otlp-grpc-exporter-base/test/OTLPGRPCExporterNodeBase.test.ts index 11f5618a98b..5c9cc4b6b4a 100644 --- a/experimental/packages/otlp-grpc-exporter-base/test/OTLPGRPCExporterNodeBase.test.ts +++ b/experimental/packages/otlp-grpc-exporter-base/test/OTLPGRPCExporterNodeBase.test.ts @@ -14,3 +14,197 @@ * limitations under the License. */ +import { ReadableSpan } from '@opentelemetry/sdk-trace-base'; +import * as assert from 'assert'; +import { OTLPGRPCExporterNodeBase } from '../src/OTLPGRPCExporterNodeBase'; +import { OTLPGRPCExporterConfigNode } from '../src/types'; +import { mockedReadableSpan } from './traceHelper'; +import { ExportResponse } from '../src/export-response'; +import { IExporterTransport } from '../src/exporter-transport'; +import { ISerializer } from '../src'; +import sinon = require('sinon'); + +class MockCollectorExporter extends OTLPGRPCExporterNodeBase< + ReadableSpan, + ReadableSpan[], + any +> { + getDefaultUrl(config: OTLPGRPCExporterConfigNode): string { + return ''; + } + + convert(spans: ReadableSpan[]): ReadableSpan[] { + return spans; + } + + getUrlFromConfig(config: OTLPGRPCExporterConfigNode): string { + return ''; + } +} + +const successfulResponse: ExportResponse = { + status: 'success', +}; + +describe('OTLPGRPCExporterNodeBase', () => { + let exporter: MockCollectorExporter; + const concurrencyLimit = 5; + + beforeEach(done => { + const transportStubs = { + // make transport succeed + send: sinon.stub().resolves(successfulResponse), + shutdown: sinon.stub(), + }; + const mockTransport = transportStubs; + const signalSpecificMetadata: Record = { + key: 'signal-specific-metadata', + }; + + const serializerStubs = { + serializeRequest: sinon.stub().resolves(Buffer.from([1, 2, 3])), + deserializeResponse: sinon + .stub() + .resolves({ responseKey: 'responseValue' }), + }; + + const serializer = >serializerStubs; + + exporter = new MockCollectorExporter( + { concurrencyLimit }, + signalSpecificMetadata, + 'grpcName', + 'grpcPath', + serializer + ); + + exporter['_transport'] = mockTransport; + done(); + }); + + afterEach(function () { + sinon.restore(); + }); + + describe('export', () => { + it('should export requests concurrently', async () => { + const sendResolveFunctions: ((response: ExportResponse) => void)[] = []; + const transportStubs = { + send: sinon.stub().returns( + new Promise(resolve => { + sendResolveFunctions.push(resolve); + }) + ), + shutdown: sinon.stub(), + }; + exporter['_transport'] = transportStubs; + + const spans = [Object.assign({}, mockedReadableSpan)]; + const numToExport = concurrencyLimit; + + for (let i = 0; i < numToExport; ++i) { + exporter.export(spans, () => {}); + } + + assert.strictEqual(exporter['_sendingPromises'].length, numToExport); + const promisesAllDone = Promise.all(exporter['_sendingPromises']); + // Mock that all requests finish sending + sendResolveFunctions.forEach(resolve => resolve(successfulResponse)); + + // All finished promises should be popped off + await promisesAllDone; + assert.strictEqual(exporter['_sendingPromises'].length, 0); + }); + + it('should drop new export requests when already sending at concurrencyLimit', async () => { + const sendResolveFunctions: ((response: ExportResponse) => void)[] = []; + const transportStubs = { + send: sinon.stub().returns( + new Promise(resolve => { + sendResolveFunctions.push(resolve); + }) + ), + shutdown: sinon.stub(), + }; + exporter['_transport'] = transportStubs; + + const spans = [Object.assign({}, mockedReadableSpan)]; + const numToExport = concurrencyLimit + 5; + + for (let i = 0; i < numToExport; ++i) { + exporter.export(spans, () => {}); + } + + assert.strictEqual(exporter['_sendingPromises'].length, concurrencyLimit); + const promisesAllDone = Promise.all(exporter['_sendingPromises']); + // Mock that all requests finish sending + sendResolveFunctions.forEach(resolve => resolve(successfulResponse)); + + // All finished promises should be popped off + await promisesAllDone; + assert.strictEqual(exporter['_sendingPromises'].length, 0); + }); + + it('should pop export request promises even if they failed', async () => { + const sendRejectFunctions: ((error: Error) => void)[] = []; + const transportStubs = { + send: sinon.stub().returns( + new Promise((_, reject) => { + sendRejectFunctions.push(reject); + }) + ), + shutdown: sinon.stub(), + }; + exporter['_transport'] = transportStubs; + + const spans = [Object.assign({}, mockedReadableSpan)]; + + exporter.export(spans, () => {}); + assert.strictEqual(exporter['_sendingPromises'].length, 1); + const promisesAllDone = Promise.all(exporter['_sendingPromises']); + // Mock that all requests fail sending + sendRejectFunctions.forEach(reject => reject(new Error('export failed'))); + + // All finished promises should be popped off + await promisesAllDone; + assert.strictEqual(exporter['_sendingPromises'].length, 0); + }); + + it('should pop export request promises even if resolve throws error', async () => { + const transportStubs = { + send: sinon.stub().returns( + new Promise(_ => { + throw new Error('this failed'); + }) + ), + shutdown: sinon.stub(), + }; + exporter['_transport'] = transportStubs; + + const spans = [Object.assign({}, mockedReadableSpan)]; + exporter.export(spans, () => {}); + + assert.strictEqual(exporter['_sendingPromises'].length, 1); + + const promisesAllDone = Promise.all(exporter['_sendingPromises']) + // catch expected unhandled exception + .catch(() => {}); + + // All finished promises should be popped off + await promisesAllDone; + assert.strictEqual(exporter['_sendingPromises'].length, 0); + }); + }); + + describe('shutdown', function () { + it('calls shutdown on transport', function () { + const transportStubs = { + send: sinon.stub(), + shutdown: sinon.stub(), + }; + exporter['_transport'] = transportStubs; + exporter.shutdown(); + sinon.assert.calledOnce(transportStubs.shutdown); + }); + }); +}); From 34bc307df28293571089b8583df7acb9e078c5b7 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Thu, 25 Jan 2024 15:04:01 +0100 Subject: [PATCH 03/13] fix: test --- .../exporter-logs-otlp-grpc/test/OTLPLogExporter.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/experimental/packages/exporter-logs-otlp-grpc/test/OTLPLogExporter.test.ts b/experimental/packages/exporter-logs-otlp-grpc/test/OTLPLogExporter.test.ts index dd3d620ee3f..e48a1cef684 100644 --- a/experimental/packages/exporter-logs-otlp-grpc/test/OTLPLogExporter.test.ts +++ b/experimental/packages/exporter-logs-otlp-grpc/test/OTLPLogExporter.test.ts @@ -342,7 +342,9 @@ describe('when configuring via environment', () => { }); it('should include user-agent header by default', () => { const collectorExporter = new OTLPLogExporter(); - assert.deepStrictEqual(collectorExporter.metadata?.get('User-Agent'), [ + const actualMetadata = + collectorExporter['_transport']['_parameters'].metadata(); + assert.deepStrictEqual(actualMetadata.get('User-Agent'), [ `OTel-OTLP-Exporter-JavaScript/${VERSION}`, ]); }); From 25140c0edbdd3fcc170fa4292cef970c58b14428 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Thu, 25 Jan 2024 15:24:34 +0100 Subject: [PATCH 04/13] fix(changelog): add entry --- experimental/CHANGELOG.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index af30ef3c7ed..d21c5fe1e18 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -11,6 +11,19 @@ All notable changes to experimental packages in this project will be documented * This breaking change only affects users that are using the *experimental* `@opentelemetry/instrumentation/hook.mjs` loader hook AND Node.js 18.19 or later: * This reverts back to an older version of `import-in-the-middle` due to * This version does not support Node.js 18.19 or later +* fix(exporter-*-otlp-grpc)!: lazy load gRPC to improve compatibility with `@opentelemetry/instrumenation-grpc` [#4432](https://github.com/open-telemetry/opentelemetry-js/pull/4432) @pichlermarc + * Fixes a bug where requiring up the gRPC exporter before enabling the instrumentation from `@opentelemetry/instrumentation-grpc` would lead to missing telemetry + * Breaking changes, removes several functions and properties that were used internally and were not intended for end-users + * `getServiceClientType()` + * this returned a static enum value that would denote the export type (`SPAN`, `METRICS`, `LOGS`) + * `getServiceProtoPath()` + * this returned a static enum value that would correspond to the gRPC service path + * `metadata` + * was used internally to access metadata, but as a side effect allowed end-users to modify metadata on runtime. + * `serviceClient` + * was used internally to keep track of the service client used by the exporter, as a side effect it allowed end-users to modify the gRPC service client that was used + * `compression` + * was used internally to keep track of the compression to use but was unintentionally exposed to the users. It allowed to read and write the value, writing, however, would have no effect. ### :rocket: (Enhancement) From 06600ad81def8f8da356045283456e255bb2d012 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Thu, 25 Jan 2024 15:29:05 +0100 Subject: [PATCH 05/13] fix: lint --- .../otlp-grpc-exporter-base/src/grpc-exporter-transport.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts b/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts index e714b843ab1..330f00847c6 100644 --- a/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts +++ b/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts @@ -18,7 +18,7 @@ // as they'd be imported before the http/https modules can be wrapped. import type { Metadata, ServiceError, ChannelCredentials } from '@grpc/grpc-js'; import { ExportResponse } from './export-response'; -import {IExporterTransport} from "./exporter-transport"; +import { IExporterTransport } from './exporter-transport'; const GRPC_COMPRESSION_NONE = 0; const GRPC_COMPRESSION_GZIP = 2; From e6e39c199ac9760f2cf2eb047985b7a185573f33 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Thu, 25 Jan 2024 16:35:08 +0100 Subject: [PATCH 06/13] test(otlp-exporter-grpc-base): test new GrpcExporterTransport --- .../src/grpc-exporter-transport.ts | 91 +++--- .../test/grpc-exporter-transport.test.ts | 307 ++++++++++++++++++ .../otlp-grpc-exporter-base/tsconfig.json | 2 +- 3 files changed, 360 insertions(+), 40 deletions(-) create mode 100644 experimental/packages/otlp-grpc-exporter-base/test/grpc-exporter-transport.test.ts diff --git a/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts b/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts index 330f00847c6..7ba618d6be7 100644 --- a/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts +++ b/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts @@ -60,39 +60,39 @@ export function createEmptyMetadata(): Metadata { return new Metadata(); } +export interface GrpcExporterTransportParameters { + grpcPath: string; + grpcName: string; + address: string; + /** + * NOTE: Ensure that you're only importing/requiring gRPC inside the function providing the channel credentials, + * otherwise, gRPC and http/https instrumentations may break. + * + * For common cases, you can avoid to import/require gRPC your function by using + * - {@link createSslCredentials} + * - {@link createInsecureCredentials} + */ + credentials: () => ChannelCredentials; + /** + * NOTE: Ensure that you're only importing/requiring gRPC inside the function providing the metadata, + * otherwise, gRPC and http/https instrumentations may break. + * + * To avoid having to import/require gRPC from your function to create a new Metadata object, + * use {@link createEmptyMetadata} + */ + metadata: () => Metadata; + compression: 'gzip' | 'none'; + timeoutMillis: number; +} + export class GrpcExporterTransport implements IExporterTransport { private _client?: any; private _metadata?: Metadata; - constructor( - private _parameters: { - grpcPath: string; - grpcName: string; - address: string; - /** - * NOTE: Ensure that you're only importing/requiring gRPC inside the function providing the channel credentials, - * otherwise, gRPC and http/https instrumentations may break. - * - * For common cases, you can avoid to import/require gRPC your function by using - * - {@link createSslCredentials} - * - {@link createInsecureCredentials} - */ - credentials: () => ChannelCredentials; - /** - * NOTE: Ensure that you're only importing/requiring gRPC inside the function providing the metadata, - * otherwise, gRPC and http/https instrumentations may break. - * - * To avoid having to import/require gRPC from your function to create a new Metadata object, - * use {@link createEmptyMetadata} - */ - metadata: () => Metadata; - compression: 'gzip' | 'none'; - timeoutMillis: number; - } - ) {} + constructor(private _parameters: GrpcExporterTransportParameters) {} shutdown() { - this._client.shutdown(); + this._client?.shutdown(); } send(data: Uint8Array): Promise { @@ -106,23 +106,36 @@ export class GrpcExporterTransport implements IExporterTransport { // eslint-disable-next-line @typescript-eslint/no-var-requires } = require('./create-service-client-constructor'); - const channelCredentials = this._parameters.credentials(); - this._metadata = this._parameters.metadata(); + try { + this._metadata = this._parameters.metadata(); + } catch (error) { + return Promise.resolve({ + status: 'failure', + error: error, + }); + } const clientConstructor = createServiceClientConstructor( this._parameters.grpcPath, this._parameters.grpcName ); - this._client = new clientConstructor( - this._parameters.address, - channelCredentials, - { - 'grpc.default_compression_algorithm': toGrpcCompression( - this._parameters.compression - ), - } - ); + try { + this._client = new clientConstructor( + this._parameters.address, + this._parameters.credentials(), + { + 'grpc.default_compression_algorithm': toGrpcCompression( + this._parameters.compression + ), + } + ); + } catch (error) { + return Promise.resolve({ + status: 'failure', + error: error, + }); + } } return new Promise(resolve => { @@ -133,7 +146,7 @@ export class GrpcExporterTransport implements IExporterTransport { // this should never happen if (this._metadata == null) { return resolve({ - error: new Error('could not get metadata'), + error: new Error('metadata was null'), status: 'failure', }); } diff --git a/experimental/packages/otlp-grpc-exporter-base/test/grpc-exporter-transport.test.ts b/experimental/packages/otlp-grpc-exporter-base/test/grpc-exporter-transport.test.ts new file mode 100644 index 00000000000..d9ec6f61478 --- /dev/null +++ b/experimental/packages/otlp-grpc-exporter-base/test/grpc-exporter-transport.test.ts @@ -0,0 +1,307 @@ +/* + * 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 { + createEmptyMetadata, + createInsecureCredentials, + createSslCredentials, + GrpcExporterTransport, + GrpcExporterTransportParameters, +} from '../src/grpc-exporter-transport'; +import * as assert from 'assert'; +import * as fs from 'fs'; +import sinon = require('sinon'); +import { Metadata, Server, ServerCredentials } from '@grpc/grpc-js'; +import { types } from 'util'; + +const testServiceDefinition = { + export: { + path: '/test/Export', + requestStream: false, + responseStream: false, + requestSerialize: (arg: Buffer) => { + return arg; + }, + requestDeserialize: (arg: Buffer) => { + return arg; + }, + responseSerialize: (arg: Buffer) => { + return arg; + }, + responseDeserialize: (arg: Buffer) => { + return arg; + }, + }, +}; + +const simpleClientConfig: GrpcExporterTransportParameters = { + metadata: () => { + const metadata = createEmptyMetadata(); + metadata.set('foo', 'bar'); + return metadata; + }, + timeoutMillis: 100, + grpcPath: '/test/Export', + grpcName: 'name', + credentials: createInsecureCredentials, + compression: 'none', + address: 'localhost:1234', +}; + +interface ExportedData { + request: Buffer; + metadata: Metadata; +} + +interface ServerTestContext { + requests: ExportedData[]; + serverResponseProvider: () => { error: Error | null; buffer?: Buffer }; +} + +/** + * Starts a customizable server that saves all responses to context.responses + * Returns data as defined in context.ServerResponseProvider + * + * @return shutdown handle, needs to be called to ensure that mocha exits + * @param context context for storing responses and to define server behavior. + */ +function startServer(context: ServerTestContext): Promise<() => void> { + const server = new Server(); + server.addService(testServiceDefinition, { + export: (data: ExportedData, callback: any) => { + context.requests.push(data); + const response = context.serverResponseProvider(); + callback(response.error, response.buffer); + }, + }); + + return new Promise<() => void>((resolve, reject) => { + server.bindAsync( + 'localhost:1234', + ServerCredentials.createInsecure(), + (error, port) => { + server.start(); + if (error != null) { + reject(error); + } + resolve(() => { + server.forceShutdown(); + }); + } + ); + }); +} + +describe('GrpcExporterTransport', function () { + describe('utilities', function () { + describe('createEmptyMetadata', function () { + it('returns new empty Metadata', function () { + const metadata = createEmptyMetadata(); + assert.strictEqual(Object.keys(metadata.getMap()).length, 0); + }); + }); + + describe('createInsecureCredentials', function () { + it('creates insecure grpc credentials', function () { + const credentials = createInsecureCredentials(); + assert.ok(!credentials._isSecure()); + }); + }); + + describe('createSslCredentials', function () { + it('creates SSL grpc credentials', function () { + const credentials = createSslCredentials( + Buffer.from(fs.readFileSync('./test/certs/ca.crt')), + Buffer.from(fs.readFileSync('./test/certs/client.key')), + Buffer.from(fs.readFileSync('./test/certs/client.crt')) + ); + assert.ok(credentials._isSecure()); + }); + }); + }); + describe('shutdown', function () { + afterEach(function () { + sinon.restore(); + }); + it('before send() does not error', function () { + const transport = new GrpcExporterTransport(simpleClientConfig); + transport.shutdown(); + + // no assertions, just checking that it does not throw any errors. + }); + + it('calls client shutdown if client is defined', function () { + // arrange + const transport = new GrpcExporterTransport({ + metadata: createEmptyMetadata, + timeoutMillis: 100, + grpcPath: 'path', + grpcName: 'name', + credentials: createInsecureCredentials, + compression: 'gzip', + address: 'localhost:1234', + }); + const shutdownStub = sinon.stub(); + transport['_client'] = { + shutdown: shutdownStub, + }; + + // act + transport.shutdown(); + + // assert + sinon.assert.calledOnce(shutdownStub); + }); + }); + describe('send', function () { + let shutdownHandle: () => void | undefined; + const serverTestContext: ServerTestContext = { + requests: [], + serverResponseProvider: () => { + return { error: null, buffer: Buffer.from([]) }; + }, + }; + + beforeEach(async function () { + shutdownHandle = await startServer(serverTestContext); + }); + + afterEach(function () { + shutdownHandle(); + + // clear context + serverTestContext.requests = []; + serverTestContext.serverResponseProvider = () => { + return { error: null, buffer: Buffer.from([]) }; + }; + }); + + it('sends data', async function () { + const transport = new GrpcExporterTransport(simpleClientConfig); + + const result = await transport.send(Buffer.from([1, 2, 3])); + + assert.strictEqual(result.status, 'success'); + assert.strictEqual(result.error, undefined); + assert.deepEqual(result.data, Buffer.from([])); + assert.strictEqual(result.retryInMillis, undefined); + assert.strictEqual(serverTestContext.requests.length, 1); + assert.deepEqual( + serverTestContext.requests[0].request, + Buffer.from([1, 2, 3]) + ); + assert.deepEqual( + serverTestContext.requests[0].metadata.get('foo'), + simpleClientConfig.metadata().get('foo') + ); + }); + + it('forwards response', async function () { + const expectedResponseData = Buffer.from([1, 2, 3]); + serverTestContext.serverResponseProvider = () => { + return { + buffer: expectedResponseData, + error: null, + }; + }; + const transport = new GrpcExporterTransport(simpleClientConfig); + + const result = await transport.send(Buffer.from([])); + + assert.strictEqual(result.status, 'success'); + assert.strictEqual(result.error, undefined); + assert.deepEqual(result.data, expectedResponseData); + assert.strictEqual(result.retryInMillis, undefined); + }); + + it('forwards handled server error as failure', async function () { + serverTestContext.serverResponseProvider = () => { + return { + buffer: Buffer.from([]), + error: new Error('handled server error'), + }; + }; + const transport = new GrpcExporterTransport(simpleClientConfig); + + const result = await transport.send(Buffer.from([])); + + assert.strictEqual(result.status, 'failure'); + assert.ok(types.isNativeError(result.error)); + assert.strictEqual(result.data, undefined); + assert.strictEqual(result.retryInMillis, undefined); + }); + + it('forwards unhandled server error as failure', async function () { + serverTestContext.serverResponseProvider = () => { + throw new Error('unhandled server error'); + }; + const transport = new GrpcExporterTransport(simpleClientConfig); + + const result = await transport.send(Buffer.from([])); + assert.strictEqual(result.status, 'failure'); + assert.ok(types.isNativeError(result.error)); + assert.strictEqual(result.data, undefined); + assert.strictEqual(result.retryInMillis, undefined); + }); + + it('forwards metadataProvider error as failure', async function () { + const expectedError = new Error('metadata provider error'); + const config = Object.assign({}, simpleClientConfig); + config.metadata = () => { + throw expectedError; + }; + + const transport = new GrpcExporterTransport(config); + + const result = await transport.send(Buffer.from([])); + assert.strictEqual(result.status, 'failure'); + assert.strictEqual(result.error, expectedError); + assert.strictEqual(result.data, undefined); + assert.strictEqual(result.retryInMillis, undefined); + }); + + it('forwards metadataProvider returns null value as failure', async function () { + const expectedError = new Error('metadata was null'); + const config = Object.assign({}, simpleClientConfig); + config.metadata = () => { + return null as unknown as Metadata; + }; + + const transport = new GrpcExporterTransport(config); + + const result = await transport.send(Buffer.from([])); + assert.strictEqual(result.status, 'failure'); + assert.deepEqual(result.error, expectedError); + assert.strictEqual(result.data, undefined); + assert.strictEqual(result.retryInMillis, undefined); + }); + + it('forwards credential error as failure', async function () { + const expectedError = new Error('credential provider error'); + const config = Object.assign({}, simpleClientConfig); + config.credentials = () => { + throw expectedError; + }; + + const transport = new GrpcExporterTransport(config); + + const result = await transport.send(Buffer.from([])); + assert.strictEqual(result.status, 'failure'); + assert.strictEqual(result.error, expectedError); + assert.strictEqual(result.data, undefined); + assert.strictEqual(result.retryInMillis, undefined); + }); + }); +}); diff --git a/experimental/packages/otlp-grpc-exporter-base/tsconfig.json b/experimental/packages/otlp-grpc-exporter-base/tsconfig.json index ddc035cf3a4..73906122795 100644 --- a/experimental/packages/otlp-grpc-exporter-base/tsconfig.json +++ b/experimental/packages/otlp-grpc-exporter-base/tsconfig.json @@ -1,7 +1,7 @@ { "extends": "../../../tsconfig.base.json", "compilerOptions": { - "allowJs": true, + //"allowJs": true, "outDir": "build", "rootDir": "." }, From 2bf2034343e141acb2c0d1bbe9f0030305a1fbac Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Fri, 26 Jan 2024 12:47:32 +0100 Subject: [PATCH 07/13] fix: set allowJs to true again --- experimental/packages/otlp-grpc-exporter-base/tsconfig.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/packages/otlp-grpc-exporter-base/tsconfig.json b/experimental/packages/otlp-grpc-exporter-base/tsconfig.json index 73906122795..ddc035cf3a4 100644 --- a/experimental/packages/otlp-grpc-exporter-base/tsconfig.json +++ b/experimental/packages/otlp-grpc-exporter-base/tsconfig.json @@ -1,7 +1,7 @@ { "extends": "../../../tsconfig.base.json", "compilerOptions": { - //"allowJs": true, + "allowJs": true, "outDir": "build", "rootDir": "." }, From 3391a0f18764e779544c49caceb7516fb50980e6 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Fri, 26 Jan 2024 12:50:16 +0100 Subject: [PATCH 08/13] fix: lint --- experimental/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 51a5c64beea..f8a7972cbe7 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -31,7 +31,7 @@ All notable changes to experimental packages in this project will be documented * `getServiceProtoPath()` * this returned a static enum value that would correspond to the gRPC service path * `metadata` - * was used internally to access metadata, but as a side effect allowed end-users to modify metadata on runtime. + * was used internally to access metadata, but as a side effect allowed end-users to modify metadata on runtime. * `serviceClient` * was used internally to keep track of the service client used by the exporter, as a side effect it allowed end-users to modify the gRPC service client that was used * `compression` From 02f703c2bb764792cadf1ed0fcb4bfa87f625ab3 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Thu, 22 Feb 2024 14:20:45 +0100 Subject: [PATCH 09/13] refactor: remove unnecessary/silly ts-ignore comment --- .../otlp-grpc-exporter-base/src/OTLPGRPCExporterNodeBase.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/experimental/packages/otlp-grpc-exporter-base/src/OTLPGRPCExporterNodeBase.ts b/experimental/packages/otlp-grpc-exporter-base/src/OTLPGRPCExporterNodeBase.ts index d0b741c1a13..cfa7fba3d73 100644 --- a/experimental/packages/otlp-grpc-exporter-base/src/OTLPGRPCExporterNodeBase.ts +++ b/experimental/packages/otlp-grpc-exporter-base/src/OTLPGRPCExporterNodeBase.ts @@ -125,8 +125,6 @@ export abstract class OTLPGRPCExporterNodeBase< } const converted = this.convert(objects); - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore TODO: no time for type stuff, it will work, trust me (TM) - that's what TM stands for lol. const data = this._serializer.serializeRequest(converted); if (data == null) { From 69f4e7bada7571a165db2a6a9ec8e43024f239b7 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Thu, 22 Feb 2024 14:45:55 +0100 Subject: [PATCH 10/13] feat: split ExportResponse into success and failure cases --- .../src/export-response.ts | 13 ++++-- .../test/OTLPGRPCExporterNodeBase.test.ts | 4 +- .../test/grpc-exporter-transport.test.ts | 46 ++++++++++--------- 3 files changed, 36 insertions(+), 27 deletions(-) diff --git a/experimental/packages/otlp-grpc-exporter-base/src/export-response.ts b/experimental/packages/otlp-grpc-exporter-base/src/export-response.ts index 623c4532925..c13af631e13 100644 --- a/experimental/packages/otlp-grpc-exporter-base/src/export-response.ts +++ b/experimental/packages/otlp-grpc-exporter-base/src/export-response.ts @@ -14,9 +14,14 @@ * limitations under the License. */ -export interface ExportResponse { - status: 'success' | 'failure'; +export interface ExportResponseSuccess { + status: 'success'; data?: Uint8Array; - retryInMillis?: number; - error?: Error; } + +export interface ExportResponseFailure { + status: 'failure'; + error: Error; +} + +export type ExportResponse = ExportResponseSuccess | ExportResponseFailure; diff --git a/experimental/packages/otlp-grpc-exporter-base/test/OTLPGRPCExporterNodeBase.test.ts b/experimental/packages/otlp-grpc-exporter-base/test/OTLPGRPCExporterNodeBase.test.ts index 5c9cc4b6b4a..f70ffd08cd3 100644 --- a/experimental/packages/otlp-grpc-exporter-base/test/OTLPGRPCExporterNodeBase.test.ts +++ b/experimental/packages/otlp-grpc-exporter-base/test/OTLPGRPCExporterNodeBase.test.ts @@ -19,7 +19,7 @@ import * as assert from 'assert'; import { OTLPGRPCExporterNodeBase } from '../src/OTLPGRPCExporterNodeBase'; import { OTLPGRPCExporterConfigNode } from '../src/types'; import { mockedReadableSpan } from './traceHelper'; -import { ExportResponse } from '../src/export-response'; +import { ExportResponse, ExportResponseSuccess } from '../src/export-response'; import { IExporterTransport } from '../src/exporter-transport'; import { ISerializer } from '../src'; import sinon = require('sinon'); @@ -42,7 +42,7 @@ class MockCollectorExporter extends OTLPGRPCExporterNodeBase< } } -const successfulResponse: ExportResponse = { +const successfulResponse: ExportResponseSuccess = { status: 'success', }; diff --git a/experimental/packages/otlp-grpc-exporter-base/test/grpc-exporter-transport.test.ts b/experimental/packages/otlp-grpc-exporter-base/test/grpc-exporter-transport.test.ts index d9ec6f61478..965c01607ce 100644 --- a/experimental/packages/otlp-grpc-exporter-base/test/grpc-exporter-transport.test.ts +++ b/experimental/packages/otlp-grpc-exporter-base/test/grpc-exporter-transport.test.ts @@ -25,6 +25,10 @@ import * as fs from 'fs'; import sinon = require('sinon'); import { Metadata, Server, ServerCredentials } from '@grpc/grpc-js'; import { types } from 'util'; +import { + ExportResponseFailure, + ExportResponseSuccess, +} from '../src/export-response'; const testServiceDefinition = { export: { @@ -191,12 +195,12 @@ describe('GrpcExporterTransport', function () { it('sends data', async function () { const transport = new GrpcExporterTransport(simpleClientConfig); - const result = await transport.send(Buffer.from([1, 2, 3])); + const result = (await transport.send( + Buffer.from([1, 2, 3]) + )) as ExportResponseSuccess; assert.strictEqual(result.status, 'success'); - assert.strictEqual(result.error, undefined); assert.deepEqual(result.data, Buffer.from([])); - assert.strictEqual(result.retryInMillis, undefined); assert.strictEqual(serverTestContext.requests.length, 1); assert.deepEqual( serverTestContext.requests[0].request, @@ -218,12 +222,12 @@ describe('GrpcExporterTransport', function () { }; const transport = new GrpcExporterTransport(simpleClientConfig); - const result = await transport.send(Buffer.from([])); + const result = (await transport.send( + Buffer.from([]) + )) as ExportResponseSuccess; assert.strictEqual(result.status, 'success'); - assert.strictEqual(result.error, undefined); assert.deepEqual(result.data, expectedResponseData); - assert.strictEqual(result.retryInMillis, undefined); }); it('forwards handled server error as failure', async function () { @@ -235,12 +239,12 @@ describe('GrpcExporterTransport', function () { }; const transport = new GrpcExporterTransport(simpleClientConfig); - const result = await transport.send(Buffer.from([])); + const result = (await transport.send( + Buffer.from([]) + )) as ExportResponseFailure; assert.strictEqual(result.status, 'failure'); assert.ok(types.isNativeError(result.error)); - assert.strictEqual(result.data, undefined); - assert.strictEqual(result.retryInMillis, undefined); }); it('forwards unhandled server error as failure', async function () { @@ -249,11 +253,11 @@ describe('GrpcExporterTransport', function () { }; const transport = new GrpcExporterTransport(simpleClientConfig); - const result = await transport.send(Buffer.from([])); + const result = (await transport.send( + Buffer.from([]) + )) as ExportResponseFailure; assert.strictEqual(result.status, 'failure'); assert.ok(types.isNativeError(result.error)); - assert.strictEqual(result.data, undefined); - assert.strictEqual(result.retryInMillis, undefined); }); it('forwards metadataProvider error as failure', async function () { @@ -265,11 +269,11 @@ describe('GrpcExporterTransport', function () { const transport = new GrpcExporterTransport(config); - const result = await transport.send(Buffer.from([])); + const result = (await transport.send( + Buffer.from([]) + )) as ExportResponseFailure; assert.strictEqual(result.status, 'failure'); assert.strictEqual(result.error, expectedError); - assert.strictEqual(result.data, undefined); - assert.strictEqual(result.retryInMillis, undefined); }); it('forwards metadataProvider returns null value as failure', async function () { @@ -281,11 +285,11 @@ describe('GrpcExporterTransport', function () { const transport = new GrpcExporterTransport(config); - const result = await transport.send(Buffer.from([])); + const result = (await transport.send( + Buffer.from([]) + )) as ExportResponseFailure; assert.strictEqual(result.status, 'failure'); assert.deepEqual(result.error, expectedError); - assert.strictEqual(result.data, undefined); - assert.strictEqual(result.retryInMillis, undefined); }); it('forwards credential error as failure', async function () { @@ -297,11 +301,11 @@ describe('GrpcExporterTransport', function () { const transport = new GrpcExporterTransport(config); - const result = await transport.send(Buffer.from([])); + const result = (await transport.send( + Buffer.from([]) + )) as ExportResponseFailure; assert.strictEqual(result.status, 'failure'); assert.strictEqual(result.error, expectedError); - assert.strictEqual(result.data, undefined); - assert.strictEqual(result.retryInMillis, undefined); }); }); }); From 231259e3e1d212b5ad426c90675a2f3a6a3acdf0 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Thu, 22 Feb 2024 16:26:23 +0100 Subject: [PATCH 11/13] feat: add comment about lazy-loading in utility functions --- .../otlp-grpc-exporter-base/src/grpc-exporter-transport.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts b/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts index 7ba618d6be7..10a19ed2c77 100644 --- a/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts +++ b/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts @@ -20,6 +20,7 @@ import type { Metadata, ServiceError, ChannelCredentials } from '@grpc/grpc-js'; import { ExportResponse } from './export-response'; import { IExporterTransport } from './exporter-transport'; +// values taken from '@grpc/grpc-js` so that we don't need to require/import it. const GRPC_COMPRESSION_NONE = 0; const GRPC_COMPRESSION_GZIP = 2; @@ -33,6 +34,7 @@ function toGrpcCompression(compression: 'gzip' | 'none'): number { } export function createInsecureCredentials(): ChannelCredentials { + // Lazy-load so that we don't need to require/import '@grpc/grpc-js' before it can be wrapped by instrumentation. const { credentials, // eslint-disable-next-line @typescript-eslint/no-var-requires @@ -45,6 +47,7 @@ export function createSslCredentials( privateKey?: Buffer, certChain?: Buffer ): ChannelCredentials { + // Lazy-load so that we don't need to require/import '@grpc/grpc-js' before it can be wrapped by instrumentation. const { credentials, // eslint-disable-next-line @typescript-eslint/no-var-requires From 638bc6f1984ded569b957686e2ffbfd946b83199 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Thu, 22 Feb 2024 16:26:42 +0100 Subject: [PATCH 12/13] feat: add comment about lazy-loading in utility functions --- .../otlp-grpc-exporter-base/src/grpc-exporter-transport.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts b/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts index 10a19ed2c77..57be7e5e285 100644 --- a/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts +++ b/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts @@ -56,6 +56,7 @@ export function createSslCredentials( } export function createEmptyMetadata(): Metadata { + // Lazy-load so that we don't need to require/import '@grpc/grpc-js' before it can be wrapped by instrumentation. const { Metadata, // eslint-disable-next-line @typescript-eslint/no-var-requires From c762e17619dc9ff0a2dabe6237dd2420dfc0cad5 Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Thu, 22 Feb 2024 16:27:26 +0100 Subject: [PATCH 13/13] feat: apply compression suggestion --- .../otlp-grpc-exporter-base/src/grpc-exporter-transport.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts b/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts index 57be7e5e285..77038648ce8 100644 --- a/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts +++ b/experimental/packages/otlp-grpc-exporter-base/src/grpc-exporter-transport.ts @@ -25,12 +25,7 @@ const GRPC_COMPRESSION_NONE = 0; const GRPC_COMPRESSION_GZIP = 2; function toGrpcCompression(compression: 'gzip' | 'none'): number { - if (compression === 'none') { - return GRPC_COMPRESSION_NONE; - } else if (compression === 'gzip') { - return GRPC_COMPRESSION_GZIP; - } - return GRPC_COMPRESSION_NONE; + return compression === 'gzip' ? GRPC_COMPRESSION_GZIP : GRPC_COMPRESSION_NONE; } export function createInsecureCredentials(): ChannelCredentials {