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.

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

Separate context propagation (OTEP 66) #769

Merged
merged 31 commits into from Feb 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
90e23f0
feat: separate context propagation
dyladan Feb 6, 2020
5a03016
chore: fix api dependencies on builds
dyladan Feb 6, 2020
04fb334
chore: use bootstrap for doc deps
dyladan Feb 6, 2020
3391207
chore: allow builds as root in docs
dyladan Feb 6, 2020
8467706
chore: bootstrap docs after checking code
dyladan Feb 6, 2020
7076553
chore: allow unsafe builds in docs
dyladan Feb 6, 2020
1318000
chore: fix api test build
dyladan Feb 6, 2020
9415269
Merge branch 'master' into context
dyladan Feb 6, 2020
5533881
chore: lint
dyladan Feb 6, 2020
02605b2
chore: bust cache
dyladan Feb 6, 2020
7961494
chore: revert unnecessary changes from api package
dyladan Feb 6, 2020
02e07cb
chore: revert carrier so it can be a separate pr
dyladan Feb 6, 2020
fb0d170
chore: typo
dyladan Feb 6, 2020
d433528
chore: avoid extra allocation on context set value
dyladan Feb 6, 2020
38cda1d
chore: lint
dyladan Feb 6, 2020
d4caaba
chore: do not extend context
dyladan Feb 7, 2020
d89fcc6
chore: remove extra allocation
dyladan Feb 7, 2020
68234f7
chore: review comments
dyladan Feb 7, 2020
216cac3
Merge remote-tracking branch 'origin/master' into context
dyladan Feb 7, 2020
4d05383
Merge branch 'master' into context
dyladan Feb 11, 2020
2987a59
chore: use variables for context keys
dyladan Feb 12, 2020
5ad4383
chore: use unique symbols to identify context values
dyladan Feb 12, 2020
d5ef1d8
chore: use TODO context reference
dyladan Feb 12, 2020
e750d4b
chore: remove root context from dummy propagator
dyladan Feb 12, 2020
adff03c
chore: lint
dyladan Feb 12, 2020
d33f9c0
Merge remote-tracking branch 'origin/master' into context
dyladan Feb 12, 2020
8e433f1
chore: rename getKey to createKey
dyladan Feb 12, 2020
282a5e1
Merge branch 'master' into context
dyladan Feb 18, 2020
afaf91f
Merge branch 'master' into context
dyladan Feb 18, 2020
e90d731
Merge branch 'master' into context
dyladan Feb 18, 2020
4b356ba
Merge branch 'master' into context
dyladan Feb 18, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 11 additions & 6 deletions .circleci/config.yml
Expand Up @@ -40,7 +40,7 @@ mysql_service: &mysql_service
MYSQL_ROOT_PASSWORD: rootpw

cache_1: &cache_1
key: npm-cache-01-{{ .Environment.CIRCLE_JOB }}-{{ checksum "/tmp/checksums.txt" }}-FDF2088C
key: npm-cache-01-{{ .Environment.CIRCLE_JOB }}-{{ checksum "/tmp/checksums.txt" }}-F267A71D
paths:
- ./node_modules
- ./package-lock.json
Expand All @@ -60,7 +60,7 @@ cache_1: &cache_1
- packages/opentelemetry-plugin-dns/node_modules

cache_2: &cache_2
key: npm-cache-02-{{ .Environment.CIRCLE_JOB }}-{{ checksum "/tmp/checksums.txt" }}-FDF2088C
key: npm-cache-02-{{ .Environment.CIRCLE_JOB }}-{{ checksum "/tmp/checksums.txt" }}-F267A71D
paths:
- packages/opentelemetry-plugin-grpc/node_modules
- packages/opentelemetry-plugin-http/node_modules
Expand Down Expand Up @@ -95,10 +95,10 @@ node_unit_tests: &node_unit_tests
echo "CIRCLE_NODE_VERSION=${CIRCLE_NODE_VERSION}"
- restore_cache:
keys:
- npm-cache-01-{{ .Environment.CIRCLE_JOB }}-{{ checksum "/tmp/checksums.txt" }}-FDF2088C
- npm-cache-01-{{ .Environment.CIRCLE_JOB }}-{{ checksum "/tmp/checksums.txt" }}-F267A71D
- restore_cache:
keys:
- npm-cache-02-{{ .Environment.CIRCLE_JOB }}-{{ checksum "/tmp/checksums.txt" }}-FDF2088C
- npm-cache-02-{{ .Environment.CIRCLE_JOB }}-{{ checksum "/tmp/checksums.txt" }}-F267A71D
- run:
name: Install Root Dependencies
command: npm install --ignore-scripts
Expand Down Expand Up @@ -135,10 +135,10 @@ browsers_unit_tests: &browsers_unit_tests
echo "CIRCLE_NODE_VERSION=${CIRCLE_NODE_VERSION}"
- restore_cache:
keys:
- npm-cache-01-{{ .Environment.CIRCLE_JOB }}-{{ checksum "/tmp/checksums.txt" }}-FDF2088C
- npm-cache-01-{{ .Environment.CIRCLE_JOB }}-{{ checksum "/tmp/checksums.txt" }}-F267A71D
- restore_cache:
keys:
- npm-cache-02-{{ .Environment.CIRCLE_JOB }}-{{ checksum "/tmp/checksums.txt" }}-FDF2088C
- npm-cache-02-{{ .Environment.CIRCLE_JOB }}-{{ checksum "/tmp/checksums.txt" }}-F267A71D
- run:
name: Install Root Dependencies
command: npm install --ignore-scripts
Expand All @@ -160,6 +160,8 @@ jobs:
lint_&_docs:
docker:
- image: node:12
environment:
dyladan marked this conversation as resolved.
Show resolved Hide resolved
NPM_CONFIG_UNSAFE_PERM: true
steps:
- checkout
- run:
Expand All @@ -171,6 +173,9 @@ jobs:
- run:
name: Check code style and linting
command: npm run check
- run:
name: Install API dependencies
command: lerna bootstrap --scope @opentelemetry/api --include-filtered-dependencies
- run:
name: Docs tests
command: npm run docs-test
Expand Down
3 changes: 3 additions & 0 deletions packages/opentelemetry-api/package.json
Expand Up @@ -45,6 +45,9 @@
"publishConfig": {
"access": "public"
},
"dependencies": {
Copy link
Member Author

Choose a reason for hiding this comment

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

API now depends on scope base because that is where context is. Context is a separate mechanism outside of the API and the SDK, but which is depended on by both.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe Context and the dummy implementation should be moved like it was done with a other parts recently?

Copy link
Member Author

Choose a reason for hiding this comment

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

I would be willing to move Context into it's own package. @mayurkale22 wdyt?

Copy link
Member

Choose a reason for hiding this comment

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

Still thinking on this, I will also check how other libraries are doing it.

Copy link
Member

Choose a reason for hiding this comment

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

When we started it we decided to move it back so it could be used by other projects. However since it's started to get heavily linked to the OT specs (and not storing generic context) i would be in favor to move it

Copy link
Member Author

Choose a reason for hiding this comment

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

@vmarchaud I'm sorry I don't understand, you would be in favor of context having it's own package or you would be in favor of moving it into core/api/other?

"@opentelemetry/scope-base": "^0.4.0"
},
"devDependencies": {
"@types/mocha": "^5.2.7",
"@types/node": "^12.6.8",
Expand Down
Expand Up @@ -14,40 +14,35 @@
* limitations under the License.
*/

import { SpanContext } from '../../trace/span_context';
import { Context } from '@opentelemetry/scope-base';
import { Carrier } from './carrier';

/**
* Injects and extracts a value as text into carriers that travel in-band
* across process boundaries. Encoding is expected to conform to the HTTP
* Injects {@link Context} into and extracts it from carriers that travel
* in-band across process boundaries. Encoding is expected to conform to the HTTP
* Header Field semantics. Values are often encoded as RPC/HTTP request headers.
*
* The carrier of propagated data on both the client (injector) and server
* (extractor) side is usually an http request. Propagation is usually
* implemented via library- specific request interceptors, where the
* client-side injects values and the server-side extracts them.
* (extractor) side is usually an object such as http headers.
*/
export interface HttpTextFormat {
/**
* Injects the given {@link SpanContext} instance to transmit over the wire.
* Injects values from a given {@link Context} into a carrier.
*
* OpenTelemetry defines a common set of format values (BinaryFormat and
* HTTPTextFormat), and each has an expected `carrier` type.
*
* @param spanContext the SpanContext to transmit over the wire.
* @param format the format of the carrier.
* @param context the Context from which to extract values to transmit over the wire.
* @param carrier the carrier of propagation fields, such as http request headers.
*/
inject(spanContext: SpanContext, format: string, carrier: Carrier): void;
inject(context: Context, carrier: Carrier): void;

/**
* Returns a {@link SpanContext} instance extracted from `carrier` in the
* given format from upstream.
* Given a {@link Context} and a carrier, extract context values from a carrier and
* return a new context, created from the old context, with the extracted values.
*
* @param format the format of the carrier.
* @param context the Context from which to extract values to transmit over the wire.
* @param carrier the carrier of propagation fields, such as http request headers.
* @returns SpanContext The extracted SpanContext, or null if no such
* SpanContext could be found in carrier.
*/
extract(format: string, carrier: Carrier): SpanContext | null;
extract(context: Context, carrier: Carrier): Context;
}
Expand Up @@ -14,19 +14,19 @@
* limitations under the License.
*/

import { SpanContext } from '../../trace/span_context';
import { Context } from '@opentelemetry/scope-base';
import { Carrier } from './carrier';
import { HttpTextFormat } from './HttpTextFormat';

/**
* No-op implementations of {@link HttpTextFormat}.
*/
export class NoopHttpTextFormat implements HttpTextFormat {
// By default does nothing
inject(spanContext: SpanContext, format: string, carrier: Carrier): void {}
// By default does nothing
extract(format: string, carrier: Carrier): SpanContext | null {
return null;
/** Noop inject function does nothing */
inject(context: Context, carrier: Carrier): void {}
/** Noop extract function does nothing and returns the input context */
extract(context: Context, carrier: Carrier): Context {
return context;
}
}

Expand Down
2 changes: 2 additions & 0 deletions packages/opentelemetry-api/src/index.ts
Expand Up @@ -46,6 +46,8 @@ export * from './trace/NoopTracerProvider';
export * from './metrics/NoopMeterProvider';
export * from './metrics/NoopMeter';

export { Context } from '@opentelemetry/scope-base';
mayurkale22 marked this conversation as resolved.
Show resolved Hide resolved

import { TraceAPI } from './api/trace';
/** Entrypoint for trace API */
export const trace = TraceAPI.getInstance();
Expand Down
Expand Up @@ -16,6 +16,7 @@

import * as assert from 'assert';
import { NoopTracer, NOOP_SPAN, SpanKind } from '../../src';
import { Context } from '@opentelemetry/scope-base';

describe('NoopTracer', () => {
it('should not crash', () => {
Expand All @@ -38,8 +39,12 @@ describe('NoopTracer', () => {
assert.deepStrictEqual(tracer.getCurrentSpan(), NOOP_SPAN);
const httpTextFormat = tracer.getHttpTextFormat();
assert.ok(httpTextFormat);
httpTextFormat.inject(spanContext, 'HttpTextFormat', {});
assert.deepStrictEqual(httpTextFormat.extract('HttpTextFormat', {}), null);

httpTextFormat.inject(Context.ROOT_CONTEXT, {});
assert.deepStrictEqual(
httpTextFormat.extract(Context.ROOT_CONTEXT, {}),
Context.ROOT_CONTEXT
);

const binaryFormat = tracer.getBinaryFormat();
assert.ok(binaryFormat);
Expand Down
1 change: 1 addition & 0 deletions packages/opentelemetry-core/package.json
Expand Up @@ -79,6 +79,7 @@
},
"dependencies": {
"@opentelemetry/api": "^0.4.0",
"@opentelemetry/scope-base": "^0.4.0",
"semver": "^6.3.0"
}
}
83 changes: 83 additions & 0 deletions packages/opentelemetry-core/src/context/context.ts
@@ -0,0 +1,83 @@
/*!
* Copyright 2020, 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 { Span, SpanContext } from '@opentelemetry/api';
import { Context } from '@opentelemetry/scope-base';

const ACTIVE_SPAN_KEY = Context.createKey(
'OpenTelemetry Context Key ACTIVE_SPAN'
);
const EXTRACTED_SPAN_CONTEXT_KEY = Context.createKey(
'OpenTelemetry Context Key EXTRACTED_SPAN_CONTEXT'
);

/**
* Return the active span if one exists
*
* @param context context to get span from
*/
export function getActiveSpan(context: Context): Span | undefined {
return (context.getValue(ACTIVE_SPAN_KEY) as Span) || undefined;
}

/**
* Set the active span on a context
*
* @param context context to use as parent
* @param span span to set active
*/
export function setActiveSpan(context: Context, span: Span): Context {
return context.setValue(ACTIVE_SPAN_KEY, span);
}

/**
* Get the extracted span context from a context
*
* @param context context to get span context from
*/
export function getExtractedSpanContext(
context: Context
): SpanContext | undefined {
return (
(context.getValue(EXTRACTED_SPAN_CONTEXT_KEY) as SpanContext) || undefined
);
}

/**
* Set the extracted span context on a context
*
* @param context context to set span context on
* @param spanContext span context to set
*/
export function setExtractedSpanContext(
context: Context,
spanContext: SpanContext
): Context {
return context.setValue(EXTRACTED_SPAN_CONTEXT_KEY, spanContext);
}

/**
* Get the span context of the parent span if it exists,
* or the extracted span context if there is no active
* span.
*
* @param context context to get values from
*/
export function getParentSpanContext(
context: Context
): SpanContext | undefined {
return getActiveSpan(context)?.context() || getExtractedSpanContext(context);
}
18 changes: 11 additions & 7 deletions packages/opentelemetry-core/src/context/propagation/B3Format.ts
Expand Up @@ -16,10 +16,11 @@

import {
Carrier,
Context,
HttpTextFormat,
SpanContext,
TraceFlags,
} from '@opentelemetry/api';
import { getParentSpanContext, setExtractedSpanContext } from '../context';

export const X_B3_TRACE_ID = 'x-b3-traceid';
export const X_B3_SPAN_ID = 'x-b3-spanid';
Expand All @@ -41,7 +42,10 @@ function isValidSpanId(spanId: string): boolean {
* Based on: https://github.com/openzipkin/b3-propagation
*/
export class B3Format implements HttpTextFormat {
inject(spanContext: SpanContext, format: string, carrier: Carrier): void {
inject(context: Context, carrier: Carrier) {
const spanContext = getParentSpanContext(context);
if (!spanContext) return;

if (
isValidTraceId(spanContext.traceId) &&
isValidSpanId(spanContext.spanId)
Expand All @@ -57,11 +61,11 @@ export class B3Format implements HttpTextFormat {
}
}

extract(format: string, carrier: Carrier): SpanContext | null {
extract(context: Context, carrier: Carrier): Context {
const traceIdHeader = carrier[X_B3_TRACE_ID];
const spanIdHeader = carrier[X_B3_SPAN_ID];
const sampledHeader = carrier[X_B3_SAMPLED];
if (!traceIdHeader || !spanIdHeader) return null;
if (!traceIdHeader || !spanIdHeader) return context;
const traceId = Array.isArray(traceIdHeader)
? traceIdHeader[0]
: traceIdHeader;
Expand All @@ -71,15 +75,15 @@ export class B3Format implements HttpTextFormat {
: sampledHeader;

if (isValidTraceId(traceId) && isValidSpanId(spanId)) {
return {
return setExtractedSpanContext(context, {
traceId,
spanId,
isRemote: true,
traceFlags: isNaN(Number(options))
? TraceFlags.UNSAMPLED
: Number(options),
};
});
}
return null;
return context;
}
}
Expand Up @@ -16,11 +16,13 @@

import {
Carrier,
Context,
HttpTextFormat,
SpanContext,
TraceFlags,
} from '@opentelemetry/api';
import { TraceState } from '../../trace/TraceState';
import { getParentSpanContext, setExtractedSpanContext } from '../context';

export const TRACE_PARENT_HEADER = 'traceparent';
export const TRACE_STATE_HEADER = 'tracestate';
Expand Down Expand Up @@ -61,7 +63,10 @@ export function parseTraceParent(traceParent: string): SpanContext | null {
* https://www.w3.org/TR/trace-context/
*/
export class HttpTraceContext implements HttpTextFormat {
inject(spanContext: SpanContext, format: string, carrier: Carrier) {
inject(context: Context, carrier: Carrier) {
const spanContext = getParentSpanContext(context);
if (!spanContext) return;

const traceParent = `${VERSION}-${spanContext.traceId}-${
spanContext.spanId
}-0${Number(spanContext.traceFlags || TraceFlags.UNSAMPLED).toString(16)}`;
Expand All @@ -72,14 +77,14 @@ export class HttpTraceContext implements HttpTextFormat {
}
}

extract(format: string, carrier: Carrier): SpanContext | null {
extract(context: Context, carrier: Carrier): Context {
const traceParentHeader = carrier[TRACE_PARENT_HEADER];
if (!traceParentHeader) return null;
if (!traceParentHeader) return context;
const traceParent = Array.isArray(traceParentHeader)
? traceParentHeader[0]
: traceParentHeader;
const spanContext = parseTraceParent(traceParent);
if (!spanContext) return null;
if (!spanContext) return context;

spanContext.isRemote = true;

Expand All @@ -92,6 +97,6 @@ export class HttpTraceContext implements HttpTextFormat {
: traceStateHeader;
spanContext.traceState = new TraceState(state as string);
}
return spanContext;
return setExtractedSpanContext(context, spanContext);
}
}
1 change: 1 addition & 0 deletions packages/opentelemetry-core/src/index.ts
Expand Up @@ -19,6 +19,7 @@ export * from './common/NoopLogger';
export * from './common/time';
export * from './common/types';
export * from './version';
export * from './context/context';
export * from './context/propagation/B3Format';
export * from './context/propagation/BinaryTraceContext';
export * from './context/propagation/HttpTraceContext';
Expand Down