From 409693af6858908962c373b958677a14fef087ec Mon Sep 17 00:00:00 2001 From: Kelvin Jin Date: Mon, 14 May 2018 15:47:44 -0700 Subject: [PATCH] chore: address comments, pt 2 --- src/index.ts | 35 +++++++++++------------------------ src/tracing.ts | 11 +++++++---- 2 files changed, 18 insertions(+), 28 deletions(-) diff --git a/src/index.ts b/src/index.ts index 6a163134a..5cc78daa9 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,5 +1,5 @@ /** - * Copyright 2018 Google LLC + * Copyright 2015 Google Inc. All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,10 +26,11 @@ import * as PluginTypes from './plugin-types'; import {tracing, Tracing, NormalizedConfig} from './tracing'; import {Singleton, FORCE_NEW, Forceable} from './util'; import {Constants} from './constants'; +import {TraceAgent} from './trace-api'; export {Config, PluginTypes}; -let tracingSingleton: typeof tracing; +let traceAgent: TraceAgent; /** * Normalizes the user-provided configuration object by adding default values @@ -110,14 +111,15 @@ export function start(config?: Config): PluginTypes.TraceAgent { require('continuation-local-storage'); } - if (!tracingSingleton) { - tracingSingleton = require('./tracing').tracing; + if (!traceAgent) { + traceAgent = new (require('./trace-api').TraceAgent)(); } try { let tracing: Tracing; try { - tracing = tracingSingleton.create(normalizedConfig, {}); + tracing = + require('./tracing').tracing.create(normalizedConfig, traceAgent); } catch (e) { // An error could be thrown if create() is called multiple times. // It's not a helpful error message for the end user, so make it more @@ -126,9 +128,7 @@ export function start(config?: Config): PluginTypes.TraceAgent { } tracing.enable(); tracing.logModulesLoadedBeforeTrace(filesLoadedBeforeTrace); - return tracingSingleton.get().traceAgent; - } catch (e) { - throw e; + return traceAgent; } finally { // Stop storing these entries in memory filesLoadedBeforeTrace.length = 0; @@ -140,23 +140,10 @@ export function start(config?: Config): PluginTypes.TraceAgent { * @returns An object exposing functions for creating custom spans. */ export function get(): PluginTypes.TraceAgent { - if (!tracingSingleton) { - tracingSingleton = require('./tracing').tracing; - } - if (tracingSingleton.exists()) { - return tracingSingleton.get().traceAgent; - } else { - // This code path maintains the current contract that calling get() before - // start() yields a disabled custom span API. It assumes that the use case - // for doing so (instead of returning null) is when get() is called in - // a file where it is unknown whether start() has been called. - - // Based on this assumption, and because we document that start() must be - // called first in an application, it's OK to create a permanently disabled - // Trace Agent here and assume that start() will never be called to enable - // it. - return tracingSingleton.create({enabled: false}, {}).traceAgent; + if (!traceAgent) { + traceAgent = new (require('./trace-api').TraceAgent)(); } + return traceAgent; } // If the module was --require'd from the command line, start the agent. diff --git a/src/tracing.ts b/src/tracing.ts index 3341b5dae..c506d22bb 100644 --- a/src/tracing.ts +++ b/src/tracing.ts @@ -42,16 +42,19 @@ export type NormalizedConfig = */ export class Tracing implements Component { /** An object representing the custom span API. */ - readonly traceAgent: TraceAgent = new TraceAgent('Custom Trace API'); - private logger: common.Logger; - private config: Forceable; + private readonly traceAgent: TraceAgent; + /** A logger. */ + private readonly logger: common.Logger; + /** The configuration object for this instance. */ + private readonly config: Forceable; /** * Constructs a new Tracing instance. * @param config The configuration for this instance. */ - constructor(config: NormalizedConfig) { + constructor(config: NormalizedConfig, traceAgent: TraceAgent) { this.config = config; + this.traceAgent = traceAgent; let logLevel = config.enabled ? config.logLevel : 0; // Clamp the logger level. if (logLevel < 0) {