chore: apply code review

experimental/packages/exporter-logs-otlp-http/package.json

@@ -88,7 +88,6 @@
     "karma-spec-reporter": "0.0.36",
     "karma-webpack": "4.0.2",
     "mocha": "10.2.0",
-    "mock-require": "3.0.3",
     "nyc": "15.1.0",
     "sinon": "14.0.2",
     "ts-loader": "8.4.0",

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;
+  }
+}

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<string, any>;
+  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<string, any>;
@@ -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();
+      });
     });
   });
 });

experimental/packages/exporter-logs-otlp-http/tsconfig.json

@@ -1,7 +1,6 @@
 {
   "extends": "../../../tsconfig.base.json",
   "compilerOptions": {
-    "esModuleInterop": true,
     "outDir": "build",
     "rootDir": "."
   },