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

Move SpanContext isValid to the API #1447

Merged
merged 24 commits into from
Aug 31, 2020
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
5a7640d
refactor: make spanContext into class with isValid
srjames90 Aug 18, 2020
ee305c0
refactor: use class instantiation for spanContext
srjames90 Aug 18, 2020
9abb326
refactor: moves test and fix tests
srjames90 Aug 18, 2020
9c34b01
fix: add check for isValid
srjames90 Aug 19, 2020
3773f2a
revert: the changes to spanContext
srjames90 Aug 24, 2020
81b5506
refactor: move spancontext-utils to api package
srjames90 Aug 24, 2020
e83ae92
fix: lint and invalid psan id and invalid trace id
srjames90 Aug 24, 2020
2221074
fix: make isValid more robust
srjames90 Aug 24, 2020
c02e362
Merge branch 'master' into span-context-is-valid
srjames90 Aug 24, 2020
895db7a
refactor: rename isValid to isSpanContextValid
srjames90 Aug 25, 2020
cf20897
Merge branch 'master' into span-context-is-valid
srjames90 Aug 25, 2020
42db3c3
refactor: update regex and checks
srjames90 Aug 25, 2020
8641192
fix: export isSpanContextValid from api.trace
srjames90 Aug 26, 2020
6f695be
fix: run npm run lint:fix
srjames90 Aug 26, 2020
d555ab0
fix: run lint fix in api
srjames90 Aug 26, 2020
d537d85
Merge branch 'master' of https://github.com/open-telemetry/openteleme…
srjames90 Aug 26, 2020
f163513
refactor: prevent function overhead
srjames90 Aug 27, 2020
6e7f94f
fix: lint
srjames90 Aug 27, 2020
f38d97c
fix: remove unused import
srjames90 Aug 27, 2020
f8b7a6e
Merge branch 'master' into span-context-is-valid
srjames90 Aug 27, 2020
efe94a8
Merge branch 'master' into span-context-is-valid
srjames90 Aug 27, 2020
b72e068
Merge branch 'master' into span-context-is-valid
srjames90 Aug 31, 2020
73ee51d
Merge branch 'master' into span-context-is-valid
srjames90 Aug 31, 2020
9f8f599
Merge branch 'master' into span-context-is-valid
dyladan Aug 31, 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
1 change: 1 addition & 0 deletions packages/opentelemetry-api/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export * from './trace/NoopTracer';
export * from './trace/NoopTracerProvider';
export * from './trace/Sampler';
export * from './trace/SamplingResult';
export * from './trace/spancontext-utils';
export * from './trace/span_context';
export * from './trace/span_kind';
export * from './trace/span';
Expand Down
10 changes: 1 addition & 9 deletions packages/opentelemetry-api/src/trace/NoopSpan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,7 @@ import { Attributes } from './attributes';
import { Span } from './span';
import { SpanContext } from './span_context';
import { Status } from './status';
import { TraceFlags } from './trace_flags';

export const INVALID_TRACE_ID = '0';
export const INVALID_SPAN_ID = '0';
const INVALID_SPAN_CONTEXT: SpanContext = {
traceId: INVALID_TRACE_ID,
spanId: INVALID_SPAN_ID,
traceFlags: TraceFlags.NONE,
};
import { INVALID_SPAN_CONTEXT } from './spancontext-utils';

/**
* The NoopSpan is the default {@link Span} that is used when no Span
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,33 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { SpanContext } from './span_context';
import { TraceFlags } from './trace_flags';

import { SpanContext, TraceFlags } from '@opentelemetry/api';

export const INVALID_SPANID = '0';
export const INVALID_TRACEID = '0';
const VALID_TRACEID_REGEX = /^([0-9a-f]{32})$/i;
const VALID_SPANID_REGEX = /^[0-9a-f]{16}$/i;
export const INVALID_SPANID = '0000000000000000';
export const INVALID_TRACEID = '00000000000000000000000000000000';
export const INVALID_SPAN_CONTEXT: SpanContext = {
traceId: INVALID_TRACEID,
spanId: INVALID_SPANID,
traceFlags: TraceFlags.NONE,
};

export function isValidTraceId(traceId: string): boolean {
return VALID_TRACEID_REGEX.test(traceId) && traceId !== INVALID_TRACEID;
}

export function isValidSpanId(spanId: string): boolean {
return VALID_SPANID_REGEX.test(spanId) && spanId !== INVALID_SPANID;
}

/**
* Returns true if this {@link SpanContext} is valid.
* @return true if this {@link SpanContext} is valid.
*/
export function isValid(spanContext: SpanContext): boolean {
export function isSpanContextValid(spanContext: SpanContext): boolean {
return (
spanContext.traceId !== INVALID_TRACEID &&
spanContext.spanId !== INVALID_SPANID
isValidTraceId(spanContext.traceId) && isValidSpanId(spanContext.spanId)
srjames90 marked this conversation as resolved.
Show resolved Hide resolved
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
import * as assert from 'assert';
import {
CanonicalCode,
INVALID_SPAN_ID,
INVALID_TRACE_ID,
INVALID_SPANID,
INVALID_TRACEID,
NoopSpan,
TraceFlags,
} from '../../src';
Expand All @@ -45,8 +45,8 @@ describe('NoopSpan', () => {

assert.ok(!span.isRecording());
assert.deepStrictEqual(span.context(), {
traceId: INVALID_TRACE_ID,
spanId: INVALID_SPAN_ID,
traceId: INVALID_TRACEID,
spanId: INVALID_SPANID,
traceFlags: TraceFlags.NONE,
});
span.end();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import * as assert from 'assert';
import * as context from '../../src/trace/spancontext-utils';
import { TraceFlags } from '@opentelemetry/api';
import { TraceFlags } from '../../src';

describe('spancontext-utils', () => {
it('should return true for valid spancontext', () => {
Expand All @@ -25,7 +25,7 @@ describe('spancontext-utils', () => {
spanId: '6e0c63257de34c92',
traceFlags: TraceFlags.NONE,
};
assert.ok(context.isValid(spanContext));
assert.ok(context.isSpanContextValid(spanContext));
});

it('should return false when traceId is invalid', () => {
Expand All @@ -34,7 +34,7 @@ describe('spancontext-utils', () => {
spanId: '6e0c63257de34c92',
traceFlags: TraceFlags.NONE,
};
assert.ok(!context.isValid(spanContext));
assert.ok(!context.isSpanContextValid(spanContext));
});

it('should return false when spanId is invalid', () => {
Expand All @@ -43,7 +43,7 @@ describe('spancontext-utils', () => {
spanId: context.INVALID_SPANID,
traceFlags: TraceFlags.NONE,
};
assert.ok(!context.isValid(spanContext));
assert.ok(!context.isSpanContextValid(spanContext));
});

it('should return false when traceId & spanId is invalid', () => {
Expand All @@ -52,6 +52,6 @@ describe('spancontext-utils', () => {
spanId: context.INVALID_SPANID,
traceFlags: TraceFlags.NONE,
};
assert.ok(!context.isValid(spanContext));
assert.ok(!context.isSpanContextValid(spanContext));
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
import {
Context,
GetterFunction,
isValidSpanId,
isValidTraceId,
TextMapPropagator,
SetterFunction,
TraceFlags,
Expand All @@ -34,20 +36,9 @@ export const PARENT_SPAN_ID_KEY = Context.createKey(
export const DEBUG_FLAG_KEY = Context.createKey(
'OpenTelemetry Context Key B3 Debug Flag'
);
const VALID_TRACEID_REGEX = /^([0-9a-f]{16}){1,2}$/i;
dyladan marked this conversation as resolved.
Show resolved Hide resolved
const VALID_SPANID_REGEX = /^[0-9a-f]{16}$/i;
const INVALID_ID_REGEX = /^0+$/i;
const VALID_SAMPLED_VALUES = new Set([true, 'true', 'True', '1', 1]);
const VALID_UNSAMPLED_VALUES = new Set([false, 'false', 'False', '0', 0]);

function isValidTraceId(traceId: string): boolean {
return VALID_TRACEID_REGEX.test(traceId) && !INVALID_ID_REGEX.test(traceId);
}

function isValidSpanId(spanId: string): boolean {
return VALID_SPANID_REGEX.test(spanId) && !INVALID_ID_REGEX.test(spanId);
}

function isValidParentSpanID(spanId: string | undefined): boolean {
return spanId === undefined || isValidSpanId(spanId);
}
Expand Down
1 change: 0 additions & 1 deletion packages/opentelemetry-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ export * from './trace/sampler/AlwaysOffSampler';
export * from './trace/sampler/AlwaysOnSampler';
export * from './trace/sampler/ParentOrElseSampler';
export * from './trace/sampler/ProbabilitySampler';
export * from './trace/spancontext-utils';
export * from './trace/TraceState';
export * from './trace/IdGenerator';
export * from './utils/url';
Expand Down
7 changes: 5 additions & 2 deletions packages/opentelemetry-core/src/trace/NoRecordingSpan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,11 @@
* limitations under the License.
*/

import { SpanContext, NoopSpan } from '@opentelemetry/api';
import { INVALID_SPAN_CONTEXT } from '../trace/spancontext-utils';
import {
SpanContext,
NoopSpan,
INVALID_SPAN_CONTEXT,
} from '@opentelemetry/api';

/**
* The NoRecordingSpan extends the {@link NoopSpan}, making all operations no-op
Expand Down
3 changes: 1 addition & 2 deletions packages/opentelemetry-shim-opentracing/test/Shim.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,12 @@ import * as opentracing from 'opentracing';
import { BasicTracerProvider, Span } from '@opentelemetry/tracing';
import { TracerShim, SpanShim, SpanContextShim } from '../src/shim';
import {
INVALID_SPAN_CONTEXT,
timeInputToHrTime,
HttpTraceContext,
CompositePropagator,
HttpCorrelationContext,
} from '@opentelemetry/core';
import { propagation } from '@opentelemetry/api';
import { INVALID_SPAN_CONTEXT, propagation } from '@opentelemetry/api';
import { performance } from 'perf_hooks';

describe('OpenTracing Shim', () => {
Expand Down
3 changes: 1 addition & 2 deletions packages/opentelemetry-tracing/src/Tracer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import {
getActiveSpan,
getParentSpanContext,
InstrumentationLibrary,
isValid,
NoRecordingSpan,
IdGenerator,
RandomIdGenerator,
Expand Down Expand Up @@ -79,7 +78,7 @@ export class Tracer implements api.Tracer {
const spanId = this._idGenerator.generateSpanId();
let traceId;
let traceState;
if (!parentContext || !isValid(parentContext)) {
if (!parentContext || !api.isSpanContextValid(parentContext)) {
// New root span.
traceId = this._idGenerator.generateTraceId();
} else {
Expand Down