Skip to content

Commit

Permalink
[tracing] - Remove spanContext from our public API (#19786)
Browse files Browse the repository at this point in the history
Taken from #19734 (comment) - this demonstrates what that might look like

This PR removes `TracingSpanContext` from our public types and relies on TracingContext and runtime checks to serialize and deserialize a TracingSpan
  • Loading branch information
maorleger committed Jan 11, 2022
1 parent 9eb67db commit b827600
Show file tree
Hide file tree
Showing 15 changed files with 91 additions and 124 deletions.
15 changes: 3 additions & 12 deletions sdk/core/core-tracing/review/core-tracing.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export function createTracingClient(options: TracingClientOptions): TracingClien
// @public
export interface Instrumenter {
createRequestHeaders(tracingContext?: TracingContext): Record<string, string>;
parseTraceparentHeader(traceparentHeader: string): TracingSpanContext | undefined;
parseTraceparentHeader(traceparentHeader: string): TracingContext | undefined;
startSpan(name: string, spanOptions: InstrumenterSpanOptions): {
span: TracingSpan;
tracingContext: TracingContext;
Expand Down Expand Up @@ -41,7 +41,7 @@ export type SpanStatus = {
// @public
export interface TracingClient {
createRequestHeaders(tracingContext?: TracingContext): Record<string, string>;
parseTraceparentHeader(traceparentHeader: string): TracingSpanContext | undefined;
parseTraceparentHeader(traceparentHeader: string): TracingContext | undefined;
startSpan<Options extends {
tracingOptions?: OperationTracingOptions;
}>(name: string, operationOptions?: Options, spanOptions?: TracingSpanOptions): {
Expand Down Expand Up @@ -75,15 +75,6 @@ export interface TracingSpan {
recordException(exception: Error | string): void;
setAttribute(name: string, value: unknown): void;
setStatus(status: SpanStatus): void;
spanContext(): TracingSpanContext;
}

// @public
export interface TracingSpanContext {
spanId: string;
traceFlags: number;
traceId: string;
traceState?: unknown;
}

// @public
Expand All @@ -94,7 +85,7 @@ export interface TracingSpanLink {
attributes?: {
[key: string]: unknown;
};
spanContext: TracingSpanContext;
tracingContext: TracingContext;
}

// @public
Expand Down
1 change: 0 additions & 1 deletion sdk/core/core-tracing/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ export {
TracingClientOptions,
TracingContext,
TracingSpan,
TracingSpanContext,
TracingSpanKind,
TracingSpanLink,
TracingSpanOptions,
Expand Down
18 changes: 2 additions & 16 deletions sdk/core/core-tracing/src/instrumenter.ts
Original file line number Diff line number Diff line change
@@ -1,28 +1,14 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import {
Instrumenter,
InstrumenterSpanOptions,
TracingContext,
TracingSpan,
TracingSpanContext,
} from "./interfaces";
import { Instrumenter, InstrumenterSpanOptions, TracingContext, TracingSpan } from "./interfaces";
import { createTracingContext } from "./tracingContext";

export function createDefaultTracingSpan(): TracingSpan {
return {
end: () => {
// noop
},
spanContext() {
// The world could always use more zeroes.
return {
spanId: "00000000-0000-0000-0000-000000000000",
traceId: "00000000-0000-0000-0000-000000000000",
traceFlags: 0x0,
};
},
isRecording: () => false,
recordException: () => {
// noop
Expand All @@ -41,7 +27,7 @@ export function createDefaultInstrumenter(): Instrumenter {
createRequestHeaders: (): Record<string, string> => {
return {};
},
parseTraceparentHeader: (): TracingSpanContext | undefined => {
parseTraceparentHeader: (): TracingContext | undefined => {
return undefined;
},
startSpan: (
Expand Down
29 changes: 4 additions & 25 deletions sdk/core/core-tracing/src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export interface TracingClient {
* @param traceparentHeader - The traceparent header to parse.
* @returns An implementation-specific identifier for the span.
*/
parseTraceparentHeader(traceparentHeader: string): TracingSpanContext | undefined;
parseTraceparentHeader(traceparentHeader: string): TracingContext | undefined;

/**
* Creates a set of request headers to propagate tracing information to a backend.
Expand Down Expand Up @@ -114,30 +114,12 @@ export interface TracingSpanOptions {

/** A pointer from the current {@link TracingSpan} to another span in the same or a different trace. */
export interface TracingSpanLink {
/** The {@link TracingSpanContext} to link to. */
spanContext: TracingSpanContext;
/** The {@link TracingContext} containing the span context to link to. */
tracingContext: TracingContext;
/** A set of attributes on the link. */
attributes?: { [key: string]: unknown };
}

/**
* A unique, serializable identifier for a span.
*/
export interface TracingSpanContext {
/** The span UUID within the trace. */
spanId: string;
/** The trace UUID. */
traceId: string;
/**
* https://www.w3.org/TR/trace-context/#trace-flags
*/
traceFlags: number;
/**
* An implementation-specific value representing system-specific trace info to serialize.
*/
traceState?: unknown;
}

/**
* Represents an implementation agnostic instrumenter.
*/
Expand Down Expand Up @@ -174,7 +156,7 @@ export interface Instrumenter {
* Provides an implementation-specific method to parse a {@link https://www.w3.org/TR/trace-context/#traceparent-header}
* into a {@link TracingSpanContext} which can be used to link non-parented spans together.
*/
parseTraceparentHeader(traceparentHeader: string): TracingSpanContext | undefined;
parseTraceparentHeader(traceparentHeader: string): TracingContext | undefined;
/**
* Provides an implementation-specific method to serialize a {@link TracingSpan} to a set of headers.
* @param tracingContext - The context containing the span to serialize.
Expand Down Expand Up @@ -248,9 +230,6 @@ export interface TracingSpan {
* Depending on the span implementation, this may return false if the span is not being sampled.
*/
isRecording(): boolean;

/** Returns the {@link TracingSpanContext} - an immutable, serializable span identifier. */
spanContext(): TracingSpanContext;
}

/** An immutable context bag of tracing values for the current operation. */
Expand Down
3 changes: 1 addition & 2 deletions sdk/core/core-tracing/src/tracingClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
TracingClientOptions,
TracingContext,
TracingSpan,
TracingSpanContext,
TracingSpanOptions,
} from "./interfaces";
import { getInstrumenter } from "./instrumenter";
Expand Down Expand Up @@ -98,7 +97,7 @@ export function createTracingClient(options: TracingClientOptions): TracingClien
* @param traceparentHeader - The traceparent header to parse.
* @returns An implementation-specific identifier for the span.
*/
function parseTraceparentHeader(traceparentHeader: string): TracingSpanContext | undefined {
function parseTraceparentHeader(traceparentHeader: string): TracingContext | undefined {
return getInstrumenter().parseTraceparentHeader(traceparentHeader);
}

Expand Down
5 changes: 0 additions & 5 deletions sdk/core/core-tracing/test/instrumenter.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,6 @@ describe("Instrumenter", () => {
span.setStatus({ status: "success" });
span.setAttribute("foo", "bar");
span.recordException(new Error("test"));
assert.deepEqual(span.spanContext(), {
spanId: "00000000-0000-0000-0000-000000000000",
traceId: "00000000-0000-0000-0000-000000000000",
traceFlags: 0x0,
});
span.end();
assert.isFalse(span.isRecording());
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
InstrumenterSpanOptions,
TracingContext,
TracingSpan,
TracingSpanContext,
} from "@azure/core-tracing";

import { trace, context, defaultTextMapGetter, defaultTextMapSetter } from "@opentelemetry/api";
Expand Down Expand Up @@ -50,19 +49,17 @@ export class OpenTelemetryInstrumenter implements Instrumenter {
);
}

parseTraceparentHeader(traceparentHeader: string): TracingSpanContext | undefined {
const newContext = propagator.extract(
parseTraceparentHeader(traceparentHeader: string): TracingContext {
return propagator.extract(
context.active(),
{ traceparent: traceparentHeader },
defaultTextMapGetter
);
return trace.getSpanContext(newContext);
}

createRequestHeaders(tracingContext?: TracingContext): Record<string, string> {
const headers: Record<string, string> = {};
propagator.inject(tracingContext || context.active(), headers, defaultTextMapSetter);

return headers;
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import { SpanStatus, TracingSpan, TracingSpanContext } from "@azure/core-tracing";
import { SpanStatus, TracingSpan } from "@azure/core-tracing";
import { Span, SpanStatusCode, SpanAttributeValue } from "@opentelemetry/api";

export class OpenTelemetrySpanWrapper implements TracingSpan {
Expand Down Expand Up @@ -42,10 +42,6 @@ export class OpenTelemetrySpanWrapper implements TracingSpan {
return this._span.isRecording();
}

spanContext(): TracingSpanContext {
return this._span.spanContext();
}

/**
* Allows getting the wrapped span as needed.
* @internal
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
SpanAttributes,
SpanKind,
SpanOptions,
TraceState,
trace,
} from "@opentelemetry/api";

/**
Expand Down Expand Up @@ -43,15 +43,16 @@ type SpanKindMapping = {
* @returns A set of {@link Link}s
*/
function toOpenTelemetryLinks(spanLinks: TracingSpanLink[] = []): Link[] {
return spanLinks.map((tracingSpanLink) => {
return {
context: {
...tracingSpanLink.spanContext,
traceState: tracingSpanLink.spanContext.traceState as TraceState,
},
attributes: toOpenTelemetrySpanAttributes(tracingSpanLink.attributes),
};
});
return spanLinks.reduce((acc, tracingSpanLink) => {
const spanContext = trace.getSpanContext(tracingSpanLink.tracingContext);
if (spanContext) {
acc.push({
context: spanContext,
attributes: toOpenTelemetrySpanAttributes(tracingSpanLink.attributes),
});
}
return acc;
}, [] as Link[]);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ import sinon from "sinon";
import { Context } from "mocha";
import { OpenTelemetrySpanWrapper } from "../../src/spanWrapper";

function unwrap(span: TracingSpan): TestSpan {
return (span as OpenTelemetrySpanWrapper).unwrap() as TestSpan;
}

describe("OpenTelemetryInstrumenter", () => {
const instrumenter = new OpenTelemetryInstrumenter();

Expand Down Expand Up @@ -39,9 +43,6 @@ describe("OpenTelemetryInstrumenter", () => {
// TODO: the following still uses existing test support for OTel.
// Once the new APIs are available we should move away from those.
describe("#startSpan", () => {
function unwrap(span: TracingSpan): TestSpan {
return (span as OpenTelemetrySpanWrapper).unwrap() as TestSpan;
}
let tracer: TestTracer;
const packageName = "test-package";
const packageVersion = "test-version";
Expand Down Expand Up @@ -150,12 +151,15 @@ describe("OpenTelemetryInstrumenter", () => {
});

it("supports spanLinks", () => {
const { span: linkedSpan } = instrumenter.startSpan("linked", { packageName });
const { tracingContext: linkedSpanContext } = instrumenter.startSpan("linked", {
packageName,
});

const { span } = instrumenter.startSpan("test", {
packageName,
spanLinks: [
{
spanContext: linkedSpan.spanContext(),
tracingContext: linkedSpanContext,
attributes: {
attr1: "value1",
},
Expand All @@ -165,16 +169,23 @@ describe("OpenTelemetryInstrumenter", () => {

const links = unwrap(span).links;
assert.equal(links.length, 1);
assert.deepEqual(links[0].attributes, { attr1: "value1" });
assert.deepEqual(links[0].context, trace.getSpan(linkedSpanContext)?.spanContext());
});

it("supports spanLinks from traceparentHeader", () => {
const linkedContext = instrumenter.parseTraceparentHeader(
"00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01"
);

assert.deepEqual(links[0], {
attributes: {
attr1: "value1",
},
context: {
...linkedSpan.spanContext(),
traceState: undefined,
},
const { span } = instrumenter.startSpan("test", {
packageName,
spanLinks: [{ tracingContext: linkedContext! }],
});

const links = unwrap(span).links;
assert.equal(links.length, 1);
assert.deepEqual(links[0].context, trace.getSpan(linkedContext!)?.spanContext());
});
});
});
Expand Down Expand Up @@ -219,4 +230,15 @@ describe("OpenTelemetryInstrumenter", () => {
assert.equal(result, 42);
});
});

describe("#parseTraceparentHeader", () => {
it("returns a new context with spanContext set", () => {
const validTraceparentHeader = "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01";
const updatedContext = instrumenter.parseTraceparentHeader(validTraceparentHeader);
assert.exists(updatedContext);
const spanContext = trace.getSpanContext(updatedContext!);
assert.equal(spanContext?.spanId, "00f067aa0ba902b7");
assert.equal(spanContext?.traceId, "4bf92f3577b34da6a3ce929d0e0e4736");
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,4 @@ describe("OpenTelemetrySpanWrapper", () => {
assert.equal(span.isRecording(), otSpan.isRecording());
});
});

describe("#spanContext", () => {
it("returns the wrapped span context", () => {
assert.deepEqual(span.spanContext(), otSpan.spanContext());
});
});
});
Loading

0 comments on commit b827600

Please sign in to comment.