Skip to content

Commit

Permalink
feat: add rootSpan.createChildSpan and change none CLS semantics (#731)
Browse files Browse the repository at this point in the history
PR-URL: #731
  • Loading branch information
kjin authored May 2, 2018
1 parent 6e46ed1 commit d0009ff
Show file tree
Hide file tree
Showing 12 changed files with 289 additions and 88 deletions.
20 changes: 12 additions & 8 deletions src/cls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ import * as semver from 'semver';
import {AsyncHooksCLS} from './cls/async-hooks';
import {AsyncListenerCLS} from './cls/async-listener';
import {CLS, Func} from './cls/base';
import {UniversalCLS} from './cls/universal';
import {NullCLS} from './cls/null';
import {SpanDataType} from './constants';
import {SpanData, SpanOptions} from './plugin-types';
import {Trace, TraceSpan} from './trace';
import {Singleton} from './util';

Expand All @@ -31,6 +32,7 @@ const asyncHooksAvailable = semver.satisfies(process.version, '>=8');
export interface RealRootContext {
readonly span: TraceSpan;
readonly trace: Trace;
createChildSpan(options: SpanOptions): SpanData;
readonly type: SpanDataType.ROOT;
}

Expand Down Expand Up @@ -119,7 +121,7 @@ export class TraceCLS implements CLS<RootContext> {
this.rootSpanStackOffset = 8;
break;
case TraceCLSMechanism.NONE:
this.CLSClass = UniversalCLS;
this.CLSClass = NullCLS;
this.rootSpanStackOffset = 4;
break;
default:
Expand All @@ -128,7 +130,7 @@ export class TraceCLS implements CLS<RootContext> {
}
this.logger.info(
`TraceCLS#constructor: Created [${config.mechanism}] CLS instance.`);
this.currentCLS = new UniversalCLS(TraceCLS.UNTRACED);
this.currentCLS = new NullCLS(TraceCLS.UNTRACED);
this.currentCLS.enable();
}

Expand All @@ -137,23 +139,25 @@ export class TraceCLS implements CLS<RootContext> {
}

enable(): void {
if (!this.enabled) {
// if this.CLSClass = NullCLS, the user specifically asked not to use
// any context propagation mechanism. So nothing should change.
if (!this.enabled && this.CLSClass !== NullCLS) {
this.logger.info('TraceCLS#enable: Enabling CLS.');
this.enabled = true;
this.currentCLS.disable();
this.currentCLS = new this.CLSClass(TraceCLS.UNCORRELATED);
this.currentCLS.enable();
}
this.enabled = true;
}

disable(): void {
if (this.enabled) {
if (this.enabled && this.CLSClass !== NullCLS) {
this.logger.info('TraceCLS#disable: Disabling CLS.');
this.enabled = false;
this.currentCLS.disable();
this.currentCLS = new UniversalCLS(TraceCLS.UNTRACED);
this.currentCLS = new NullCLS(TraceCLS.UNTRACED);
this.currentCLS.enable();
}
this.enabled = false;
}

getContext(): RootContext {
Expand Down
20 changes: 6 additions & 14 deletions src/cls/universal.ts → src/cls/null.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,13 @@ import {EventEmitter} from 'events';
import {CLS, Func} from './base';

/**
* A trivial implementation of continuation-local storage where everything is
* in the same continuation.
* A trivial implementation of continuation-local storage where context takes on
* a default, immutable value.
*/
export class UniversalCLS<Context> implements CLS<Context> {
export class NullCLS<Context> implements CLS<Context> {
private enabled = false;
private currentContext: Context;

constructor(private readonly defaultContext: Context) {
this.currentContext = this.defaultContext;
}
constructor(private readonly defaultContext: Context) {}

isEnabled(): boolean {
return this.enabled;
Expand All @@ -40,18 +37,13 @@ export class UniversalCLS<Context> implements CLS<Context> {

disable(): void {
this.enabled = false;
this.setContext(this.defaultContext);
}

getContext(): Context {
return this.currentContext;
return this.defaultContext;
}

setContext(value: Context): void {
if (this.enabled) {
this.currentContext = value;
}
}
setContext(value: Context): void {}

runWithNewContext<T>(fn: Func<T>): T {
return fn();
Expand Down
31 changes: 24 additions & 7 deletions src/plugin-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export interface TraceAgentExtension { _google_trace_patched: boolean; }

/**
* Represents a trace span.
* TODO(kjin): This should be called `Span`.
*/
export interface SpanData {
/**
Expand Down Expand Up @@ -55,6 +56,22 @@ export interface SpanData {
endSpan(): void;
}

/**
* Represents the root span within a trace.
*/
export interface RootSpanData extends SpanData {
/**
* Creates and starts a child span under this root span.
* If the root span is a real span (type = ROOT), the child span will be as
* well (type = CHILD).
* Otherwise, if the root span's type is UNTRACED or UNCORRELATED, the child
* span will be of the same type.
* @param options Options for creating the child span.
* @returns A new SpanData object.
*/
createChildSpan(options?: SpanOptions): SpanData;
}

/**
* An interface that describes the available options for creating a span in
* general.
Expand Down Expand Up @@ -105,7 +122,7 @@ export interface TraceAgent {
* a phantom SpanData object.
* @returns The return value of calling fn.
*/
runInRootSpan<T>(options: RootSpanOptions, fn: (span: SpanData) => T): T;
runInRootSpan<T>(options: RootSpanOptions, fn: (span: RootSpanData) => T): T;

/**
* Returns a unique identifier for the currently active context. This can be
Expand All @@ -125,14 +142,14 @@ export interface TraceAgent {
getWriterProjectId(): string|null;

/**
* Creates and returns a new SpanData object nested within the root span.
* If there is no root span, a phantom SpanData object will be
* returned instead.
* @param options An object that specifies options for how the child
* span is created and propagated.
* Creates and returns a new SpanData object nested within the current root
* span, which is detected automatically.
* If the root span is a phantom span or doesn't exist, the child span will
* be a phantom span as well.
* @param options Options for creating the child span.
* @returns A new SpanData object.
*/
createChildSpan(options: SpanOptions): SpanData;
createChildSpan(options?: SpanOptions): SpanData;

/**
* Returns whether a given span is real or not by checking its SpanDataType.
Expand Down
50 changes: 43 additions & 7 deletions src/span-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ import * as crypto from 'crypto';
import * as util from 'util';

import {Constants, SpanDataType} from './constants';
import {SpanData as SpanData} from './plugin-types';
import * as types from './plugin-types';
import {SpanData, SpanOptions} from './plugin-types';
import {SpanKind, Trace, TraceSpan} from './trace';
import {TraceLabels} from './trace-labels';
import {traceWriter} from './trace-writer';
Expand Down Expand Up @@ -110,7 +111,7 @@ export abstract class BaseSpanData implements SpanData {
/**
* Represents a real root span, which corresponds to an incoming request.
*/
export class RootSpanData extends BaseSpanData {
export class RootSpanData extends BaseSpanData implements types.RootSpanData {
readonly type = SpanDataType.ROOT;

constructor(
Expand All @@ -120,6 +121,16 @@ export class RootSpanData extends BaseSpanData {
this.span.kind = SpanKind.RPC_SERVER;
}

createChildSpan(options?: SpanOptions): SpanData {
options = options || {name: ''};
const skipFrames = options.skipFrames ? options.skipFrames + 1 : 1;
return new ChildSpanData(
this.trace, /* Trace object */
options.name, /* Span name */
this.span.spanId, /* Parent's span ID */
skipFrames); /* # of frames to skip in stack trace */
}

endSpan() {
super.endSpan();
traceWriter.get().writeSpan(this.trace);
Expand Down Expand Up @@ -156,14 +167,39 @@ function createPhantomSpanData<T extends SpanDataType>(spanType: T): SpanData&
}

/**
* A virtual trace span that indicates that a real trace span couldn't be
* created because context was lost.
* A virtual trace span that indicates that a real child span couldn't be
* created because the correct root span couldn't be determined.
*/
export const UNCORRELATED_SPAN =
export const UNCORRELATED_CHILD_SPAN =
createPhantomSpanData(SpanDataType.UNCORRELATED);

/**
* A virtual trace span that indicates that a real trace span couldn't be
* A virtual trace span that indicates that a real child span couldn't be
* created because the corresponding root span was disallowed by user
* configuration.
*/
export const UNTRACED_CHILD_SPAN = createPhantomSpanData(SpanDataType.UNTRACED);

/**
* A virtual trace span that indicates that a real root span couldn't be
* created because an active root span context already exists.
*/
export const UNCORRELATED_ROOT_SPAN = Object.freeze(Object.assign(
{
createChildSpan() {
return UNCORRELATED_CHILD_SPAN;
}
},
UNCORRELATED_CHILD_SPAN));

/**
* A virtual trace span that indicates that a real root span couldn't be
* created because it was disallowed by user configuration.
*/
export const UNTRACED_SPAN = createPhantomSpanData(SpanDataType.UNTRACED);
export const UNTRACED_ROOT_SPAN = Object.freeze(Object.assign(
{
createChildSpan() {
return UNTRACED_CHILD_SPAN;
}
},
UNTRACED_CHILD_SPAN));
40 changes: 20 additions & 20 deletions src/trace-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ import * as uuid from 'uuid';

import {cls} from './cls';
import {Constants, SpanDataType} from './constants';
import {Func, RootSpanOptions, SpanData, SpanOptions, TraceAgent as TraceAgentInterface} from './plugin-types';
import {ChildSpanData, RootSpanData, UNCORRELATED_SPAN, UNTRACED_SPAN} from './span-data';
import {Func, RootSpanData as RootSpanDataInterface, RootSpanOptions, SpanData, SpanOptions, TraceAgent as TraceAgentInterface} from './plugin-types';
import {ChildSpanData, RootSpanData, UNCORRELATED_CHILD_SPAN, UNCORRELATED_ROOT_SPAN, UNTRACED_CHILD_SPAN, UNTRACED_ROOT_SPAN} from './span-data';
import {SpanKind, Trace} from './trace';
import {TraceLabels} from './trace-labels';
import * as TracingPolicy from './tracing-policy';
Expand Down Expand Up @@ -121,18 +121,20 @@ export class TraceAgent implements TraceAgentInterface {
return !!this.config && this.config.enhancedDatabaseReporting;
}

runInRootSpan<T>(options: RootSpanOptions, fn: (span: SpanData) => T): T {
runInRootSpan<T>(
options: RootSpanOptions, fn: (span: RootSpanDataInterface) => T): T {
if (!this.isActive()) {
return fn(UNTRACED_SPAN);
return fn(UNTRACED_ROOT_SPAN);
}

// TODO validate options
options = options || {name: ''};

// Don't create a root span if we are already in a root span
const rootSpan = cls.get().getContext();
if (rootSpan.type === SpanDataType.ROOT && !rootSpan.span.endTime) {
this.logger!.warn(`TraceApi#runInRootSpan: [${
this.pluginName}] Cannot create nested root spans.`);
return fn(UNCORRELATED_SPAN);
return fn(UNCORRELATED_ROOT_SPAN);
}

return cls.get().runWithNewContext(() => {
Expand All @@ -155,8 +157,8 @@ export class TraceAgent implements TraceAgentInterface {
!!(incomingTraceContext.options &
Constants.TRACE_OPTIONS_TRACE_ENABLED);
if (!locallyAllowed || !remotelyAllowed) {
cls.get().setContext(UNTRACED_SPAN);
return fn(UNTRACED_SPAN);
cls.get().setContext(UNTRACED_ROOT_SPAN);
return fn(UNTRACED_ROOT_SPAN);
}

// Create a new root span, and invoke fn with it.
Expand Down Expand Up @@ -193,11 +195,12 @@ export class TraceAgent implements TraceAgentInterface {
}
}

createChildSpan(options: SpanOptions): SpanData {
createChildSpan(options?: SpanOptions): SpanData {
if (!this.isActive()) {
return UNTRACED_SPAN;
return UNTRACED_CHILD_SPAN;
}

options = options || {name: ''};
const rootSpan = cls.get().getContext();
if (rootSpan.type === SpanDataType.ROOT) {
if (!!rootSpan.span.endTime) {
Expand All @@ -212,29 +215,26 @@ export class TraceAgent implements TraceAgentInterface {
this.pluginName}] Creating phantom child span [${
options.name}] because root span [${
rootSpan.span.name}] was already closed.`);
return UNCORRELATED_SPAN;
return UNCORRELATED_CHILD_SPAN;
}
// Create a new child span and return it.
options = options || {name: ''};
const skipFrames = options.skipFrames ? options.skipFrames + 1 : 1;
const childContext = new ChildSpanData(
rootSpan.trace, /* Trace object */
options.name, /* Span name */
rootSpan.span.spanId, /* Parent's span ID */
skipFrames); /* # of frames to skip in stack trace */
const childContext = rootSpan.createChildSpan({
name: options.name,
skipFrames: options.skipFrames ? options.skipFrames + 1 : 1
});
this.logger!.info(`TraceApi#createChildSpan: [${
this.pluginName}] Created child span [${options.name}]`);
return childContext;
} else if (rootSpan.type === SpanDataType.UNTRACED) {
// Context wasn't lost, but there's no root span, indicating that this
// request should not be traced.
return UNTRACED_SPAN;
return UNTRACED_CHILD_SPAN;
} else {
// Context was lost.
this.logger!.warn(`TraceApi#createChildSpan: [${
this.pluginName}] Creating phantom child span [${
options.name}] because there is no root span.`);
return UNCORRELATED_SPAN;
return UNCORRELATED_CHILD_SPAN;
}
}

Expand Down
4 changes: 2 additions & 2 deletions test/plugins/test-trace-grpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { TraceLabels } from '../../src/trace-labels';
import * as TracingPolicy from '../../src/tracing-policy';
import * as util from '../../src/util';
import * as assert from 'assert';
import { asBaseSpanData } from '../utils';
import { asRootSpanData } from '../utils';
import { SpanData } from '../../src/plugin-types';
import { FORCE_NEW } from '../../src/util';

Expand Down Expand Up @@ -55,7 +55,7 @@ function checkServerMetadata(metadata) {
assert.ok(/[a-f0-9]{32}\/[0-9]+;o=1/.test(traceContext));
var parsedContext = util.parseContextFromHeader(traceContext);
assert.ok(parsedContext);
var root = asBaseSpanData(cls.get().getContext() as SpanData);
var root = asRootSpanData(cls.get().getContext() as SpanData);
assert.strictEqual(root.span.parentSpanId, parsedContext!.spanId);
}
}
Expand Down
1 change: 1 addition & 0 deletions test/plugins/test-trace-http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ for (const nodule of Object.keys(servers) as Array<keyof typeof servers>) {
describe(`${nodule} client tracing`, () => {
let http: {get: HttpRequest; request: HttpRequest;};
before(() => {
trace.setCLS();
trace.setPluginLoader();
trace.start({
plugins: {
Expand Down
Loading

0 comments on commit d0009ff

Please sign in to comment.