Skip to content

Commit

Permalink
Merge branch 'main' into logs-exporter-otlp-http
Browse files Browse the repository at this point in the history
  • Loading branch information
pichlermarc authored May 25, 2023
2 parents 3db21ea + c6635fa commit 24d2f52
Show file tree
Hide file tree
Showing 11 changed files with 49 additions and 48 deletions.
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -188,13 +188,16 @@ We have a weekly SIG meeting! See the [community page](https://github.com/open-t

- [Gerhard Stöbich](https://github.com/Flarna), Dynatrace
- [Haddas Bronfman](https://github.com/haddasbronfman), Cisco
- [Hector Hernandez](https://github.com/hectorhdzg), Microsoft
- [Jamie Danielson](https://github.com/JamieDanielson), Honeycomb
- [John Bley](https://github.com/johnbley), Splunk
- [Mark Wolff](https://github.com/markwolff), Microsoft
- [Martin Kuba](https://github.com/martinkuba), Lightstep
- [Matthew Wear](https://github.com/mwear), LightStep
- [Naseem K. Ullah](https://github.com/naseemkullah), Transit
- [Neville Wylie](https://github.com/MSNev), Microsoft
- [Olivier Albertini](https://github.com/OlivierAlbertini), Ville de Montréal
- [Purvi Kanal](https://github.com/pkanal), Honeycomb
- [Svetlana Brennan](https://github.com/svetlanabrennan), New Relic

*Find more about the approver role in [community repository](https://github.com/open-telemetry/community/blob/main/community-membership.md#approver).*
Expand Down
3 changes: 3 additions & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ All notable changes to experimental packages in this project will be documented

### :boom: Breaking Change

* fix(exporter-logs-otlp-grpc): change OTLPLogsExporter to OTLPLogExporter [#3819](https://github.com/open-telemetry/opentelemetry-js/pull/3819) @fuaiyi

### :rocket: (Enhancement)

* feat(instrumentation): add ESM support for instrumentation. [#3698](https://github.com/open-telemetry/opentelemetry-js/pull/3698) @JamieDanielson, @pkanal, @vmarchaud, @lizthegrey, @bengl
Expand All @@ -16,6 +18,7 @@ All notable changes to experimental packages in this project will be documented
### :bug: (Bug Fix)

* fix(sdk-node): use resource interface instead of concrete class [#3803](https://github.com/open-telemetry/opentelemetry-js/pull/3803) @blumamir
* fix(sdk-logs): remove includeTraceContext configuration and use LogRecord context when available [#3817](https://github.com/open-telemetry/opentelemetry-js/pull/3817) @hectorhdzg

### :books: (Refine Doc)

Expand Down
6 changes: 3 additions & 3 deletions experimental/packages/exporter-logs-otlp-grpc/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,22 +22,22 @@ To see documentation and sample code for the metric exporter, see the [exporter-

## Logs in Node - GRPC

The OTLPLogsExporter in Node expects the URL to only be the hostname. It will not work with `/v1/logs`. All
The OTLPLogExporter in Node expects the URL to only be the hostname. It will not work with `/v1/logs`. All
options that work with trace also work with logs.

```js
import {
LoggerProvider,
BatchLogRecordProcessor,
} from '@opentelemetry/sdk-logs';
import { OTLPLogsExporter } from '@opentelemetry/exporter-logs-otlp-grpc';
import { OTLPLogExporter } from '@opentelemetry/exporter-logs-otlp-grpc';

const collectorOptions = {
// url is optional and can be omitted - default is http://localhost:4317
url: 'http://<collector-hostname>:<port>',
};

const loggerExporter = new OTLPLogsExporter(collectorOptions);
const loggerExporter = new OTLPLogExporter(collectorOptions);
const loggerProvider = new LoggerProvider();

loggerProvider.addLogRecordProcessor(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import {
/**
* OTLP Logs Exporter for Node
*/
export class OTLPLogsExporter
export class OTLPLogExporter
extends OTLPGRPCExporterNodeBase<ReadableLogRecord, IExportLogsServiceRequest>
implements LogRecordExporter
{
Expand Down
2 changes: 1 addition & 1 deletion experimental/packages/exporter-logs-otlp-grpc/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,4 @@
* limitations under the License.
*/

export * from './OTLPLogsExporter';
export * from './OTLPLogExporter';
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import * as fs from 'fs';
import * as grpc from '@grpc/grpc-js';
import * as path from 'path';
import * as sinon from 'sinon';
import { OTLPLogsExporter } from '../src';
import { OTLPLogExporter } from '../src';

import {
ensureExportedLogRecordIsCorrect,
Expand Down Expand Up @@ -55,10 +55,10 @@ const metadata = new grpc.Metadata();
metadata.set('k', 'v');

const testCollectorExporter = (params: TestParams) =>
describe(`OTLPLogsExporter - node ${
params.useTLS ? 'with' : 'without'
} TLS, ${params.metadata ? 'with' : 'without'} metadata`, () => {
let collectorExporter: OTLPLogsExporter;
describe(`OTLPLogExporter - node ${params.useTLS ? 'with' : 'without'} TLS, ${
params.metadata ? 'with' : 'without'
} metadata`, () => {
let collectorExporter: OTLPLogExporter;
let server: grpc.Server;
let exportedData: IResourceLogs | undefined;
let reqMetadata: grpc.Metadata | undefined;
Expand Down Expand Up @@ -122,7 +122,7 @@ const testCollectorExporter = (params: TestParams) =>
fs.readFileSync('./test/certs/client.crt')
)
: grpc.credentials.createInsecure();
collectorExporter = new OTLPLogsExporter({
collectorExporter = new OTLPLogExporter({
url: 'https://' + address,
credentials,
metadata: params.metadata,
Expand All @@ -140,7 +140,7 @@ const testCollectorExporter = (params: TestParams) =>
it('should warn about headers when using grpc', () => {
// Need to stub/spy on the underlying logger as the 'diag' instance is global
const spyLoggerWarn = sinon.stub(diag, 'warn');
collectorExporter = new OTLPLogsExporter({
collectorExporter = new OTLPLogExporter({
url: `http://${address}`,
headers: {
foo: 'bar',
Expand All @@ -151,7 +151,7 @@ const testCollectorExporter = (params: TestParams) =>
});
it('should warn about path in url', () => {
const spyLoggerWarn = sinon.stub(diag, 'warn');
collectorExporter = new OTLPLogsExporter({
collectorExporter = new OTLPLogExporter({
url: `http://${address}/v1/logs`,
});
const args = spyLoggerWarn.args[0];
Expand Down Expand Up @@ -198,7 +198,7 @@ const testCollectorExporter = (params: TestParams) =>
)
: grpc.credentials.createInsecure();

const collectorExporterWithTimeout = new OTLPLogsExporter({
const collectorExporterWithTimeout = new OTLPLogExporter({
url: 'grpcs://' + address,
credentials,
metadata: params.metadata,
Expand Down Expand Up @@ -229,7 +229,7 @@ const testCollectorExporter = (params: TestParams) =>
fs.readFileSync('./test/certs/client.crt')
)
: grpc.credentials.createInsecure();
collectorExporter = new OTLPLogsExporter({
collectorExporter = new OTLPLogExporter({
url: 'https://' + address,
credentials,
metadata: params.metadata,
Expand Down Expand Up @@ -272,7 +272,7 @@ const testCollectorExporter = (params: TestParams) =>
: grpc.credentials.createInsecure();

envSource.OTEL_EXPORTER_OTLP_COMPRESSION = 'gzip';
collectorExporter = new OTLPLogsExporter({
collectorExporter = new OTLPLogExporter({
url: 'https://' + address,
credentials,
metadata: params.metadata,
Expand All @@ -286,17 +286,17 @@ const testCollectorExporter = (params: TestParams) =>
});
});

describe('OTLPLogsExporter - node (getDefaultUrl)', () => {
describe('OTLPLogExporter - node (getDefaultUrl)', () => {
it('should default to localhost', done => {
const collectorExporter = new OTLPLogsExporter({});
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 OTLPLogsExporter({ url });
const collectorExporter = new OTLPLogExporter({ url });
setTimeout(() => {
assert.strictEqual(collectorExporter['url'], 'foo.bar.com');
done();
Expand All @@ -308,21 +308,21 @@ describe('when configuring via environment', () => {
const envSource = process.env;
it('should use url defined in env', () => {
envSource.OTEL_EXPORTER_OTLP_ENDPOINT = 'http://foo.bar';
const collectorExporter = new OTLPLogsExporter();
const collectorExporter = new OTLPLogExporter();
assert.strictEqual(collectorExporter.url, 'foo.bar');
envSource.OTEL_EXPORTER_OTLP_ENDPOINT = '';
});
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 OTLPLogsExporter();
const collectorExporter = new OTLPLogExporter();
assert.strictEqual(collectorExporter.url, 'foo.logs');
envSource.OTEL_EXPORTER_OTLP_ENDPOINT = '';
envSource.OTEL_EXPORTER_OTLP_LOGS_ENDPOINT = '';
});
it('should use headers defined via env', () => {
envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=bar';
const collectorExporter = new OTLPLogsExporter();
const collectorExporter = new OTLPLogExporter();
assert.deepStrictEqual(collectorExporter.metadata?.get('foo'), ['bar']);
envSource.OTEL_EXPORTER_OTLP_HEADERS = '';
});
Expand All @@ -332,7 +332,7 @@ describe('when configuring via environment', () => {
metadata.set('goo', 'lol');
envSource.OTEL_EXPORTER_OTLP_HEADERS = 'foo=jar,bar=foo';
envSource.OTEL_EXPORTER_OTLP_LOGS_HEADERS = 'foo=boo';
const collectorExporter = new OTLPLogsExporter({ metadata });
const collectorExporter = new OTLPLogExporter({ metadata });
assert.deepStrictEqual(collectorExporter.metadata?.get('foo'), ['boo']);
assert.deepStrictEqual(collectorExporter.metadata?.get('bar'), ['foo']);
assert.deepStrictEqual(collectorExporter.metadata?.get('goo'), ['lol']);
Expand Down
4 changes: 1 addition & 3 deletions experimental/packages/sdk-logs/src/Logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,7 @@ export class Logger implements logsAPI.Logger {
}

public emit(logRecord: logsAPI.LogRecord): void {
const currentContext = this._loggerConfig.includeTraceContext
? context.active()
: undefined;
const currentContext = logRecord.context || context.active();
/**
* If a Logger was obtained with include_trace_context=true,
* the LogRecords it emits MUST automatically include the Trace Context from the active Context,
Expand Down
1 change: 0 additions & 1 deletion experimental/packages/sdk-logs/src/LoggerProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ export class LoggerProvider implements logsAPI.LoggerProvider {
{ name: loggerName, version, schemaUrl: options?.schemaUrl },
{
logRecordLimits: this._config.logRecordLimits,
includeTraceContext: options?.includeTraceContext,
},
this
)
Expand Down
1 change: 1 addition & 0 deletions experimental/packages/sdk-logs/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

export {
LoggerConfig,
LoggerProviderConfig,
LogRecordLimits,
BufferConfig,
BatchLogRecordProcessorBrowserConfig,
Expand Down
3 changes: 0 additions & 3 deletions experimental/packages/sdk-logs/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,6 @@ export interface LoggerProviderConfig {
export interface LoggerConfig {
/** Log Record Limits*/
logRecordLimits?: LogRecordLimits;

/** include Trace Context */
includeTraceContext?: boolean;
}

export interface LogRecordLimits {
Expand Down
36 changes: 18 additions & 18 deletions experimental/packages/sdk-logs/test/common/Logger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ import * as assert from 'assert';
import * as sinon from 'sinon';

import { LogRecord, Logger, LoggerConfig, LoggerProvider } from '../../src';
import { loadDefaultConfig } from '../../src/config';
import { context } from '@opentelemetry/api';
import { ROOT_CONTEXT, TraceFlags, context, trace } from '@opentelemetry/api';
import { LogRecord as ApiLogRecord } from '@opentelemetry/api-logs';

const setup = (loggerConfig: LoggerConfig = {}) => {
const logger = new Logger(
Expand All @@ -40,14 +40,6 @@ describe('Logger', () => {
const { logger } = setup();
assert.ok(logger instanceof Logger);
});

it('should a default value with config.includeTraceContext', () => {
const { logger } = setup();
assert.ok(
logger['_loggerConfig'].includeTraceContext ===
loadDefaultConfig().includeTraceContext
);
});
});

describe('emit', () => {
Expand All @@ -69,22 +61,30 @@ describe('Logger', () => {
assert.ok(makeOnlySpy.called);
});

it('should emit with current Context when includeTraceContext is true', () => {
const { logger } = setup({ includeTraceContext: true });
it('should emit with current Context', () => {
const { logger } = setup({});
const callSpy = sinon.spy(logger.getActiveLogRecordProcessor(), 'onEmit');
logger.emit({
body: 'test log body',
});
assert.ok(callSpy.calledWith(sinon.match.any, context.active()));
});

it('should emit with empty Context when includeTraceContext is false', () => {
const { logger } = setup({ includeTraceContext: false });
it('should emit with Context specified in LogRecord', () => {
const { logger } = setup({});
const spanContext = {
traceId: 'd4cda95b652f4a1592b449d5929fda1b',
spanId: '6e0c63257de34c92',
traceFlags: TraceFlags.SAMPLED,
};
const activeContext = trace.setSpanContext(ROOT_CONTEXT, spanContext);
const logRecordData: ApiLogRecord = {
context: activeContext,
};

const callSpy = sinon.spy(logger.getActiveLogRecordProcessor(), 'onEmit');
logger.emit({
body: 'test log body',
});
assert.ok(callSpy.calledWith(sinon.match.any, undefined));
logger.emit(logRecordData);
assert.ok(callSpy.calledWith(sinon.match.any, activeContext));
});
});
});

0 comments on commit 24d2f52

Please sign in to comment.