From 0de66c1bbd119dbdc7db601b94b6a4ec8d90cd28 Mon Sep 17 00:00:00 2001 From: draffensperger Date: Fri, 1 Sep 2017 10:43:20 -0400 Subject: [PATCH] Move getCurrentTraceFromAgent out of logging package --- packages/logging-bunyan/src/index.js | 26 +++++++++++++- packages/logging-bunyan/test/index.js | 36 ++++++++++++++++++++ packages/logging-winston/src/index.js | 26 +++++++++++++- packages/logging-winston/test/index.js | 37 ++++++++++++++++++++ packages/logging/src/index.js | 36 -------------------- packages/logging/test/index.js | 47 -------------------------- 6 files changed, 123 insertions(+), 85 deletions(-) diff --git a/packages/logging-bunyan/src/index.js b/packages/logging-bunyan/src/index.js index ac917d1bad6..84c00bc65c1 100644 --- a/packages/logging-bunyan/src/index.js +++ b/packages/logging-bunyan/src/index.js @@ -186,6 +186,30 @@ LoggingBunyan.prototype.formatEntry_ = function(record) { return this.log_.entry(entryMetadata, record); }; +/** + * Gets the current fully qualified trace ID when available from the + * @google-cloud/trace-agent library in the LogEntry.trace field format of: + * "projects/[PROJECT-ID]/traces/[TRACE-ID]". + */ +function getCurrentTraceFromAgent() { + var agent = global._google_trace_agent; + if (!agent || !agent.getCurrentContextId || !agent.getWriterProjectId) { + return null; + } + + var traceId = agent.getCurrentContextId(); + if (!traceId) { + return null; + } + + var traceProjectId = agent.getWriterProjectId(); + if (!traceProjectId) { + return null; + } + + return `projects/${traceProjectId}/traces/${traceId}`; +} + /** * Intercept log entries as they are written so we can attempt to add the trace * ID in the same continuation as the function that wrote the log, because the @@ -197,7 +221,7 @@ LoggingBunyan.prototype.formatEntry_ = function(record) { LoggingBunyan.prototype.write = function(record, encoding, callback) { record = extend({}, record); if (!record[LOGGING_TRACE_KEY]) { - var trace = logging.getCurrentTraceFromAgent(); + var trace = getCurrentTraceFromAgent(); if (trace) { record[LOGGING_TRACE_KEY] = trace; } diff --git a/packages/logging-bunyan/test/index.js b/packages/logging-bunyan/test/index.js index 92d1dbd64f0..5137afb79e9 100644 --- a/packages/logging-bunyan/test/index.js +++ b/packages/logging-bunyan/test/index.js @@ -298,6 +298,7 @@ describe('logging-bunyan', function() { }); it('should leave prefixed trace property as is if set', function(done) { + var oldTraceAgent = global._google_trace_agent; global._google_trace_agent = { getCurrentContextId: function() { return 'trace-from-agent'; }, getWriterProjectId: function() { return 'project1'; } @@ -315,9 +316,44 @@ describe('logging-bunyan', function() { }; loggingBunyan.write(recordWithTraceAlreadySet, '', assert.ifError); + + global._google_trace_agent = oldTraceAgent; }); }); + it('should not set prefixed trace property if trace unavailable', function() { + FakeWritable.prototype.write = function(record, encoding, callback) { + assert.deepStrictEqual(record, RECORD); + assert.strictEqual(encoding, ''); + assert.strictEqual(callback, assert.ifError); + assert.strictEqual(this, loggingBunyan); + }; + var oldTraceAgent = global._google_trace_agent; + + global._google_trace_agent = {}; + loggingBunyan.write(RECORD, '', assert.ifError); + + global._google_trace_agent = { + getCurrentContextId: function() { return null; }, + getWriterProjectId: function() { return null; } + }; + loggingBunyan.write(RECORD, '', assert.ifError); + + global._google_trace_agent = { + getCurrentContextId: function() { return null; }, + getWriterProjectId: function() { return 'project1'; } + }; + loggingBunyan.write(RECORD, '', assert.ifError); + + global._google_trace_agent = { + getCurrentContextId: function() { return 'trace1'; }, + getWriterProjectId: function() { return null; } + }; + loggingBunyan.write(RECORD, '', assert.ifError); + + global._google_trace_agent = oldTraceAgent; + }); + describe('_write', function() { beforeEach(function() { fakeLogInstance.entry = function() {}; diff --git a/packages/logging-winston/src/index.js b/packages/logging-winston/src/index.js index f2aa2447288..a6f717637a2 100644 --- a/packages/logging-winston/src/index.js +++ b/packages/logging-winston/src/index.js @@ -130,6 +130,30 @@ function LoggingWinston(options) { winston.transports.StackdriverLogging = LoggingWinston; util.inherits(LoggingWinston, winston.Transport); +/** + * Gets the current fully qualified trace ID when available from the + * @google-cloud/trace-agent library in the LogEntry.trace field format of: + * "projects/[PROJECT-ID]/traces/[TRACE-ID]". + */ +function getCurrentTraceFromAgent() { + var agent = global._google_trace_agent; + if (!agent || !agent.getCurrentContextId || !agent.getWriterProjectId) { + return null; + } + + var traceId = agent.getCurrentContextId(); + if (!traceId) { + return null; + } + + var traceProjectId = agent.getWriterProjectId(); + if (!traceProjectId) { + return null; + } + + return `projects/${traceProjectId}/traces/${traceId}`; +} + /** * Relay a log entry to the logging agent. This is normally called by winston. * @@ -207,7 +231,7 @@ LoggingWinston.prototype.log = function(levelName, msg, metadata, callback) { entryMetadata.trace = metadata[LOGGING_TRACE_KEY]; delete data.metadata[LOGGING_TRACE_KEY]; } else { - var trace = logging.getCurrentTraceFromAgent(); + var trace = getCurrentTraceFromAgent(); if (trace) { entryMetadata.trace = trace; } diff --git a/packages/logging-winston/test/index.js b/packages/logging-winston/test/index.js index f0ccba7448a..08678183368 100644 --- a/packages/logging-winston/test/index.js +++ b/packages/logging-winston/test/index.js @@ -350,6 +350,43 @@ describe('logging-winston', function() { global._google_trace_agent = oldTraceAgent; }); + it('should leave out trace metadata if trace unavailable', function() { + loggingWinston.log_.entry = function(entryMetadata, data) { + assert.deepStrictEqual(entryMetadata, { + resource: loggingWinston.resource_, + }); + assert.deepStrictEqual(data, { + message: MESSAGE, + metadata: METADATA + }); + }; + + var oldTraceAgent = global._google_trace_agent; + + global._google_trace_agent = {}; + loggingWinston.log(LEVEL, MESSAGE, METADATA, assert.ifError); + + global._google_trace_agent = { + getCurrentContextId: function() { return null; }, + getWriterProjectId: function() { return null; } + }; + loggingWinston.log(LEVEL, MESSAGE, METADATA, assert.ifError); + + global._google_trace_agent = { + getCurrentContextId: function() { return null; }, + getWriterProjectId: function() { return 'project1'; } + }; + loggingWinston.log(LEVEL, MESSAGE, METADATA, assert.ifError); + + global._google_trace_agent = { + getCurrentContextId: function() { return 'trace1'; }, + getWriterProjectId: function() { return null; } + }; + loggingWinston.log(LEVEL, MESSAGE, METADATA, assert.ifError); + + global._google_trace_agent = oldTraceAgent; + }); + it('should write to the log', function(done) { var entry = {}; diff --git a/packages/logging/src/index.js b/packages/logging/src/index.js index 2a399213f20..3894d8431eb 100644 --- a/packages/logging/src/index.js +++ b/packages/logging/src/index.js @@ -773,41 +773,6 @@ Logging.prototype.setAclForTopic_ = function(name, config, callback) { }); }; -/** - * Attempts to get the current fully qualified trace ID from the - * @google-cloud/trace-agent (if installed) in the format - * "projects/[PROJECT-ID]/traces/[TRACE-ID]". - * - * That is the specified format to set the LogEntry.trace field so that the - * Cloud Console logs and trace viewer can correlate log entries and associate - * them with traces. - * - * This returns null if the trace agent is not installed, there is no available - * trace context or no trace writer project ID. - */ -function getCurrentTraceFromAgent() { - var traceAgent = global._google_trace_agent; - if (!traceAgent || !traceAgent.getCurrentContextId || - !traceAgent.getWriterProjectId) { - // Trace agent is not installed or is an older version - return null; - } - - var traceId = traceAgent.getCurrentContextId(); - if (!traceId) { - // No trace context because request not sampled or context lost - return null; - } - - var traceProjectId = traceAgent.getWriterProjectId(); - if (!traceProjectId) { - // No project, possibly because async project auto-discovery not yet done - return null; - } - - return `projects/${traceProjectId}/traces/${traceId}`; -} - /*! Developer Documentation * * These methods can be auto-paginated. @@ -832,7 +797,6 @@ Logging.Entry = Entry; Logging.Log = Log; Logging.Logging = Logging; Logging.Sink = Sink; -Logging.getCurrentTraceFromAgent = getCurrentTraceFromAgent; module.exports = Logging; module.exports.v2 = v2; diff --git a/packages/logging/test/index.js b/packages/logging/test/index.js index 1a6e8a4ee15..2b333b873a4 100644 --- a/packages/logging/test/index.js +++ b/packages/logging/test/index.js @@ -1379,51 +1379,4 @@ describe('Logging', function() { }); }); }); - - describe('getCurrentTraceFromAgent', function() { - var oldTraceAgent; - - beforeEach(function() { - oldTraceAgent = global._google_trace_agent; - }); - - afterEach(function() { - global._google_trace_agent = oldTraceAgent; - }); - - it('returns null if there is no trace agent', function() { - global._google_trace_agent = undefined; - assert.strictEqual(Logging.getCurrentTraceFromAgent(), null); - }); - - it('returns null if agent does not support context methods', function() { - global._google_trace_agent = {}; - assert.strictEqual(Logging.getCurrentTraceFromAgent(), null); - }); - - it('returns null if context id is missing', function() { - global._google_trace_agent = { - getCurrentContextId: function() { return null; }, - getWriterProjectId: function() { return 'project1'; } - }; - assert.strictEqual(Logging.getCurrentTraceFromAgent(), null); - }); - - it('returns null if project id is missing', function() { - global._google_trace_agent = { - getCurrentContextId: function() { return 'trace1'; }, - getWriterProjectId: function() { return null; } - }; - assert.strictEqual(Logging.getCurrentTraceFromAgent(), null); - }); - - it('returns trace path if project and context available', function() { - global._google_trace_agent = { - getCurrentContextId: function() { return 'trace1'; }, - getWriterProjectId: function() { return 'project1'; } - }; - assert.strictEqual(Logging.getCurrentTraceFromAgent(), - 'projects/project1/traces/trace1'); - }); - }); });