From e5b09677e7ed3bbe42570fab3c54a2075c3ad06f Mon Sep 17 00:00:00 2001 From: Nico385412 Date: Thu, 8 Jun 2023 11:55:28 +0200 Subject: [PATCH 1/7] fix(exporter-logs-otlp-http): set useHex to true --- .../exporter-logs-otlp-http/package.json | 1 + .../src/platform/node/OTLPLogExporter.ts | 2 +- .../test/node/OTLPLogExporter.test.ts | 16 ++++++++++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/experimental/packages/exporter-logs-otlp-http/package.json b/experimental/packages/exporter-logs-otlp-http/package.json index 19ec49940b..ff351eb5a9 100644 --- a/experimental/packages/exporter-logs-otlp-http/package.json +++ b/experimental/packages/exporter-logs-otlp-http/package.json @@ -90,6 +90,7 @@ "karma-webpack": "4.0.2", "lerna": "7.0.2", "mocha": "10.2.0", + "mock-require": "^3.0.3", "nyc": "15.1.0", "sinon": "15.1.2", "ts-loader": "8.4.0", diff --git a/experimental/packages/exporter-logs-otlp-http/src/platform/node/OTLPLogExporter.ts b/experimental/packages/exporter-logs-otlp-http/src/platform/node/OTLPLogExporter.ts index 7f7c538df7..a1d101e87c 100644 --- a/experimental/packages/exporter-logs-otlp-http/src/platform/node/OTLPLogExporter.ts +++ b/experimental/packages/exporter-logs-otlp-http/src/platform/node/OTLPLogExporter.ts @@ -48,7 +48,7 @@ export class OTLPLogExporter } convert(logRecords: ReadableLogRecord[]): IExportLogsServiceRequest { - return createExportLogsServiceRequest(logRecords); + return createExportLogsServiceRequest(logRecords, true); } getDefaultUrl(config: OTLPExporterNodeConfigBase): string { diff --git a/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts b/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts index aa0d345348..9c187776e4 100644 --- a/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts +++ b/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts @@ -16,8 +16,13 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; +import { default as mock } from 'mock-require'; + +const fakeOTLPTransformer = { createExportLogsServiceRequest: sinon.stub() }; +mock('@opentelemetry/otlp-transformer', fakeOTLPTransformer); import * as Config from '../../src/platform/config'; + import { OTLPLogExporter } from '../../src/platform/node'; describe('OTLPLogExporter', () => { @@ -61,4 +66,15 @@ describe('OTLPLogExporter', () => { assert.strictEqual(getDefaultUrl.callCount, 2); }); }); + + describe('convert', () => { + it('should call createExportLogsServiceRequest with useHex parameter to true', () => { + const exporter = new OTLPLogExporter(); + exporter.convert([]); + assert.strictEqual( + fakeOTLPTransformer.createExportLogsServiceRequest.calledWith([], true), + true + ); + }); + }); }); From c55af6053d8ef715a4ea182b1fbe54c9735dea2f Mon Sep 17 00:00:00 2001 From: Nico385412 Date: Thu, 8 Jun 2023 12:14:12 +0200 Subject: [PATCH 2/7] chore: update changelog --- experimental/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 1ac87111b5..4ce74a2709 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -14,6 +14,8 @@ All notable changes to experimental packages in this project will be documented ### :bug: (Bug Fix) +* fix(exporter-logs-otlp-http): set useHex to true [#3875](https://github.com/open-telemetry/opentelemetry-js/pull/3875) @Nico385412 + ### :books: (Refine Doc) ### :house: (Internal) From da79112ec75b311cc470002c34c384e617a7d2ae Mon Sep 17 00:00:00 2001 From: Nico385412 Date: Thu, 8 Jun 2023 13:18:15 +0200 Subject: [PATCH 3/7] fix(typo): package.json --- experimental/packages/exporter-logs-otlp-http/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/packages/exporter-logs-otlp-http/package.json b/experimental/packages/exporter-logs-otlp-http/package.json index ff351eb5a9..38ead618b3 100644 --- a/experimental/packages/exporter-logs-otlp-http/package.json +++ b/experimental/packages/exporter-logs-otlp-http/package.json @@ -90,7 +90,7 @@ "karma-webpack": "4.0.2", "lerna": "7.0.2", "mocha": "10.2.0", - "mock-require": "^3.0.3", + "mock-require": "3.0.3", "nyc": "15.1.0", "sinon": "15.1.2", "ts-loader": "8.4.0", From 1b5d06971d1292dba84876cf3215e4a3f9f0913e Mon Sep 17 00:00:00 2001 From: Nico385412 Date: Fri, 9 Jun 2023 16:47:23 +0200 Subject: [PATCH 4/7] chore: apply code review --- .../exporter-logs-otlp-http/package.json | 1 - .../exporter-logs-otlp-http/test/logHelper.ts | 186 ++++++++++++++++++ .../test/node/OTLPLogExporter.test.ts | 156 +++++++++++++-- .../exporter-logs-otlp-http/tsconfig.json | 1 - 4 files changed, 329 insertions(+), 15 deletions(-) create mode 100644 experimental/packages/exporter-logs-otlp-http/test/logHelper.ts diff --git a/experimental/packages/exporter-logs-otlp-http/package.json b/experimental/packages/exporter-logs-otlp-http/package.json index 38ead618b3..19ec49940b 100644 --- a/experimental/packages/exporter-logs-otlp-http/package.json +++ b/experimental/packages/exporter-logs-otlp-http/package.json @@ -90,7 +90,6 @@ "karma-webpack": "4.0.2", "lerna": "7.0.2", "mocha": "10.2.0", - "mock-require": "3.0.3", "nyc": "15.1.0", "sinon": "15.1.2", "ts-loader": "8.4.0", diff --git a/experimental/packages/exporter-logs-otlp-http/test/logHelper.ts b/experimental/packages/exporter-logs-otlp-http/test/logHelper.ts new file mode 100644 index 0000000000..6ce45632df --- /dev/null +++ b/experimental/packages/exporter-logs-otlp-http/test/logHelper.ts @@ -0,0 +1,186 @@ +/* + * 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 { HrTime, TraceFlags } from '@opentelemetry/api'; +import { SeverityNumber } from '@opentelemetry/api-logs'; +import { Resource } from '@opentelemetry/resources'; +import * as assert from 'assert'; +import { VERSION } from '@opentelemetry/core'; +import { + IAnyValue, + IExportLogsServiceRequest, + IKeyValue, + ILogRecord, + IResource, +} from '@opentelemetry/otlp-transformer'; +import { ReadableLogRecord } from '@opentelemetry/sdk-logs'; +import { Stream } from 'stream'; + +export const mockedReadableLogRecord: ReadableLogRecord = { + resource: Resource.default().merge( + new Resource({ + 'resource-attribute': 'some resource-attr value', + }) + ), + instrumentationScope: { + name: 'scope_name_1', + version: '0.1.0', + schemaUrl: 'http://url.to.schema', + }, + hrTime: [1680253513, 123241635] as HrTime, + hrTimeObserved: [1680253513, 123241635] as HrTime, + attributes: { + 'some-attribute': 'some attribute value', + }, + severityNumber: SeverityNumber.ERROR, + severityText: 'error', + body: 'some_log_body', + spanContext: { + traceFlags: TraceFlags.SAMPLED, + traceId: '1f1008dc8e270e85c40a0d7c3939b278', + spanId: '5e107261f64fa53e', + }, +}; +export function ensureExportedAttributesAreCorrect(attributes: IKeyValue[]) { + assert.deepStrictEqual( + attributes, + [ + { + key: 'some-attribute', + value: { + stringValue: 'some attribute value', + }, + }, + ], + 'exported attributes are incorrect' + ); +} + +export function ensureExportedBodyIsCorrect(body?: IAnyValue) { + assert.deepStrictEqual( + body, + { stringValue: 'some_log_body' }, + 'exported attributes are incorrect' + ); +} + +export function ensureExportedLogRecordIsCorrect(logRecord: ILogRecord) { + ensureExportedBodyIsCorrect(logRecord.body); + ensureExportedAttributesAreCorrect(logRecord.attributes); + assert.strictEqual( + logRecord.timeUnixNano, + 1680253513123241700, + 'timeUnixNano is wrong' + ); + assert.strictEqual( + logRecord.observedTimeUnixNano, + 1680253513123241700, + 'observedTimeUnixNano is wrong' + ); + assert.strictEqual( + logRecord.severityNumber, + SeverityNumber.ERROR, + 'severityNumber is wrong' + ); + assert.strictEqual(logRecord.severityText, 'error', 'severityText is wrong'); + assert.strictEqual( + logRecord.droppedAttributesCount, + 0, + 'droppedAttributesCount is wrong' + ); + assert.strictEqual(logRecord.flags, TraceFlags.SAMPLED, 'flags is wrong'); +} + +export function ensureResourceIsCorrect(resource: IResource) { + assert.deepStrictEqual(resource, { + attributes: [ + { + key: 'service.name', + value: { + stringValue: `unknown_service:${process.argv0}`, + value: 'stringValue', + }, + }, + { + key: 'telemetry.sdk.language', + value: { + stringValue: 'nodejs', + value: 'stringValue', + }, + }, + { + key: 'telemetry.sdk.name', + value: { + stringValue: 'opentelemetry', + value: 'stringValue', + }, + }, + { + key: 'telemetry.sdk.version', + value: { + stringValue: VERSION, + value: 'stringValue', + }, + }, + { + key: 'resource-attribute', + value: { + stringValue: 'some resource-attr value', + value: 'stringValue', + }, + }, + ], + droppedAttributesCount: 0, + }); +} + +export function ensureExportLogsServiceRequestIsSet( + json: IExportLogsServiceRequest +) { + const resourceLogs = json.resourceLogs; + assert.strictEqual(resourceLogs?.length, 1, 'resourceLogs is missing'); + + const resource = resourceLogs?.[0].resource; + assert.ok(resource, 'resource is missing'); + + const scopeLogs = resourceLogs?.[0].scopeLogs; + assert.strictEqual(scopeLogs?.length, 1, 'scopeLogs is missing'); + + const scope = scopeLogs?.[0].scope; + assert.ok(scope, 'scope is missing'); + + const logRecords = scopeLogs?.[0].logRecords; + assert.strictEqual(logRecords?.length, 1, 'logs are missing'); +} + +export class MockedResponse extends Stream { + constructor(private _code: number, private _msg?: string) { + super(); + } + + send(data: string) { + this.emit('data', data); + this.emit('end'); + } + + get statusCode() { + return this._code; + } + + get statusMessage() { + return this._msg; + } +} diff --git a/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts b/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts index 9c187776e4..60a7638041 100644 --- a/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts +++ b/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts @@ -14,19 +14,32 @@ * limitations under the License. */ +import { diag } from '@opentelemetry/api'; import * as assert from 'assert'; +import * as http from 'http'; import * as sinon from 'sinon'; -import { default as mock } from 'mock-require'; - -const fakeOTLPTransformer = { createExportLogsServiceRequest: sinon.stub() }; -mock('@opentelemetry/otlp-transformer', fakeOTLPTransformer); - import * as Config from '../../src/platform/config'; import { OTLPLogExporter } from '../../src/platform/node'; +import { OTLPExporterNodeConfigBase } from '@opentelemetry/otlp-exporter-base'; +import { ReadableLogRecord } from '@opentelemetry/sdk-logs'; +import { MockedResponse, ensureExportLogsServiceRequestIsSet, ensureExportedLogRecordIsCorrect, mockedReadableLogRecord } from '../logHelper'; +import { PassThrough, Stream } from 'stream'; +import { IExportLogsServiceRequest } from '@opentelemetry/otlp-transformer'; +import { ExportResultCode } from '@opentelemetry/core'; + +let fakeRequest: PassThrough; describe('OTLPLogExporter', () => { let envSource: Record; + let collectorExporter: OTLPLogExporter; + let collectorExporterConfig: OTLPExporterNodeConfigBase; + let logs: ReadableLogRecord[]; + + afterEach(() => { + fakeRequest = new Stream.PassThrough(); + sinon.restore(); + }); if (typeof process === 'undefined') { envSource = globalThis as unknown as Record; @@ -67,14 +80,131 @@ describe('OTLPLogExporter', () => { }); }); - describe('convert', () => { - it('should call createExportLogsServiceRequest with useHex parameter to true', () => { - const exporter = new OTLPLogExporter(); - exporter.convert([]); - assert.strictEqual( - fakeOTLPTransformer.createExportLogsServiceRequest.calledWith([], true), - true - ); + describe('export', () => { + beforeEach(() => { + collectorExporterConfig = { + headers: { + foo: 'bar', + }, + hostname: 'foo', + url: 'http://foo.bar.com', + keepAlive: true, + httpAgentOptions: { keepAliveMsecs: 2000 }, + }; + collectorExporter = new OTLPLogExporter(collectorExporterConfig); + logs = []; + logs.push(Object.assign({}, mockedReadableLogRecord)); + }); + afterEach(() => { + sinon.restore(); + }); + + it('should open the connection', done => { + sinon.stub(http, 'request').callsFake((options: any, cb: any) => { + assert.strictEqual(options.hostname, 'foo.bar.com'); + assert.strictEqual(options.method, 'POST'); + assert.strictEqual(options.path, '/'); + + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send('success'); + done(); + return fakeRequest as any; + }); + collectorExporter.export(logs, () => {}); + }); + + it('should set custom headers', done => { + + sinon.stub(http, 'request').callsFake((options: any, cb: any) => { + assert.strictEqual(options.headers['foo'], 'bar'); + + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send('success'); + done(); + return fakeRequest as any; + }); + + collectorExporter.export(logs, () => {}); + }); + + it('should have keep alive and keepAliveMsecs option set', done => { + + sinon.stub(http, 'request').callsFake((options: any, cb: any) => { + assert.strictEqual(options.agent.keepAlive, true); + assert.strictEqual(options.agent.options.keepAliveMsecs, 2000); + + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send('success'); + done(); + return fakeRequest as any; + }); + + collectorExporter.export(logs, () => {}); + }); + + it('should successfully send the logs', done => { + const fakeRequest = new Stream.PassThrough(); + sinon.stub(http, 'request').returns(fakeRequest as any); + + let buff = Buffer.from(''); + fakeRequest.on('end', () => { + const responseBody = buff.toString(); + const json = JSON.parse(responseBody) as IExportLogsServiceRequest; + const log1 = json.resourceLogs?.[0].scopeLogs?.[0].logRecords?.[0]; + assert.ok(typeof log1 !== 'undefined', "log doesn't exist"); + ensureExportedLogRecordIsCorrect(log1); + + ensureExportLogsServiceRequestIsSet(json); + + done(); + }); + + fakeRequest.on('data', chunk => { + buff = Buffer.concat([buff, chunk]); + }); + + const clock = sinon.useFakeTimers(); + collectorExporter.export(logs, () => {}); + clock.tick(200); + clock.restore(); + }); + + it('should log the successful message', done => { + // Need to stub/spy on the underlying logger as the "diag" instance is global + const spyLoggerError = sinon.stub(diag, 'error'); + + sinon.stub(http, 'request').callsFake((options: any, cb: any) => { + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send('success'); + return fakeRequest as any; + }); + + collectorExporter.export(logs, result => { + 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(); + }); }); }); }); diff --git a/experimental/packages/exporter-logs-otlp-http/tsconfig.json b/experimental/packages/exporter-logs-otlp-http/tsconfig.json index c4eda837a9..e42e043148 100644 --- a/experimental/packages/exporter-logs-otlp-http/tsconfig.json +++ b/experimental/packages/exporter-logs-otlp-http/tsconfig.json @@ -1,7 +1,6 @@ { "extends": "../../../tsconfig.base.json", "compilerOptions": { - "esModuleInterop": true, "outDir": "build", "rootDir": "." }, From 8b85f87b209056d89d098e688cb059b796a015c2 Mon Sep 17 00:00:00 2001 From: Nico385412 Date: Tue, 13 Jun 2023 08:36:42 +0200 Subject: [PATCH 5/7] fix(otlp-log-exporter-http): usehex for browser module --- .../src/platform/browser/OTLPLogExporter.ts | 2 +- .../test/browser/OTLPLogExporter.test.ts | 144 ++++++++++++++++++ 2 files changed, 145 insertions(+), 1 deletion(-) diff --git a/experimental/packages/exporter-logs-otlp-http/src/platform/browser/OTLPLogExporter.ts b/experimental/packages/exporter-logs-otlp-http/src/platform/browser/OTLPLogExporter.ts index ef837614e2..a7ecbbac95 100644 --- a/experimental/packages/exporter-logs-otlp-http/src/platform/browser/OTLPLogExporter.ts +++ b/experimental/packages/exporter-logs-otlp-http/src/platform/browser/OTLPLogExporter.ts @@ -48,7 +48,7 @@ export class OTLPLogExporter } convert(logRecords: ReadableLogRecord[]): IExportLogsServiceRequest { - return createExportLogsServiceRequest(logRecords); + return createExportLogsServiceRequest(logRecords, true); } getDefaultUrl(config: OTLPExporterConfigBase): string { diff --git a/experimental/packages/exporter-logs-otlp-http/test/browser/OTLPLogExporter.test.ts b/experimental/packages/exporter-logs-otlp-http/test/browser/OTLPLogExporter.test.ts index 66958a9fef..1ca3368308 100644 --- a/experimental/packages/exporter-logs-otlp-http/test/browser/OTLPLogExporter.test.ts +++ b/experimental/packages/exporter-logs-otlp-http/test/browser/OTLPLogExporter.test.ts @@ -14,14 +14,32 @@ * limitations under the License. */ +import { diag } from '@opentelemetry/api'; import * as assert from 'assert'; import * as sinon from 'sinon'; +import * as http from 'http'; 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'; +import { MockedResponse, ensureExportLogsServiceRequestIsSet, ensureExportedLogRecordIsCorrect, mockedReadableLogRecord } from '../logHelper'; +import { PassThrough, Stream } from 'stream'; +import { IExportLogsServiceRequest } from '@opentelemetry/otlp-transformer'; +import { ExportResultCode } from '@opentelemetry/core'; + +let fakeRequest: PassThrough; describe('OTLPLogExporter', () => { let envSource: Record; + let collectorExporter: OTLPLogExporter; + let collectorExporterConfig: OTLPExporterConfigBase; + let logs: ReadableLogRecord[]; + + afterEach(() => { + fakeRequest = new Stream.PassThrough(); + sinon.restore(); + }); if (typeof process === 'undefined') { envSource = globalThis as unknown as Record; @@ -61,4 +79,130 @@ describe('OTLPLogExporter', () => { assert.strictEqual(getDefaultUrl.callCount, 2); }); }); + + describe('export', () => { + beforeEach(() => { + collectorExporterConfig = { + headers: { + foo: 'bar', + }, + hostname: 'foo', + url: 'http://foo.bar.com' + }; + collectorExporter = new OTLPLogExporter(collectorExporterConfig); + logs = []; + logs.push(Object.assign({}, mockedReadableLogRecord)); + }); + afterEach(() => { + sinon.restore(); + }); + + it('should open the connection', done => { + sinon.stub(http, 'request').callsFake((options: any, cb: any) => { + assert.strictEqual(options.hostname, 'foo.bar.com'); + assert.strictEqual(options.method, 'POST'); + assert.strictEqual(options.path, '/'); + + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send('success'); + done(); + return fakeRequest as any; + }); + collectorExporter.export(logs, () => {}); + }); + + it('should set custom headers', done => { + + sinon.stub(http, 'request').callsFake((options: any, cb: any) => { + assert.strictEqual(options.headers['foo'], 'bar'); + + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send('success'); + done(); + return fakeRequest as any; + }); + + collectorExporter.export(logs, () => {}); + }); + + it('should have keep alive and keepAliveMsecs option set', done => { + + sinon.stub(http, 'request').callsFake((options: any, cb: any) => { + assert.strictEqual(options.agent.keepAlive, true); + assert.strictEqual(options.agent.options.keepAliveMsecs, 2000); + + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send('success'); + done(); + return fakeRequest as any; + }); + + collectorExporter.export(logs, () => {}); + }); + + it('should successfully send the logs', done => { + const fakeRequest = new Stream.PassThrough(); + sinon.stub(http, 'request').returns(fakeRequest as any); + + let buff = Buffer.from(''); + fakeRequest.on('end', () => { + const responseBody = buff.toString(); + const json = JSON.parse(responseBody) as IExportLogsServiceRequest; + const log1 = json.resourceLogs?.[0].scopeLogs?.[0].logRecords?.[0]; + assert.ok(typeof log1 !== 'undefined', "log doesn't exist"); + ensureExportedLogRecordIsCorrect(log1); + + ensureExportLogsServiceRequestIsSet(json); + + done(); + }); + + fakeRequest.on('data', chunk => { + buff = Buffer.concat([buff, chunk]); + }); + + const clock = sinon.useFakeTimers(); + collectorExporter.export(logs, () => {}); + clock.tick(200); + clock.restore(); + }); + + it('should log the successful message', done => { + // Need to stub/spy on the underlying logger as the "diag" instance is global + const spyLoggerError = sinon.stub(diag, 'error'); + + sinon.stub(http, 'request').callsFake((options: any, cb: any) => { + const mockRes = new MockedResponse(200); + cb(mockRes); + mockRes.send('success'); + return fakeRequest as any; + }); + + collectorExporter.export(logs, result => { + 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(); + }); + }); + }); }); From 6600955cb20b819e618bac5b9a366949b03a8e5d Mon Sep 17 00:00:00 2001 From: Nico385412 Date: Tue, 13 Jun 2023 09:30:47 +0200 Subject: [PATCH 6/7] fix: linter --- .../test/browser/OTLPLogExporter.test.ts | 11 +++++++---- .../test/node/OTLPLogExporter.test.ts | 9 ++++++--- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/experimental/packages/exporter-logs-otlp-http/test/browser/OTLPLogExporter.test.ts b/experimental/packages/exporter-logs-otlp-http/test/browser/OTLPLogExporter.test.ts index 1ca3368308..c6c8b7d741 100644 --- a/experimental/packages/exporter-logs-otlp-http/test/browser/OTLPLogExporter.test.ts +++ b/experimental/packages/exporter-logs-otlp-http/test/browser/OTLPLogExporter.test.ts @@ -23,7 +23,12 @@ 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'; -import { MockedResponse, ensureExportLogsServiceRequestIsSet, ensureExportedLogRecordIsCorrect, mockedReadableLogRecord } from '../logHelper'; +import { + MockedResponse, + ensureExportLogsServiceRequestIsSet, + ensureExportedLogRecordIsCorrect, + mockedReadableLogRecord, +} from '../logHelper'; import { PassThrough, Stream } from 'stream'; import { IExportLogsServiceRequest } from '@opentelemetry/otlp-transformer'; import { ExportResultCode } from '@opentelemetry/core'; @@ -87,7 +92,7 @@ describe('OTLPLogExporter', () => { foo: 'bar', }, hostname: 'foo', - url: 'http://foo.bar.com' + url: 'http://foo.bar.com', }; collectorExporter = new OTLPLogExporter(collectorExporterConfig); logs = []; @@ -113,7 +118,6 @@ describe('OTLPLogExporter', () => { }); it('should set custom headers', done => { - sinon.stub(http, 'request').callsFake((options: any, cb: any) => { assert.strictEqual(options.headers['foo'], 'bar'); @@ -128,7 +132,6 @@ describe('OTLPLogExporter', () => { }); it('should have keep alive and keepAliveMsecs option set', done => { - sinon.stub(http, 'request').callsFake((options: any, cb: any) => { assert.strictEqual(options.agent.keepAlive, true); assert.strictEqual(options.agent.options.keepAliveMsecs, 2000); diff --git a/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts b/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts index 60a7638041..71d34d9160 100644 --- a/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts +++ b/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts @@ -23,7 +23,12 @@ import * as Config from '../../src/platform/config'; import { OTLPLogExporter } from '../../src/platform/node'; import { OTLPExporterNodeConfigBase } from '@opentelemetry/otlp-exporter-base'; import { ReadableLogRecord } from '@opentelemetry/sdk-logs'; -import { MockedResponse, ensureExportLogsServiceRequestIsSet, ensureExportedLogRecordIsCorrect, mockedReadableLogRecord } from '../logHelper'; +import { + MockedResponse, + ensureExportLogsServiceRequestIsSet, + ensureExportedLogRecordIsCorrect, + mockedReadableLogRecord, +} from '../logHelper'; import { PassThrough, Stream } from 'stream'; import { IExportLogsServiceRequest } from '@opentelemetry/otlp-transformer'; import { ExportResultCode } from '@opentelemetry/core'; @@ -115,7 +120,6 @@ describe('OTLPLogExporter', () => { }); it('should set custom headers', done => { - sinon.stub(http, 'request').callsFake((options: any, cb: any) => { assert.strictEqual(options.headers['foo'], 'bar'); @@ -130,7 +134,6 @@ describe('OTLPLogExporter', () => { }); it('should have keep alive and keepAliveMsecs option set', done => { - sinon.stub(http, 'request').callsFake((options: any, cb: any) => { assert.strictEqual(options.agent.keepAlive, true); assert.strictEqual(options.agent.options.keepAliveMsecs, 2000); From 6209f9a06879b12bc48fe7d0c4614ecff1dcdd09 Mon Sep 17 00:00:00 2001 From: Nico385412 Date: Wed, 21 Jun 2023 13:36:33 +0200 Subject: [PATCH 7/7] chore: code review --- .../exporter-logs-otlp-http/package.json | 1 + .../test/browser/OTLPLogExporter.test.ts | 185 ++++++------------ .../exporter-logs-otlp-http/test/logHelper.ts | 20 -- .../test/node/OTLPLogExporter.test.ts | 20 +- .../exporter-logs-otlp-http/tsconfig.json | 5 +- 5 files changed, 85 insertions(+), 146 deletions(-) diff --git a/experimental/packages/exporter-logs-otlp-http/package.json b/experimental/packages/exporter-logs-otlp-http/package.json index 19ec49940b..41b72085be 100644 --- a/experimental/packages/exporter-logs-otlp-http/package.json +++ b/experimental/packages/exporter-logs-otlp-http/package.json @@ -73,6 +73,7 @@ "devDependencies": { "@babel/core": "7.22.5", "@opentelemetry/api-logs": "0.40.0", + "@opentelemetry/resources": "1.14.0", "@types/mocha": "10.0.1", "@types/node": "18.6.5", "@types/sinon": "10.0.15", diff --git a/experimental/packages/exporter-logs-otlp-http/test/browser/OTLPLogExporter.test.ts b/experimental/packages/exporter-logs-otlp-http/test/browser/OTLPLogExporter.test.ts index c6c8b7d741..2443c97ef4 100644 --- a/experimental/packages/exporter-logs-otlp-http/test/browser/OTLPLogExporter.test.ts +++ b/experimental/packages/exporter-logs-otlp-http/test/browser/OTLPLogExporter.test.ts @@ -13,36 +13,22 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - -import { diag } from '@opentelemetry/api'; import * as assert from 'assert'; import * as sinon from 'sinon'; -import * as http from 'http'; 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'; -import { - MockedResponse, - ensureExportLogsServiceRequestIsSet, - ensureExportedLogRecordIsCorrect, - mockedReadableLogRecord, -} from '../logHelper'; -import { PassThrough, Stream } from 'stream'; -import { IExportLogsServiceRequest } from '@opentelemetry/otlp-transformer'; +import { mockedReadableLogRecord } from '../logHelper'; import { ExportResultCode } from '@opentelemetry/core'; -let fakeRequest: PassThrough; - describe('OTLPLogExporter', () => { let envSource: Record; let collectorExporter: OTLPLogExporter; let collectorExporterConfig: OTLPExporterConfigBase; - let logs: ReadableLogRecord[]; afterEach(() => { - fakeRequest = new Stream.PassThrough(); sinon.restore(); }); @@ -85,126 +71,77 @@ describe('OTLPLogExporter', () => { }); }); - describe('export', () => { + describe('export - common', () => { + let spySend: any; beforeEach(() => { - collectorExporterConfig = { - headers: { - foo: 'bar', - }, - hostname: 'foo', - url: 'http://foo.bar.com', - }; + spySend = sinon.stub(OTLPLogExporter.prototype, 'send'); collectorExporter = new OTLPLogExporter(collectorExporterConfig); - logs = []; - logs.push(Object.assign({}, mockedReadableLogRecord)); - }); - afterEach(() => { - sinon.restore(); - }); - - it('should open the connection', done => { - sinon.stub(http, 'request').callsFake((options: any, cb: any) => { - assert.strictEqual(options.hostname, 'foo.bar.com'); - assert.strictEqual(options.method, 'POST'); - assert.strictEqual(options.path, '/'); - - const mockRes = new MockedResponse(200); - cb(mockRes); - mockRes.send('success'); - done(); - return fakeRequest as any; - }); - collectorExporter.export(logs, () => {}); }); - it('should set custom headers', done => { - sinon.stub(http, 'request').callsFake((options: any, cb: any) => { - assert.strictEqual(options.headers['foo'], 'bar'); - - const mockRes = new MockedResponse(200); - cb(mockRes); - mockRes.send('success'); - done(); - return fakeRequest as any; - }); - - collectorExporter.export(logs, () => {}); - }); - - it('should have keep alive and keepAliveMsecs option set', done => { - sinon.stub(http, 'request').callsFake((options: any, cb: any) => { - assert.strictEqual(options.agent.keepAlive, true); - assert.strictEqual(options.agent.options.keepAliveMsecs, 2000); - - const mockRes = new MockedResponse(200); - cb(mockRes); - mockRes.send('success'); - done(); - return fakeRequest as any; - }); + it('should export spans as otlpTypes.Spans', done => { + const logs: ReadableLogRecord[] = []; + logs.push(Object.assign({}, mockedReadableLogRecord)); collectorExporter.export(logs, () => {}); - }); - - it('should successfully send the logs', done => { - const fakeRequest = new Stream.PassThrough(); - sinon.stub(http, 'request').returns(fakeRequest as any); - - let buff = Buffer.from(''); - fakeRequest.on('end', () => { - const responseBody = buff.toString(); - const json = JSON.parse(responseBody) as IExportLogsServiceRequest; - const log1 = json.resourceLogs?.[0].scopeLogs?.[0].logRecords?.[0]; - assert.ok(typeof log1 !== 'undefined', "log doesn't exist"); - ensureExportedLogRecordIsCorrect(log1); - - ensureExportLogsServiceRequestIsSet(json); - + setTimeout(() => { + const log = spySend.args[0][0][0] as ReadableLogRecord; + assert.deepStrictEqual(logs[0], log); done(); }); - - fakeRequest.on('data', chunk => { - buff = Buffer.concat([buff, chunk]); - }); - - const clock = sinon.useFakeTimers(); - collectorExporter.export(logs, () => {}); - clock.tick(200); - clock.restore(); + assert.strictEqual(spySend.callCount, 1); }); - it('should log the successful message', done => { - // Need to stub/spy on the underlying logger as the "diag" instance is global - const spyLoggerError = sinon.stub(diag, 'error'); - - sinon.stub(http, 'request').callsFake((options: any, cb: any) => { - const mockRes = new MockedResponse(200); - cb(mockRes); - mockRes.send('success'); - return fakeRequest as any; - }); - - collectorExporter.export(logs, result => { - assert.strictEqual(result.code, ExportResultCode.SUCCESS); - assert.strictEqual(spyLoggerError.args.length, 0); - done(); - }); + describe('when exporter is shutdown', () => { + it( + 'should not export anything but return callback with code' + + ' "FailedNotRetryable"', + async () => { + const spans: ReadableLogRecord[] = []; + spans.push(Object.assign({}, mockedReadableLogRecord)); + await collectorExporter.shutdown(); + spySend.resetHistory(); + + const callbackSpy = sinon.spy(); + collectorExporter.export(spans, callbackSpy); + const returnCode = callbackSpy.args[0][0]; + + assert.strictEqual( + returnCode.code, + ExportResultCode.FAILED, + 'return value is wrong' + ); + assert.strictEqual(spySend.callCount, 0, 'should not call send'); + } + ); }); - - 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('when an error occurs', () => { + it('should return failed export result', done => { + const spans: ReadableLogRecord[] = []; + spans.push(Object.assign({}, mockedReadableLogRecord)); + spySend.throws({ + code: 100, + details: 'Test error', + metadata: {}, + message: 'Non-retryable', + stack: 'Stack', + }); + const callbackSpy = sinon.spy(); + collectorExporter.export(spans, callbackSpy); + setTimeout(() => { + const returnCode = callbackSpy.args[0][0]; + assert.strictEqual( + returnCode.code, + ExportResultCode.FAILED, + 'return value is wrong' + ); + assert.strictEqual( + returnCode.error.message, + 'Non-retryable', + 'return error message is wrong' + ); + assert.strictEqual(spySend.callCount, 1, 'should call send'); + done(); + }); }); }); }); diff --git a/experimental/packages/exporter-logs-otlp-http/test/logHelper.ts b/experimental/packages/exporter-logs-otlp-http/test/logHelper.ts index 6ce45632df..1d9461f873 100644 --- a/experimental/packages/exporter-logs-otlp-http/test/logHelper.ts +++ b/experimental/packages/exporter-logs-otlp-http/test/logHelper.ts @@ -27,7 +27,6 @@ import { IResource, } from '@opentelemetry/otlp-transformer'; import { ReadableLogRecord } from '@opentelemetry/sdk-logs'; -import { Stream } from 'stream'; export const mockedReadableLogRecord: ReadableLogRecord = { resource: Resource.default().merge( @@ -165,22 +164,3 @@ export function ensureExportLogsServiceRequestIsSet( const logRecords = scopeLogs?.[0].logRecords; assert.strictEqual(logRecords?.length, 1, 'logs are missing'); } - -export class MockedResponse extends Stream { - constructor(private _code: number, private _msg?: string) { - super(); - } - - send(data: string) { - this.emit('data', data); - this.emit('end'); - } - - get statusCode() { - return this._code; - } - - get statusMessage() { - return this._msg; - } -} diff --git a/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts b/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts index 71d34d9160..2ae11142ad 100644 --- a/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts +++ b/experimental/packages/exporter-logs-otlp-http/test/node/OTLPLogExporter.test.ts @@ -24,7 +24,6 @@ import { OTLPLogExporter } from '../../src/platform/node'; import { OTLPExporterNodeConfigBase } from '@opentelemetry/otlp-exporter-base'; import { ReadableLogRecord } from '@opentelemetry/sdk-logs'; import { - MockedResponse, ensureExportLogsServiceRequestIsSet, ensureExportedLogRecordIsCorrect, mockedReadableLogRecord, @@ -35,6 +34,25 @@ import { ExportResultCode } from '@opentelemetry/core'; let fakeRequest: PassThrough; +class MockedResponse extends Stream { + constructor(private _code: number, private _msg?: string) { + super(); + } + + send(data: string) { + this.emit('data', data); + this.emit('end'); + } + + get statusCode() { + return this._code; + } + + get statusMessage() { + return this._msg; + } +} + describe('OTLPLogExporter', () => { let envSource: Record; let collectorExporter: OTLPLogExporter; diff --git a/experimental/packages/exporter-logs-otlp-http/tsconfig.json b/experimental/packages/exporter-logs-otlp-http/tsconfig.json index e42e043148..52330c182e 100644 --- a/experimental/packages/exporter-logs-otlp-http/tsconfig.json +++ b/experimental/packages/exporter-logs-otlp-http/tsconfig.json @@ -23,6 +23,9 @@ }, { "path": "../sdk-logs" - } + }, + { + "path": "../../../packages/opentelemetry-resources" + }, ] }