Skip to content

Commit

Permalink
refactor: address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
kjin committed Sep 25, 2017
1 parent 24a558a commit 9440476
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 13 deletions.
4 changes: 2 additions & 2 deletions src/cls-ah.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ asyncHook.createHook({init, before, destroy}).enable();
const EVENT_EMITTER_METHODS =
[ 'addListener', 'on', 'once', 'prependListener', 'prependOncelistener' ];

class Namespace implements CLSNamespace {
class AsyncHooksNamespace implements CLSNamespace {
get name(): string {
throw new Error('Not implemented');
}
Expand Down Expand Up @@ -98,7 +98,7 @@ class Namespace implements CLSNamespace {
}
}

const namespace = new Namespace();
const namespace = new AsyncHooksNamespace();

// AsyncWrap Hooks

Expand Down
7 changes: 5 additions & 2 deletions src/cls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,11 @@
'use strict';

import * as semver from 'semver';
import { SpanData } from './span-data';
import * as CLS from 'continuation-local-storage';

export type RootContext = SpanData | {} /* null span */ | null;

const cls: typeof CLS = semver.satisfies(process.version, '>=8') &&
process.env.GCLOUD_TRACE_NEW_CONTEXT ? require('./cls-ah')
: require('continuation-local-storage');
Expand All @@ -37,7 +40,7 @@ export function getNamespace(): CLS.Namespace {
return cls.getNamespace(TRACE_NAMESPACE);
}

export function getRootContext(): any {
export function getRootContext(): RootContext {
// First getNamespace check is necessary in case any
// patched closures escaped before the agent was stopped and the
// namespace was destroyed.
Expand All @@ -47,6 +50,6 @@ export function getRootContext(): any {
return null;
}

export function setRootContext(rootContext: any): void {
export function setRootContext(rootContext: RootContext): void {
getNamespace().set('root', rootContext);
}
6 changes: 4 additions & 2 deletions src/trace-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,8 @@ TraceAgent.prototype.getCurrentContextId = function() {
if (!rootSpan || rootSpan === nullSpan) {
return null;
}
return rootSpan.trace.traceId;
// TODO: When converting this file to TS, remove this cast.
return (rootSpan as SpanData).trace.traceId;
};

/**
Expand All @@ -220,7 +221,8 @@ TraceAgent.prototype.getWriterProjectId = function() {
* @returns A new ChildSpan object, or null if there is no active root span.
*/
TraceAgent.prototype.createChildSpan = function(options) {
var rootSpan = cls.getRootContext();
// TODO: When this file is converted to TS, remove this cast.
var rootSpan = cls.getRootContext() as SpanData | null;
if (!rootSpan) {
// Context was lost.
this.logger_.warn(this.pluginName_ + ': Attempted to create child span ' +
Expand Down
14 changes: 7 additions & 7 deletions src/tracing-policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
/**
* An object that determines whether a request should be traced.
*/
export interface Policy {
export interface TracePolicy {
shouldTrace(dateMillis: number, url: string): boolean;
}

export class RateLimiterPolicy implements Policy {
export class RateLimiterPolicy implements TracePolicy {
private traceWindow: number;
private nextTraceStart: number;

Expand All @@ -42,9 +42,9 @@ export class RateLimiterPolicy implements Policy {
}
}

export class FilterPolicy implements Policy {
export class FilterPolicy implements TracePolicy {
constructor(
private basePolicy: Policy,
private basePolicy: TracePolicy,
private filterUrls: (string | RegExp)[]
) {}

Expand All @@ -60,13 +60,13 @@ export class FilterPolicy implements Policy {
}
}

export class TraceAllPolicy implements Policy {
export class TraceAllPolicy implements TracePolicy {
shouldTrace() {
return true;
}
}

export class TraceNonePolicy implements Policy {
export class TraceNonePolicy implements TracePolicy {
shouldTrace() {
return false;
}
Expand All @@ -78,7 +78,7 @@ export interface TracePolicyOptions {
}

// TODO(kjin): This could be a class as well.
export function createTracePolicy(config: TracePolicyOptions): Policy {
export function createTracePolicy(config: TracePolicyOptions): TracePolicy {
let basePolicy;
if (config.samplingRate < 1) {
basePolicy = new TraceAllPolicy();
Expand Down

0 comments on commit 9440476

Please sign in to comment.