Skip to content

Commit

Permalink
Merge branch 'main' into host-id-resource-attribute
Browse files Browse the repository at this point in the history
  • Loading branch information
pichlermarc authored Mar 1, 2023
2 parents 3b0e865 + 79b06a4 commit 38bd2e6
Show file tree
Hide file tree
Showing 30 changed files with 687 additions and 172 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,12 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/
* feat(sdk-metrics): apply binary search in histogram recording [#3539](https:/open-telemetry/opentelemetry-js/pull/3539) @legendecas
* perf(propagator-jaeger): improve deserializeSpanContext performance [#3541](https:/open-telemetry/opentelemetry-js/pull/3541) @doochik
* feat: support TraceState in SamplingResult [#3530](https:/open-telemetry/opentelemetry-js/pull/3530) @raphael-theriault-swi
* feat(sdk-trace-base): add diagnostic logging when spans are dropped [#3610](https:/open-telemetry/opentelemetry-js/pull/3610) @neoeinstein

### :bug: (Bug Fix)

* fix(core): added falsy check to make otel core work with browser where webpack config had process as false or null [#3613](https:/open-telemetry/opentelemetry-js/issues/3613) @ravindra-dyte

### :books: (Refine Doc)

### :house: (Internal)
Expand Down
2 changes: 1 addition & 1 deletion api/src/internal/global-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export function registerGlobal<Type extends keyof OTelGlobalAPI>(
if (api.version !== VERSION) {
// All registered APIs must be of the same version exactly
const err = new Error(
'@opentelemetry/api: All API registration versions must match'
`@opentelemetry/api: Registration of version v${api.version} for ${type} does not match previously registered API v${VERSION}`
);
diag.error(err.stack || err.message);
return false;
Expand Down
23 changes: 11 additions & 12 deletions api/test/common/internal/global.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,23 +84,22 @@ describe('Global Utils', () => {
assert.notStrictEqual(manager, api1.context['_getContextManager']());
});

it('should return the module NoOp implementation if the version is a mismatch', () => {
const newContextManager = new NoopContextManager();
api1.context.setGlobalContextManager(newContextManager);

// ensure new context manager is returned
assert.strictEqual(api1.context['_getContextManager'](), newContextManager);
it('should not register if the version is a mismatch', () => {
const logger1 = getMockLogger();
const logger2 = getMockLogger();

const globalInstance = getGlobal('context');
api1.diag.setLogger(logger1);
const globalInstance = getGlobal('diag');
assert.ok(globalInstance);
// @ts-expect-error we are modifying internals for testing purposes here
_globalThis[Symbol.for(GLOBAL_API_SYMBOL_KEY)].version = '0.0.1';

// ensure new context manager is not returned because version above is incompatible
assert.notStrictEqual(
api1.context['_getContextManager'](),
newContextManager
);
assert.equal(false, api1.diag.setLogger(logger2)); // won't happen

api1.diag.info('message');
sinon.assert.notCalled(logger2.error);
sinon.assert.notCalled(logger2.info);
sinon.assert.notCalled(logger2.warn);
});

it('should debug log registrations', () => {
Expand Down
5 changes: 5 additions & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,13 @@ All notable changes to experimental packages in this project will be documented

### :boom: Breaking Change

* feat: remove HTTP/HTTPS prefix from span name [#3603](https:/open-telemetry/opentelemetry-js/pull/3603) @Flarna

### :rocket: (Enhancement)

* feat: use HTTP_ROUTE in span name [#3603](https:/open-telemetry/opentelemetry-js/pull/3603) @Flarna
* feat: add HTTP_ROUTE attribute to http incoming metrics if present [#3581](https:/open-telemetry/opentelemetry-js/pull/3581) @hermogenes
* feat(sdk-node): install diag logger with OTEL_LOG_LEVEL [#3627](https:/open-telemetry/opentelemetry-js/pull/3627) @legendecas

### :bug: (Bug Fix)

Expand Down Expand Up @@ -71,6 +75,7 @@ All notable changes to experimental packages in this project will be documented
* deps: remove unused proto-loader dependencies and update grpc-js and proto-loader versions [#3337](https:/open-telemetry/opentelemetry-js/pull/3337) @seemk
* feat(metrics-exporters): configure temporality via environment variable [#3305](https:/open-telemetry/opentelemetry-js/pull/3305) @pichlermarc
* feat(console-metric-exporter): add temporality configuration [#3387](https:/open-telemetry/opentelemetry-js/pull/3387) @pichlermarc
* feat(otlp-exporter-base): add retries [#3207](https:/open-telemetry/opentelemetry-js/pull/3207) @svetlanabrennan

### :bug: (Bug Fix)

Expand Down
15 changes: 15 additions & 0 deletions experimental/packages/exporter-trace-otlp-http/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,21 @@ To override the default timeout duration, use the following options:

> Providing `timeoutMillis` with `collectorOptions` takes precedence and overrides timeout set with environment variables.

## OTLP Exporter Retry

OTLP requires that transient errors be handled with a [retry strategy](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#retry).

This retry policy has the following configuration, which there is currently no way to customize.

+ `DEFAULT_EXPORT_MAX_ATTEMPTS`: The maximum number of attempts, including the original request. Defaults to 5.
+ `DEFAULT_EXPORT_INITIAL_BACKOFF`: The initial backoff duration. Defaults to 1 second.
+ `DEFAULT_EXPORT_MAX_BACKOFF`: The maximum backoff duration. Defaults to 5 seconds.
+ `DEFAULT_EXPORT_BACKOFF_MULTIPLIER`: The backoff multiplier. Defaults to 1.5.

This retry policy first checks if the response has a `'Retry-After'` header. If there is a `'Retry-After'` header, the exporter will wait the amount specified in the `'Retry-After'` header before retrying. If there is no `'Retry-After'` header, the exporter will use an exponential backoff with jitter retry strategy.

> The exporter will retry exporting within the [exporter timeout configuration](#Exporter-Timeout-Configuration) time.

## Running opentelemetry-collector locally to see the traces

1. Go to `examples/otlp-exporter-node`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -582,3 +582,123 @@ describe('when configuring via environment', () => {
envSource.OTEL_EXPORTER_OTLP_HEADERS = '';
});
});

describe('export with retry - real http request destroyed', () => {
let server: any;
let collectorTraceExporter: OTLPTraceExporter;
let collectorExporterConfig: OTLPExporterConfigBase;
let spans: ReadableSpan[];

beforeEach(() => {
server = sinon.fakeServer.create({
autoRespond: true,
});
collectorExporterConfig = {
timeoutMillis: 1500,
};
});

afterEach(() => {
server.restore();
});

describe('when "sendBeacon" is NOT available', () => {
beforeEach(() => {
(window.navigator as any).sendBeacon = false;
collectorTraceExporter = new OTLPTraceExporter(collectorExporterConfig);
});
it('should log the timeout request error message when retrying with exponential backoff with jitter', done => {
spans = [];
spans.push(Object.assign({}, mockedReadableSpan));

let retry = 0;
server.respondWith(
'http://localhost:4318/v1/traces',
function (xhr: any) {
retry++;
xhr.respond(503);
}
);

collectorTraceExporter.export(spans, result => {
assert.strictEqual(result.code, core.ExportResultCode.FAILED);
const error = result.error as OTLPExporterError;
assert.ok(error !== undefined);
assert.strictEqual(error.message, 'Request Timeout');
assert.strictEqual(retry, 1);
done();
});
}).timeout(3000);

it('should log the timeout request error message when retry-after header is set to 3 seconds', done => {
spans = [];
spans.push(Object.assign({}, mockedReadableSpan));

let retry = 0;
server.respondWith(
'http://localhost:4318/v1/traces',
function (xhr: any) {
retry++;
xhr.respond(503, { 'Retry-After': 3 });
}
);

collectorTraceExporter.export(spans, result => {
assert.strictEqual(result.code, core.ExportResultCode.FAILED);
const error = result.error as OTLPExporterError;
assert.ok(error !== undefined);
assert.strictEqual(error.message, 'Request Timeout');
assert.strictEqual(retry, 1);
done();
});
}).timeout(3000);
it('should log the timeout request error message when retry-after header is a date', done => {
spans = [];
spans.push(Object.assign({}, mockedReadableSpan));

let retry = 0;
server.respondWith(
'http://localhost:4318/v1/traces',
function (xhr: any) {
retry++;
const d = new Date();
d.setSeconds(d.getSeconds() + 1);
xhr.respond(503, { 'Retry-After': d });
}
);

collectorTraceExporter.export(spans, result => {
assert.strictEqual(result.code, core.ExportResultCode.FAILED);
const error = result.error as OTLPExporterError;
assert.ok(error !== undefined);
assert.strictEqual(error.message, 'Request Timeout');
assert.strictEqual(retry, 2);
done();
});
}).timeout(3000);
it('should log the timeout request error message when retry-after header is a date with long delay', done => {
spans = [];
spans.push(Object.assign({}, mockedReadableSpan));

let retry = 0;
server.respondWith(
'http://localhost:4318/v1/traces',
function (xhr: any) {
retry++;
const d = new Date();
d.setSeconds(d.getSeconds() + 120);
xhr.respond(503, { 'Retry-After': d });
}
);

collectorTraceExporter.export(spans, result => {
assert.strictEqual(result.code, core.ExportResultCode.FAILED);
const error = result.error as OTLPExporterError;
assert.ok(error !== undefined);
assert.strictEqual(error.message, 'Request Timeout');
assert.strictEqual(retry, 1);
done();
});
}).timeout(3000);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -551,38 +551,3 @@ describe('export - real http request destroyed before response received', () =>
}, 0);
});
});

describe('export - real http request destroyed after response received', () => {
let collectorExporter: OTLPTraceExporter;
let collectorExporterConfig: OTLPExporterNodeConfigBase;
let spans: ReadableSpan[];

const server = http.createServer((_, res) => {
res.write('writing something');
});
before(done => {
server.listen(8081, done);
});
after(done => {
server.close(done);
});
it('should log the timeout request error message', done => {
collectorExporterConfig = {
url: 'http://localhost:8081',
timeoutMillis: 300,
};
collectorExporter = new OTLPTraceExporter(collectorExporterConfig);
spans = [];
spans.push(Object.assign({}, mockedReadableSpan));

setTimeout(() => {
collectorExporter.export(spans, result => {
assert.strictEqual(result.code, core.ExportResultCode.FAILED);
const error = result.error as OTLPExporterError;
assert.ok(error !== undefined);
assert.strictEqual(error.message, 'Request Timeout');
done();
});
}, 0);
});
});
15 changes: 15 additions & 0 deletions experimental/packages/exporter-trace-otlp-proto/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,21 @@ To override the default timeout duration, use the following options:

> Providing `timeoutMillis` with `collectorOptions` takes precedence and overrides timeout set with environment variables.

## OTLP Exporter Retry

OTLP requires that transient errors be handled with a [retry strategy](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#retry).

This retry policy has the following configuration, which there is currently no way to customize.

+ `DEFAULT_EXPORT_MAX_ATTEMPTS`: The maximum number of attempts, including the original request. Defaults to 5.
+ `DEFAULT_EXPORT_INITIAL_BACKOFF`: The initial backoff duration. Defaults to 1 second.
+ `DEFAULT_EXPORT_MAX_BACKOFF`: The maximum backoff duration. Defaults to 5 seconds.
+ `DEFAULT_EXPORT_BACKOFF_MULTIPLIER`: The backoff multiplier. Defaults to 1.5.

This retry policy first checks if the response has a `'Retry-After'` header. If there is a `'Retry-After'` header, the exporter will wait the amount specified in the `'Retry-After'` header before retrying. If there is no `'Retry-After'` header, the exporter will use an exponential backoff with jitter retry strategy.

> The exporter will retry exporting within the [exporter timeout configuration](#Exporter-Timeout-Configuration) time.

## Running opentelemetry-collector locally to see the traces

1. Go to examples/otlp-exporter-node
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ import {
} from '@opentelemetry/instrumentation';
import { RPCMetadata, RPCType, setRPCMetadata } from '@opentelemetry/core';
import { errorMonitor } from 'events';
import { SemanticAttributes } from '@opentelemetry/semantic-conventions';

/**
* Http instrumentation instrumentation for Opentelemetry
Expand Down Expand Up @@ -300,14 +301,12 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
* Attach event listeners to a client request to end span and add span attributes.
*
* @param request The original request object.
* @param options The arguments to the original function.
* @param span representing the current operation
* @param startTime representing the start time of the request to calculate duration in Metric
* @param metricAttributes metric attributes
*/
private _traceClientRequest(
request: http.ClientRequest,
hostname: string,
span: Span,
startTime: HrTime,
metricAttributes: MetricAttributes
Expand Down Expand Up @@ -491,11 +490,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
utils.getIncomingRequestMetricAttributes(spanAttributes);

const ctx = propagation.extract(ROOT_CONTEXT, headers);
const span = instrumentation._startHttpSpan(
`${component.toLocaleUpperCase()} ${method}`,
spanOptions,
ctx
);
const span = instrumentation._startHttpSpan(method, spanOptions, ctx);
const rpcMetadata: RPCMetadata = {
type: RPCType.HTTP,
span,
Expand Down Expand Up @@ -624,7 +619,6 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
return original.apply(this, [optionsParsed, ...args]);
}

const operationName = `${component.toUpperCase()} ${method}`;
const { hostname, port } = utils.extractHostnameAndPort(optionsParsed);

const attributes = utils.getOutgoingRequestAttributes(optionsParsed, {
Expand All @@ -645,7 +639,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
kind: SpanKind.CLIENT,
attributes,
};
const span = instrumentation._startHttpSpan(operationName, spanOptions);
const span = instrumentation._startHttpSpan(method, spanOptions);

const parentContext = context.active();
const requestContext = trace.setSpan(parentContext, span);
Expand Down Expand Up @@ -687,7 +681,6 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
context.bind(parentContext, request);
return instrumentation._traceClientRequest(
request,
hostname,
span,
startTime,
metricAttributes
Expand Down Expand Up @@ -720,6 +713,11 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
code: utils.parseResponseStatus(SpanKind.SERVER, response.statusCode),
});

const route = attributes[SemanticAttributes.HTTP_ROUTE];
if (route) {
span.updateName(`${request.method || 'GET'} ${route}`);
}

if (this._getConfig().applyCustomAttributesOnSpan) {
safeExecuteInTheMiddle(
() =>
Expand Down
Loading

0 comments on commit 38bd2e6

Please sign in to comment.