Skip to content

Commit

Permalink
feat!: move serialization to @opentelemetry/otlp-transformer (#4542)
Browse files Browse the repository at this point in the history
* feat!:move serializers to otlp-transformer

* feat!: use serializeres in protobuf and json exporters

* test(otlp-transformer): add tests for trace serializer

* test(otlp-transformer): add tests for metrics serializer

* test(otlp-transformer): add tests for logs serializer

* chore: resolve more conflicts

* fix: sync package-lock

* chore: cleanup dependencies, unused code, .gitignore

* chore: fix changelog indentation

* fix(otlp-transformer): remove unused useHex from JsonMetricsSerializer

* chore: add comment about how logs data is structured

* docs: move submodule.md, adapt contents

* fixup! Merge branch 'main' into feat/transformer-serializer

* fixup! Merge branch 'main' into feat/transformer-serializer
  • Loading branch information
pichlermarc committed Apr 26, 2024
1 parent 141b457 commit 8c12dd5
Show file tree
Hide file tree
Showing 51 changed files with 1,330 additions and 791 deletions.
6 changes: 3 additions & 3 deletions .gitmodules
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[submodule "experimental/packages/otlp-grpc-exporter-base/protos"]
path = experimental/packages/otlp-grpc-exporter-base/protos
url = https://github.com/open-telemetry/opentelemetry-proto.git
[submodule "experimental/packages/otlp-proto-exporter-base/protos"]
path = experimental/packages/otlp-proto-exporter-base/protos
url = https://github.com/open-telemetry/opentelemetry-proto.git
[submodule "experimental/packages/otlp-transformer/protos"]
path = experimental/packages/otlp-transformer/protos
url = https://github.com/open-telemetry/opentelemetry-proto.git
6 changes: 6 additions & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ All notable changes to experimental packages in this project will be documented

### :boom: Breaking Change

* feat(exporter-*-otlp-*)!: move serialization for Node.js exporters to `@opentelemetry/otlp-transformer` [#4542](https://github.com/open-telemetry/opentelemetry-js/pull/4542) @pichlermarc
* Breaking changes:
* (user-facing) `convert()` now returns an empty object and will be removed in a follow-up
* (internal) OTLPExporterNodeBase now has additional constructor parameters that are required
* (internal) OTLPExporterNodeBase now has an additional `ResponseType` type parameter

### :rocket: (Enhancement)

### :bug: (Bug Fix)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,11 @@ import {
OTLPGRPCExporterNodeBase,
validateAndNormalizeUrl,
DEFAULT_COLLECTOR_URL,
LogsSerializer,
} from '@opentelemetry/otlp-grpc-exporter-base';
import {
createExportLogsServiceRequest,
IExportLogsServiceRequest,
IExportLogsServiceResponse,
ProtobufLogsSerializer,
} from '@opentelemetry/otlp-transformer';
import { VERSION } from './version';

Expand Down Expand Up @@ -57,14 +56,10 @@ export class OTLPLogExporter
signalSpecificMetadata,
'LogsExportService',
'/opentelemetry.proto.collector.logs.v1.LogsService/Export',
LogsSerializer
ProtobufLogsSerializer
);
}

convert(logRecords: ReadableLogRecord[]): IExportLogsServiceRequest {
return createExportLogsServiceRequest(logRecords);
}

getDefaultUrl(config: OTLPGRPCExporterConfigNode) {
return validateAndNormalizeUrl(this.getUrlFromConfig(config));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,7 @@ import { VERSION } from '../src/version';

const logsServiceProtoPath =
'opentelemetry/proto/collector/logs/v1/logs_service.proto';
const includeDirs = [
path.resolve(__dirname, '../../otlp-grpc-exporter-base/protos'),
];
const includeDirs = [path.resolve(__dirname, '../../otlp-transformer/protos')];

const httpAddr = 'https://localhost:1503';
const udsAddr = 'unix:///tmp/otlp-logs.sock';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,16 @@ import type {
LogRecordExporter,
} from '@opentelemetry/sdk-logs';
import type { OTLPExporterNodeConfigBase } from '@opentelemetry/otlp-exporter-base';
import type { IExportLogsServiceRequest } from '@opentelemetry/otlp-transformer';
import type {
IExportLogsServiceRequest,
IExportLogsServiceResponse,
} from '@opentelemetry/otlp-transformer';
import { getEnv, baggageUtils } from '@opentelemetry/core';
import {
OTLPExporterNodeBase,
parseHeaders,
} from '@opentelemetry/otlp-exporter-base';
import { createExportLogsServiceRequest } from '@opentelemetry/otlp-transformer';
import { JsonLogsSerializer } from '@opentelemetry/otlp-transformer';

import { getDefaultUrl } from '../config';
import { VERSION } from '../../version';
Expand All @@ -38,15 +41,23 @@ const USER_AGENT = {
* Collector Logs Exporter for Node
*/
export class OTLPLogExporter
extends OTLPExporterNodeBase<ReadableLogRecord, IExportLogsServiceRequest>
extends OTLPExporterNodeBase<
ReadableLogRecord,
IExportLogsServiceRequest,
IExportLogsServiceResponse
>
implements LogRecordExporter
{
constructor(config: OTLPExporterNodeConfigBase = {}) {
// load OTEL_EXPORTER_OTLP_LOGS_TIMEOUT env
super({
timeoutMillis: getEnv().OTEL_EXPORTER_OTLP_LOGS_TIMEOUT,
...config,
});
super(
{
timeoutMillis: getEnv().OTEL_EXPORTER_OTLP_LOGS_TIMEOUT,
...config,
},
JsonLogsSerializer,
'application/json'
);
this.headers = {
...this.headers,
...USER_AGENT,
Expand All @@ -57,13 +68,6 @@ export class OTLPLogExporter
};
}

convert(logRecords: ReadableLogRecord[]): IExportLogsServiceRequest {
return createExportLogsServiceRequest(logRecords, {
useHex: true,
useLongBits: false,
});
}

getDefaultUrl(config: OTLPExporterNodeConfigBase): string {
return getDefaultUrl(config);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,14 @@ import {
OTLPExporterConfigBase,
appendResourcePathToUrl,
appendRootPathToUrlIfNeeded,
OTLPExporterNodeBase,
parseHeaders,
} from '@opentelemetry/otlp-exporter-base';
import { ServiceClientType } from '@opentelemetry/otlp-proto-exporter-base';
import {
OTLPProtoExporterNodeBase,
ServiceClientType,
} from '@opentelemetry/otlp-proto-exporter-base';
import {
createExportLogsServiceRequest,
IExportLogsServiceRequest,
IExportLogsServiceResponse,
ProtobufLogsSerializer,
} from '@opentelemetry/otlp-transformer';

import { ReadableLogRecord, LogRecordExporter } from '@opentelemetry/sdk-logs';
Expand All @@ -44,14 +43,15 @@ const DEFAULT_COLLECTOR_URL = `http://localhost:4318/${DEFAULT_COLLECTOR_RESOURC
* Collector Trace Exporter for Node
*/
export class OTLPLogExporter
extends OTLPProtoExporterNodeBase<
extends OTLPExporterNodeBase<
ReadableLogRecord,
IExportLogsServiceRequest
IExportLogsServiceRequest,
IExportLogsServiceResponse
>
implements LogRecordExporter
{
constructor(config: OTLPExporterConfigBase = {}) {
super(config);
super(config, ProtobufLogsSerializer, 'application/x-protobuf');
this.headers = {
...this.headers,
...USER_AGENT,
Expand All @@ -61,9 +61,6 @@ export class OTLPLogExporter
...parseHeaders(config?.headers),
};
}
convert(logs: ReadableLogRecord[]): IExportLogsServiceRequest {
return createExportLogsServiceRequest(logs);
}

getDefaultUrl(config: OTLPExporterConfigBase): string {
return typeof config.url === 'string'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,6 @@ describe('OTLPLogExporter - node with proto over http', () => {
});

it('should open the connection', done => {
collectorExporter.export(logs, () => {});

sinon.stub(http, 'request').callsFake((options: any, cb: any) => {
assert.strictEqual(options.hostname, 'foo.bar.com');
assert.strictEqual(options.method, 'POST');
Expand All @@ -206,11 +204,10 @@ describe('OTLPLogExporter - node with proto over http', () => {
done();
return fakeRequest as any;
});
collectorExporter.export(logs, () => {});
});

it('should set custom headers', done => {
collectorExporter.export(logs, () => {});

sinon.stub(http, 'request').callsFake((options: any, cb: any) => {
assert.strictEqual(options.headers['foo'], 'bar');

Expand All @@ -220,11 +217,10 @@ describe('OTLPLogExporter - node with proto over http', () => {
done();
return fakeRequest as any;
});
collectorExporter.export(logs, () => {});
});

it('should have keep alive and keepAliveMsecs option set', done => {
collectorExporter.export(logs, () => {});

sinon.stub(http, 'request').callsFake((options: any, cb: any) => {
assert.strictEqual(options.agent.keepAlive, true);
assert.strictEqual(options.agent.options.keepAliveMsecs, 2000);
Expand All @@ -235,6 +231,7 @@ describe('OTLPLogExporter - node with proto over http', () => {
done();
return fakeRequest as any;
});
collectorExporter.export(logs, () => {});
});

it('should successfully send the logs', done => {
Expand Down Expand Up @@ -271,35 +268,35 @@ describe('OTLPLogExporter - node with proto over http', () => {
// Need to stub/spy on the underlying logger as the "diag" instance is global
const spyLoggerError = sinon.stub(diag, 'error');

collectorExporter.export(logs, result => {
assert.strictEqual(result.code, ExportResultCode.SUCCESS);
assert.strictEqual(spyLoggerError.args.length, 0);
done();
});

sinon.stub(http, 'request').callsFake((options: any, cb: any) => {
const mockRes = new MockedResponse(200);
cb(mockRes);
mockRes.send('success');
return fakeRequest as any;
});
});

it('should log the error message', done => {
collectorExporter.export(logs, result => {
assert.strictEqual(result.code, ExportResultCode.FAILED);
// @ts-expect-error verify error code
assert.strictEqual(result.error.code, 400);
assert.strictEqual(result.code, ExportResultCode.SUCCESS);
assert.strictEqual(spyLoggerError.args.length, 0);
done();
});
});

it('should log the error message', done => {
sinon.stub(http, 'request').callsFake((options: any, cb: any) => {
const mockResError = new MockedResponse(400);
cb(mockResError);
mockResError.send('failed');

return fakeRequest as any;
});

collectorExporter.export(logs, result => {
assert.strictEqual(result.code, ExportResultCode.FAILED);
// @ts-expect-error verify error code
assert.strictEqual(result.error.code, 400);
done();
});
});
});
describe('export - with compression', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,11 @@ import {
OTLPGRPCExporterNodeBase,
validateAndNormalizeUrl,
DEFAULT_COLLECTOR_URL,
TraceSerializer,
} from '@opentelemetry/otlp-grpc-exporter-base';
import {
createExportTraceServiceRequest,
IExportTraceServiceRequest,
IExportTraceServiceResponse,
ProtobufTraceSerializer,
} from '@opentelemetry/otlp-transformer';
import { VERSION } from './version';

Expand Down Expand Up @@ -57,14 +56,10 @@ export class OTLPTraceExporter
signalSpecificMetadata,
'TraceExportService',
'/opentelemetry.proto.collector.trace.v1.TraceService/Export',
TraceSerializer
ProtobufTraceSerializer
);
}

convert(spans: ReadableSpan[]): IExportTraceServiceRequest {
return createExportTraceServiceRequest(spans);
}

getDefaultUrl(config: OTLPGRPCExporterConfigNode) {
return validateAndNormalizeUrl(this.getUrlFromConfig(config));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,7 @@ import {

const traceServiceProtoPath =
'opentelemetry/proto/collector/trace/v1/trace_service.proto';
const includeDirs = [
path.resolve(__dirname, '../../otlp-grpc-exporter-base/protos'),
];
const includeDirs = [path.resolve(__dirname, '../../otlp-transformer/protos')];

const httpAddr = 'https://localhost:1501';
const udsAddr = 'unix:///tmp/otlp-traces.sock';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,11 @@ import {
appendRootPathToUrlIfNeeded,
} from '@opentelemetry/otlp-exporter-base';
import {
createExportTraceServiceRequest,
IExportTraceServiceRequest,
IExportTraceServiceResponse,
} from '@opentelemetry/otlp-transformer';
import { VERSION } from '../../version';
import { JsonTraceSerializer } from '@opentelemetry/otlp-transformer';

const DEFAULT_COLLECTOR_RESOURCE_PATH = 'v1/traces';
const DEFAULT_COLLECTOR_URL = `http://localhost:4318/${DEFAULT_COLLECTOR_RESOURCE_PATH}`;
Expand All @@ -41,11 +42,15 @@ const USER_AGENT = {
* Collector Trace Exporter for Node
*/
export class OTLPTraceExporter
extends OTLPExporterNodeBase<ReadableSpan, IExportTraceServiceRequest>
extends OTLPExporterNodeBase<
ReadableSpan,
IExportTraceServiceRequest,
IExportTraceServiceResponse
>
implements SpanExporter
{
constructor(config: OTLPExporterNodeConfigBase = {}) {
super(config);
super(config, JsonTraceSerializer, 'application/json');
this.headers = {
...this.headers,
...USER_AGENT,
Expand All @@ -56,13 +61,6 @@ export class OTLPTraceExporter
};
}

convert(spans: ReadableSpan[]): IExportTraceServiceRequest {
return createExportTraceServiceRequest(spans, {
useHex: true,
useLongBits: false,
});
}

getDefaultUrl(config: OTLPExporterNodeConfigBase): string {
return typeof config.url === 'string'
? config.url
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,14 @@ import {
OTLPExporterNodeConfigBase,
appendResourcePathToUrl,
appendRootPathToUrlIfNeeded,
OTLPExporterNodeBase,
parseHeaders,
} from '@opentelemetry/otlp-exporter-base';
import { ServiceClientType } from '@opentelemetry/otlp-proto-exporter-base';
import {
OTLPProtoExporterNodeBase,
ServiceClientType,
} from '@opentelemetry/otlp-proto-exporter-base';
import {
createExportTraceServiceRequest,
IExportTraceServiceRequest,
IExportTraceServiceResponse,
ProtobufTraceSerializer,
} from '@opentelemetry/otlp-transformer';
import { VERSION } from '../../version';

Expand All @@ -42,11 +41,15 @@ const USER_AGENT = {
* Collector Trace Exporter for Node with protobuf
*/
export class OTLPTraceExporter
extends OTLPProtoExporterNodeBase<ReadableSpan, IExportTraceServiceRequest>
extends OTLPExporterNodeBase<
ReadableSpan,
IExportTraceServiceRequest,
IExportTraceServiceResponse
>
implements SpanExporter
{
constructor(config: OTLPExporterNodeConfigBase = {}) {
super(config);
super(config, ProtobufTraceSerializer, 'application/x-protobuf');
this.headers = {
...this.headers,
...USER_AGENT,
Expand All @@ -57,10 +60,6 @@ export class OTLPTraceExporter
};
}

convert(spans: ReadableSpan[]): IExportTraceServiceRequest {
return createExportTraceServiceRequest(spans);
}

getDefaultUrl(config: OTLPExporterNodeConfigBase) {
return typeof config.url === 'string'
? config.url
Expand Down
Loading

0 comments on commit 8c12dd5

Please sign in to comment.