Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(exporter-*-otlp-grpc)!: lazy load gRPC #4432

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
462cad6
fix(exporter-*-otlp-grpc)!: lazy load gRPC
pichlermarc Jan 18, 2024
9f12ea6
fix(otlp-grpc-exporter-base): add back tests
pichlermarc Jan 25, 2024
96e1c70
Merge branch 'main' into fix/grpc-lazy-load
pichlermarc Jan 25, 2024
34bc307
fix: test
pichlermarc Jan 25, 2024
25140c0
fix(changelog): add entry
pichlermarc Jan 25, 2024
06600ad
fix: lint
pichlermarc Jan 25, 2024
e6e39c1
test(otlp-exporter-grpc-base): test new GrpcExporterTransport
pichlermarc Jan 25, 2024
2bf2034
fix: set allowJs to true again
pichlermarc Jan 26, 2024
d96b67b
Merge branch 'main' into fix/grpc-lazy-load
pichlermarc Jan 26, 2024
3391a0f
fix: lint
pichlermarc Jan 26, 2024
5d93d5d
Merge branch 'main' into fix/grpc-lazy-load
pichlermarc Jan 31, 2024
1ab10f2
Merge branch 'main' into fix/grpc-lazy-load
pichlermarc Feb 1, 2024
4417b79
Merge branch 'main' into fix/grpc-lazy-load
pichlermarc Feb 5, 2024
595a6ee
Merge branch 'main' into fix/grpc-lazy-load
pichlermarc Feb 7, 2024
d248ffa
Merge branch 'main' into fix/grpc-lazy-load
pichlermarc Feb 12, 2024
2af561a
Merge branch 'main' into fix/grpc-lazy-load
pichlermarc Feb 14, 2024
02f703c
refactor: remove unnecessary/silly ts-ignore comment
pichlermarc Feb 22, 2024
69f4e7b
feat: split ExportResponse into success and failure cases
pichlermarc Feb 22, 2024
231259e
feat: add comment about lazy-loading in utility functions
pichlermarc Feb 22, 2024
638bc6f
feat: add comment about lazy-loading in utility functions
pichlermarc Feb 22, 2024
c762e17
feat: apply compression suggestion
pichlermarc Feb 22, 2024
2363dfc
Merge branch 'main' into fix/grpc-lazy-load
pichlermarc Feb 23, 2024
b3f8da8
Merge branch 'main' into fix/grpc-lazy-load
pichlermarc Feb 23, 2024
5fcbd8f
Merge branch 'main' into fix/grpc-lazy-load
pichlermarc Feb 23, 2024
50515cc
Merge branch 'main' into fix/grpc-lazy-load
pichlermarc Mar 6, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,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 <https://github.com/DataDog/import-in-the-middle/issues/57>
* 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.

### :bug: (Bug Fix)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,17 @@

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';
import { VERSION } from './version';

Expand All @@ -38,21 +38,27 @@ const USER_AGENT = {
* OTLP Logs Exporter for Node
*/
export class OTLPLogExporter
extends OTLPGRPCExporterNodeBase<ReadableLogRecord, IExportLogsServiceRequest>
extends OTLPGRPCExporterNodeBase<
ReadableLogRecord,
IExportLogsServiceRequest,
IExportLogsServiceResponse
>
implements LogRecordExporter
{
constructor(config: OTLPGRPCExporterConfigNode = {}) {
super(config);
const headers = {
const signalSpecificMetadata = {
...USER_AGENT,
...baggageUtils.parseKeyPairsIntoRecord(
getEnv().OTEL_EXPORTER_OTLP_LOGS_HEADERS
),
};
this.metadata ||= new Metadata();
for (const [k, v] of Object.entries(headers)) {
pichlermarc marked this conversation as resolved.
Show resolved Hide resolved
this.metadata.set(k, v);
}
super(
config,
signalSpecificMetadata,
'LogsExportService',
'/opentelemetry.proto.collector.logs.v1.LogsService/Export',
LogsSerializer
);
}

convert(logRecords: ReadableLogRecord[]): IExportLogsServiceRequest {
Expand All @@ -63,14 +69,6 @@ export class OTLPLogExporter
return validateAndNormalizeUrl(this.getUrlFromConfig(config));
}

getServiceClientType() {
return ServiceClientType.LOGS;
}

getServiceProtoPath(): string {
return 'opentelemetry/proto/collector/logs/v1/logs_service.proto';
}
Comment on lines -66 to -72
Copy link
Member

Choose a reason for hiding this comment

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

note: breaking change


getUrlFromConfig(config: OTLPGRPCExporterConfigNode): string {
if (typeof config.url === 'string') {
return config.url;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -292,7 +291,7 @@ const testCollectorExporter = (params: TestParams) => {
});
assert.strictEqual(
collectorExporter.compression,
GrpcCompressionAlgorithm.GZIP
CompressionAlgorithm.GZIP
);
delete envSource.OTEL_EXPORTER_OTLP_COMPRESSION;
});
Expand Down Expand Up @@ -320,44 +319,54 @@ 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 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}`,
]);
});
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']);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -38,21 +38,27 @@ const USER_AGENT = {
* OTLP Trace Exporter for Node
*/
export class OTLPTraceExporter
extends OTLPGRPCExporterNodeBase<ReadableSpan, IExportTraceServiceRequest>
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 {
Expand All @@ -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';
}
Comment on lines -66 to -72
Copy link
Member

Choose a reason for hiding this comment

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

note: breaking


getUrlFromConfig(config: OTLPGRPCExporterConfigNode): string {
if (typeof config.url === 'string') {
return config.url;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -302,7 +301,7 @@ const testCollectorExporter = (params: TestParams) => {
});
assert.strictEqual(
collectorExporter.compression,
GrpcCompressionAlgorithm.GZIP
CompressionAlgorithm.GZIP
);
delete envSource.OTEL_EXPORTER_OTLP_COMPRESSION;
});
Expand Down Expand Up @@ -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']);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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;
Comment on lines -61 to -66
Copy link
Member

Choose a reason for hiding this comment

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

note: breaking

super(
config,
signalSpecificMetadata,
'MetricsExportService',
'/opentelemetry.proto.collector.metrics.v1.MetricsService/Export',
MetricsSerializer
);
}

getDefaultUrl(config: OTLPGRPCExporterConfigNode): string {
Expand Down
Loading