From 7bdf6ea30a38887890bf673e76412b68cbedb4cc Mon Sep 17 00:00:00 2001 From: Nico385412 Date: Fri, 9 Jun 2023 16:47:23 +0200 Subject: [PATCH] chore: apply code review --- .../exporter-logs-otlp-http/test/logHelper.ts | 186 ++++++++++++++++++ .../test/node/OTLPLogExporter.test.ts | 156 +++++++++++++-- .../exporter-logs-otlp-http/tsconfig.json | 1 - 3 files changed, 329 insertions(+), 14 deletions(-) create mode 100644 experimental/packages/exporter-logs-otlp-http/test/logHelper.ts 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 00000000000..6ce45632df4 --- /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 9c187776e4c..60a7638041c 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 c4eda837a90..e42e043148c 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": "." },