Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: change default propagator to match spec #1218

Merged
merged 5 commits into from
Jun 19, 2020
Merged

fix: change default propagator to match spec #1218

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4fe8ea8
fix: change default propagator to match spec
jonahrosenblum Jun 18, 2020
e2984f6
Merge branch 'master' into change_default_propagator
jonahrosenblum Jun 18, 2020
8afce62
fix: update documentation to identify correct default propagator
jonahrosenblum Jun 18, 2020
958fdbd
Merge branch 'master' into change_default_propagator
jonahrosenblum Jun 18, 2020
daf6845
Merge branch 'master' into change_default_propagator
dyladan Jun 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/opentelemetry-core/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ This package provides default implementations of the OpenTelemetry API for trace

OpenTelemetry provides a text-based approach to propagate context to remote services using the [W3C Trace Context](https://www.w3.org/TR/trace-context/) HTTP headers.

> This is used as a default Propagator

```js
const api = require("@opentelemetry/api");
const { HttpTraceContext } = require("@opentelemetry/core");
Expand All @@ -56,6 +54,8 @@ api.propagation.setGlobalPropagator(new B3Propagator());

Combines multiple propagators into a single propagator.

> This is used as a default Propagator

```js
const api = require("@opentelemetry/api");
const { CompositePropagator } = require("@opentelemetry/core");
Expand Down
6 changes: 3 additions & 3 deletions packages/opentelemetry-node/test/registration.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
} from '@opentelemetry/api';
import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks';
import { NoopContextManager } from '@opentelemetry/context-base';
import { HttpTraceContext } from '@opentelemetry/core';
import { CompositePropagator } from '@opentelemetry/core';
import * as assert from 'assert';
import { NodeTracerProvider } from '../src';

Expand All @@ -41,7 +41,7 @@ describe('API registration', () => {
context['_getContextManager']() instanceof AsyncHooksContextManager
);
assert.ok(
propagation['_getGlobalPropagator']() instanceof HttpTraceContext
propagation['_getGlobalPropagator']() instanceof CompositePropagator
);
assert.ok(trace.getTracerProvider() === tracerProvider);
});
Expand Down Expand Up @@ -72,7 +72,7 @@ describe('API registration', () => {
assert.ok(context['_getContextManager']() instanceof NoopContextManager);

assert.ok(
propagation['_getGlobalPropagator']() instanceof HttpTraceContext
propagation['_getGlobalPropagator']() instanceof CompositePropagator
);
assert.ok(trace.getTracerProvider() === tracerProvider);
});
Expand Down
11 changes: 9 additions & 2 deletions packages/opentelemetry-tracing/src/BasicTracerProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,12 @@
*/

import * as api from '@opentelemetry/api';
import { ConsoleLogger, HttpTraceContext } from '@opentelemetry/core';
import {
ConsoleLogger,
HttpTraceContext,
HttpCorrelationContext,
CompositePropagator,
} from '@opentelemetry/core';
import { SpanProcessor, Tracer } from '.';
import { DEFAULT_CONFIG } from './config';
import { MultiSpanProcessor } from './MultiSpanProcessor';
Expand Down Expand Up @@ -78,7 +83,9 @@ export class BasicTracerProvider implements api.TracerProvider {
register(config: SDKRegistrationConfig = {}) {
api.trace.setGlobalTracerProvider(this);
if (config.propagator === undefined) {
config.propagator = new HttpTraceContext();
config.propagator = new CompositePropagator({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, HttpTraceContext is highlighted as a default propagator in the documentation (https://github.com/open-telemetry/opentelemetry-js/tree/master/packages/opentelemetry-core#built-in-propagators). Could you please update the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I added a new commit to change the documentation. Thanks for pointing this out.

propagators: [new HttpCorrelationContext(), new HttpTraceContext()],
mayurkale22 marked this conversation as resolved.
Show resolved Hide resolved
});
}

if (config.contextManager) {
Expand Down
6 changes: 3 additions & 3 deletions packages/opentelemetry-web/test/registration.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
trace,
} from '@opentelemetry/api';
import { NoopContextManager } from '@opentelemetry/context-base';
import { HttpTraceContext } from '@opentelemetry/core';
import { CompositePropagator } from '@opentelemetry/core';
import * as assert from 'assert';
import { StackContextManager, WebTracerProvider } from '../src';

Expand All @@ -38,7 +38,7 @@ describe('API registration', () => {

assert.ok(context['_getContextManager']() instanceof StackContextManager);
assert.ok(
propagation['_getGlobalPropagator']() instanceof HttpTraceContext
propagation['_getGlobalPropagator']() instanceof CompositePropagator
);
assert.ok(trace.getTracerProvider() === tracerProvider);
});
Expand Down Expand Up @@ -69,7 +69,7 @@ describe('API registration', () => {
assert.ok(context['_getContextManager']() instanceof NoopContextManager);

assert.ok(
propagation['_getGlobalPropagator']() instanceof HttpTraceContext
propagation['_getGlobalPropagator']() instanceof CompositePropagator
);
assert.ok(trace.getTracerProvider() === tracerProvider);
});
Expand Down