Skip to content

Commit

Permalink
feat(exporters)!: rewrite exporter config logic (open-telemetry#4971)
Browse files Browse the repository at this point in the history
  • Loading branch information
@pichlermarc
pichlermarc authored Sep 30, 2024
1 parent 77f12c5 commit 3007d3e
Show file tree
Hide file tree
Showing 63 changed files with 2,722 additions and 2,759 deletions.
24 changes: 24 additions & 0 deletions experimental/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,30 @@ All notable changes to experimental packages in this project will be documented
* fix(sdk-events): remove devDependencies to old `@opentelemetry/[email protected]`, `@opentelemetry/[email protected]` packages [#5013](https://github.com/open-telemetry/opentelemetry-js/pull/5013) @pichlermarc
* fix(sdk-logs): remove devDependencies to old `@opentelemetry/[email protected]` [#5013](https://github.com/open-telemetry/opentelemetry-js/pull/5013) @pichlermarc
* fix(sdk-logs): align LogRecord#setAttribute type with types from `@opentelemetry/[email protected]` [#5013](https://github.com/open-telemetry/opentelemetry-js/pull/5013) @pichlermarc
* feat(exporter-*-otlp-*)!: rewrite exporter config logic for testability [#4971](https://github.com/open-telemetry/opentelemetry-js/pull/4971) @pichlermarc
* (user-facing) `getDefaultUrl` was intended for internal use has been removed from all exporters
* (user-facing) `getUrlFromConfig` was intended for internal use and has been removed from all exporters
* (user-facing) `hostname` was intended for internal use and has been removed from all exporters
* (user-facing) `url` was intended for internal use and has been removed from all exporters
* (user-facing) `timeoutMillis` was intended for internal use and has been removed from all exporters
* (user-facing) `onInit` was intended for internal use and has been removed from all exporters
* fix(exporter-*-otlp-*): fixes a bug where signal-specific environment variables would not be applied and the trace-specific one was used instead [#4971](https://github.com/open-telemetry/opentelemetry-js/pull/4971) @pichlermarc
* Fixes:
* `OTEL_EXPORTER_OTLP_METRICS_COMPRESSION`
* `OTEL_EXPORTER_OTLP_LOGS_COMPRESSION`
* `OTEL_EXPORTER_OTLP_METRICS_CLIENT_CERTIFICATE`
* `OTEL_EXPORTER_OTLP_LOGS_CLIENT_CERTIFICATE`
* `OTEL_EXPORTER_OTLP_METRICS_CLIENT_KEY`
* `OTEL_EXPORTER_OTLP_LOGS_CLIENT_KEY`
* `OTEL_EXPORTER_OTLP_METRICS_INSECURE`
* `OTEL_EXPORTER_OTLP_LOGS_INSECURE`
* feat(otlp-exporter-base)!: do not export functions that are intended for internal use [#4971](https://github.com/open-telemetry/opentelemetry-js/pull/4971) @pichlermarc
* Drops the following functions and types that were intended for internal use from the package exports:
* `parseHeaders`
* `appendResourcePathToUrl`
* `appendResourcePathToUrlIfNeeded`
* `configureExporterTimeout`
* `invalidTimeout`

### :books: (Refine Doc)

Expand Down
34 changes: 2 additions & 32 deletions experimental/packages/exporter-logs-otlp-grpc/src/OTLPLogExporter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,14 @@
*/

import { LogRecordExporter, ReadableLogRecord } from '@opentelemetry/sdk-logs';
import { baggageUtils, getEnv } from '@opentelemetry/core';
import {
OTLPGRPCExporterConfigNode,
OTLPGRPCExporterNodeBase,
validateAndNormalizeUrl,
DEFAULT_COLLECTOR_URL,
} from '@opentelemetry/otlp-grpc-exporter-base';
import {
IExportLogsServiceResponse,
ProtobufLogsSerializer,
} from '@opentelemetry/otlp-transformer';
import { VERSION } from './version';

const USER_AGENT = {
'User-Agent': `OTel-OTLP-Exporter-JavaScript/${VERSION}`,
};

/**
* OTLP Logs Exporter for Node
Expand All @@ -43,34 +35,12 @@ export class OTLPLogExporter
implements LogRecordExporter
{
constructor(config: OTLPGRPCExporterConfigNode = {}) {
const signalSpecificMetadata = {
...USER_AGENT,
...baggageUtils.parseKeyPairsIntoRecord(
getEnv().OTEL_EXPORTER_OTLP_LOGS_HEADERS
),
};
super(
config,
signalSpecificMetadata,
ProtobufLogsSerializer,
'LogsExportService',
'/opentelemetry.proto.collector.logs.v1.LogsService/Export',
ProtobufLogsSerializer
);
}

getDefaultUrl(config: OTLPGRPCExporterConfigNode) {
return validateAndNormalizeUrl(this.getUrlFromConfig(config));
}

getUrlFromConfig(config: OTLPGRPCExporterConfigNode): string {
if (typeof config.url === 'string') {
return config.url;
}

return (
getEnv().OTEL_EXPORTER_OTLP_LOGS_ENDPOINT ||
getEnv().OTEL_EXPORTER_OTLP_ENDPOINT ||
DEFAULT_COLLECTOR_URL
'LOGS'
);
}
}
96 changes: 0 additions & 96 deletions experimental/packages/exporter-logs-otlp-grpc/test/OTLPLogExporter.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import {
IExportLogsServiceRequest,
IResourceLogs,
} from '@opentelemetry/otlp-transformer';
import { VERSION } from '../src/version';

const logsServiceProtoPath =
'opentelemetry/proto/collector/logs/v1/logs_service.proto';
Expand Down Expand Up @@ -294,104 +293,9 @@ const testCollectorExporter = (params: TestParams) => {
}, 500);
});
});
describe('Logs Exporter with compression', () => {
const envSource = process.env;
it('should return gzip compression algorithm on exporter', () => {
const credentials = useTLS
? grpc.credentials.createSsl(
fs.readFileSync('./test/certs/ca.crt'),
fs.readFileSync('./test/certs/client.key'),
fs.readFileSync('./test/certs/client.crt')
)
: grpc.credentials.createInsecure();

envSource.OTEL_EXPORTER_OTLP_COMPRESSION = 'gzip';
collectorExporter = new OTLPLogExporter({
url: address,
credentials,
metadata: metadata,
});
assert.strictEqual(
collectorExporter.compression,
CompressionAlgorithm.GZIP
);
delete envSource.OTEL_EXPORTER_OTLP_COMPRESSION;
});
});
});
};

describe('OTLPLogExporter - node (getDefaultUrl)', () => {
it('should default to localhost', done => {
const collectorExporter = new OTLPLogExporter({});
setTimeout(() => {
assert.strictEqual(collectorExporter['url'], 'localhost:4317');
done();
});
});
it('should keep the URL if included', done => {
const url = 'http://foo.bar.com';
const collectorExporter = new OTLPLogExporter({ url });
setTimeout(() => {
assert.strictEqual(collectorExporter['url'], 'foo.bar.com');
done();
});
});
});

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');
});
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');
});
it('should include user-agent header by default', () => {
const collectorExporter = new OTLPLogExporter();
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();
const actualMetadata =
collectorExporter['_transport']['_parameters'].metadata();
assert.deepStrictEqual(actualMetadata.get('foo'), ['bar']);
});
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 });
const actualMetadata =
collectorExporter['_transport']['_parameters'].metadata();
assert.deepStrictEqual(actualMetadata.get('foo'), ['bar']);
assert.deepStrictEqual(actualMetadata.get('goo'), ['lol']);
assert.deepStrictEqual(actualMetadata.get('bar'), ['foo']);
});
});

testCollectorExporter({ useTLS: true });
testCollectorExporter({ useTLS: false });
testCollectorExporter({ metadata });
Expand Down
11 changes: 4 additions & 7 deletions experimental/packages/exporter-logs-otlp-http/src/platform/browser/OTLPLogExporter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ import type { IExportLogsServiceResponse } from '@opentelemetry/otlp-transformer
import { OTLPExporterBrowserBase } from '@opentelemetry/otlp-exporter-base';
import { JsonLogsSerializer } from '@opentelemetry/otlp-transformer';

import { getDefaultUrl } from '../config';

/**
* Collector Logs Exporter for Web
*/
Expand All @@ -38,11 +36,10 @@ export class OTLPLogExporter
...config,
},
JsonLogsSerializer,
'application/json'
{
'Content-Type': 'application/json',
},
'v1/logs'
);
}

getDefaultUrl(config: OTLPExporterConfigBase): string {
return getDefaultUrl(config);
}
}
50 changes: 0 additions & 50 deletions experimental/packages/exporter-logs-otlp-http/src/platform/config.ts
View file

This file was deleted.

21 changes: 4 additions & 17 deletions experimental/packages/exporter-logs-otlp-http/src/platform/node/OTLPLogExporter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,9 @@ import type {
} from '@opentelemetry/sdk-logs';
import type { OTLPExporterNodeConfigBase } from '@opentelemetry/otlp-exporter-base';
import type { IExportLogsServiceResponse } from '@opentelemetry/otlp-transformer';
import { getEnv, baggageUtils } from '@opentelemetry/core';
import {
OTLPExporterNodeBase,
parseHeaders,
} from '@opentelemetry/otlp-exporter-base';
import { OTLPExporterNodeBase } from '@opentelemetry/otlp-exporter-base';
import { JsonLogsSerializer } from '@opentelemetry/otlp-transformer';

import { getDefaultUrl } from '../config';
import { VERSION } from '../../version';

const USER_AGENT = {
Expand All @@ -42,25 +37,17 @@ export class OTLPLogExporter
implements LogRecordExporter
{
constructor(config: OTLPExporterNodeConfigBase = {}) {
// load OTEL_EXPORTER_OTLP_LOGS_TIMEOUT env
super(
{
timeoutMillis: getEnv().OTEL_EXPORTER_OTLP_LOGS_TIMEOUT,
...config,
},
JsonLogsSerializer,
{
...baggageUtils.parseKeyPairsIntoRecord(
getEnv().OTEL_EXPORTER_OTLP_LOGS_HEADERS
),
...parseHeaders(config?.headers),
...USER_AGENT,
'Content-Type': 'application/json',
}
},
'LOGS',
'v1/logs'
);
}

getDefaultUrl(config: OTLPExporterNodeConfigBase): string {
return getDefaultUrl(config);
}
}
11 changes: 0 additions & 11 deletions experimental/packages/exporter-logs-otlp-http/test/browser/OTLPLogExporter.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import * as assert from 'assert';
import * as sinon from 'sinon';

import * as Config from '../../src/platform/config';
import { OTLPLogExporter } from '../../src/platform/browser';
import { OTLPExporterConfigBase } from '@opentelemetry/otlp-exporter-base';
import { ReadableLogRecord } from '@opentelemetry/sdk-logs';
Expand All @@ -38,16 +37,6 @@ describe('OTLPLogExporter', () => {
});
});

describe('getDefaultUrl', () => {
it('should call getDefaultUrl', () => {
const getDefaultUrl = sinon.stub(Config, 'getDefaultUrl');
const exporter = new OTLPLogExporter();
exporter.getDefaultUrl({});
// this callCount is 2, because new OTLPLogExporter also call it
assert.strictEqual(getDefaultUrl.callCount, 2);
});
});

describe('export - common', () => {
let spySend: any;
beforeEach(() => {
Expand Down
Loading

0 comments on commit 3007d3e

Please sign in to comment.