New configuration for globally controlled label value sizes (#415)

PR-URL: #415

config.js

@@ -29,9 +29,9 @@ module.exports = {
     // attached to spans representating database operations.
     enhancedDatabaseReporting: false,
 
-    // The maximum result size in characters to report on database spans if
-    // `enhancedDatabaseReporting` is enabled.
-    databaseResultReportingSize: 127,
+    // The maximum number of characters reported on a label value. This
+    // cannot exceed 16383, the maximum value accepted by the service.
+    maximumLabelValueSize: 512,
 
     // A list of trace plugins to load. Each field's key in this object is the
     // name of the module to trace, and its value is the require-friendly path

index.js

@@ -48,7 +48,11 @@ var initConfig = function(projectConfig) {
     logLevel: process.env.GCLOUD_TRACE_LOGLEVEL,
     projectId: process.env.GCLOUD_PROJECT
   };
-  return extend(true, {}, require('./config.js').trace, projectConfig, envConfig);
+  var config = extend(true, {}, require('./config.js').trace, projectConfig, envConfig);
+  if (config.maximumLabelValueSize > constants.TRACE_SERVICE_LABEL_VALUE_LIMIT) {
+    config.maximumLabelValueSize = constants.TRACE_SERVICE_LABEL_VALUE_LIMIT;
+  }
+  return config;
 };
 
 var traceApi = new TraceApi('Custom Span API');

src/plugins/plugin-mongodb-core.js

@@ -74,7 +74,7 @@ function wrapCallback(api, span, done) {
       }
       if (res) {
         var result = res.result ? res.result : res;
-        span.addLabel('result', api.summarizeDatabaseResults(result));
+        span.addLabel('result', result);
       }
     }
     span.endSpan();

src/plugins/plugin-mysql.js

@@ -55,7 +55,7 @@ function wrapCallback(api, span, done) {
         span.addLabel('error', err);
       }
       if (res) {
-        span.addLabel('result', api.summarizeDatabaseResults(res));
+        span.addLabel('result', res);
       }
     }
     span.endSpan();

src/plugins/plugin-redis.js

@@ -125,7 +125,7 @@ function wrapCallback(api, span, done) {
         span.addLabel('error', err);
       }
       if (res) {
-        span.addLabel('result', api.summarizeDatabaseResults(res));
+        span.addLabel('result', res);
       }
     }
     span.endSpan();

src/span-data.js

@@ -16,9 +16,12 @@
 
 'use strict';
 
+var constants = require('./constants.js');
 var is = require('is');
 var TraceSpan = require('./trace-span.js');
 var TraceLabels = require('./trace-labels.js');
+var traceUtil = require('./util.js');
+var util = require('util');
 
 // Auto-incrementing integer
 var uid = 1;
@@ -35,7 +38,8 @@ var uid = 1;
 function SpanData(agent, trace, name, parentSpanId, isRoot, skipFrames) {
   var spanId = uid++;
   this.agent = agent;
-  this.span = new TraceSpan(name, spanId, parentSpanId);
+  var spanName = traceUtil.truncate(name, constants.TRACE_SERVICE_SPAN_NAME_LIMIT);
+  this.span = new TraceSpan(spanName, spanId, parentSpanId);
   this.trace = trace;
   this.isRoot = isRoot;
   trace.spans.push(this.span);
@@ -71,8 +75,11 @@ function SpanData(agent, trace, name, parentSpanId, isRoot, skipFrames) {
         callSite.getFileName(), callSite.getLineNumber(),
         callSite.getColumnNumber()));
     });
+    // Set the label on the trace span directly to bypass truncation to
+    // config.maxLabelValueSize.
     this.span.setLabel(TraceLabels.STACK_TRACE_DETAILS_KEY,
-      JSON.stringify({stack_frame: stackFrames}));
+      traceUtil.truncate(JSON.stringify({stack_frame: stackFrames}),
+        constants.TRACE_SERVICE_LABEL_VALUE_LIMIT));
 
     Error.stackTraceLimit = origLimit;
     Error.prepareStackTrace = origPrepare;
@@ -92,7 +99,10 @@ SpanData.prototype.createChildSpanData = function(name, skipFrames) {
 };
 
 SpanData.prototype.addLabel = function(key, value) {
-  this.span.setLabel(key, value);
+  var k = traceUtil.truncate(key, constants.TRACE_SERVICE_LABEL_KEY_LIMIT);
+  var string_val = typeof value === 'string' ? value : util.inspect(value);
+  var v = traceUtil.truncate(string_val, this.agent.config().maximumLabelValueSize);
+  this.span.setLabel(k, v);
 };
 
 /**

src/trace-api.js

@@ -20,7 +20,6 @@ var constants = require('./constants.js');
 var extend = require('extend');
 var is = require('is');
 var TraceLabels = require('./trace-labels.js');
-var traceUtil = require('./util.js');
 
 /**
  * This file describes an interface for third-party plugins to enable tracing
@@ -126,16 +125,6 @@ TraceApiImplementation.prototype.enhancedDatabaseReportingEnabled = function() {
   return this.agent_.config_.enhancedDatabaseReporting;
 };
 
-/**
- * Summarizes database results correctly applying the databaseResultReportingSize
- * configuration option.
- * @returns a summarization of `res`.
- */
-TraceApiImplementation.prototype.summarizeDatabaseResults = function(res) {
-  return traceUtil.stringifyPrefix(res,
-         this.agent_.config_.databaseResultReportingSize);
-};
-
 /**
  * Runs the given function in a root span corresponding to an incoming request,
  * possibly passing it an object that exposes an interface for adding labels
@@ -232,7 +221,6 @@ TraceApiImplementation.prototype.labels = TraceLabels;
  */
 var phantomApiImpl = {
   enhancedDatabaseReportingEnabled: function() { return false; },
-  summarizeDatabaseResults: function(results) { return results; },
   runInRootSpan: function(opts, fn) { return fn(nullSpan); },
   createChildSpan: function(opts) { return nullSpan; },
   wrap: function(fn) { return fn; },
@@ -262,9 +250,6 @@ module.exports = function TraceApi(pluginName) {
     enhancedDatabaseReportingEnabled: function() {
       return impl.enhancedDatabaseReportingEnabled();
     },
-    summarizeDatabaseResults: function(results) {
-      return impl.summarizeDatabaseResults(results);
-    },
     runInRootSpan: function(opts, fn) {
       return impl.runInRootSpan(opts, fn);
     },

src/trace-span.js

@@ -16,34 +16,12 @@
 
 'use strict';
 
-var util = require('util');
-var constants = require('./constants.js');
-
-/**
- * Truncates the provided `string` to be at most `length` bytes
- * after utf8 encoding and the appending of '...'.
- * We produce the result by iterating over input characters to
- * avoid truncating the string potentially producing partial unicode
- * characters at the end.
- */
-function truncate(string, length) {
-  string = string.substr(0, length - 3);
-  while (Buffer.byteLength(string, 'utf8') > length - 3) {
-    string = string.substr(0, string.length - 1);
-  }
-  return string + '...';
-}
-
 /**
  * Creates a trace span object.
  * @constructor
  */
 function TraceSpan(name, spanId, parentSpanId) {
-  if (Buffer.byteLength(name, 'utf8') > constants.TRACE_SERVICE_SPAN_NAME_LIMIT) {
-    this.name = truncate(name, constants.TRACE_SERVICE_SPAN_NAME_LIMIT);
-  } else {
-    this.name = name;
-  }
+  this.name = name;
   this.parentSpanId = parentSpanId;
   this.spanId = spanId;
   this.kind = 'RPC_CLIENT';
@@ -59,14 +37,7 @@ function TraceSpan(name, spanId, parentSpanId) {
  * @param {string} value The new value of the label.
  */
 TraceSpan.prototype.setLabel = function(key, value) {
-  if (Buffer.byteLength(key, 'utf8') > constants.TRACE_SERVICE_LABEL_KEY_LIMIT) {
-    key = truncate(key, constants.TRACE_SERVICE_LABEL_KEY_LIMIT);
-  }
-  var val = typeof value === 'object' ? util.inspect(value) : '' + value;
-  if (Buffer.byteLength(val, 'utf8') > constants.TRACE_SERVICE_LABEL_VALUE_LIMIT) {
-    val = truncate(val, constants.TRACE_SERVICE_LABEL_VALUE_LIMIT);
-  }
-  this.labels[key] = val;
+  this.labels[key] = value;
 };
 
 

src/util.js

@@ -21,50 +21,21 @@ var fs = require('fs');
 var path = require('path');
 
 /**
- * Produces an object summarization of limited size. This summarization
- * does not adhere to the JSON spec so it cannot be reparsed even if
- * the entire object fits inside the desired size.
- *
- * @param {Object} o The object to be summarized.
- * @param {number} n Max length of the summary.
+ * Truncates the provided `string` to be at most `length` bytes
+ * after utf8 encoding and the appending of '...'.
+ * We produce the result by iterating over input characters to
+ * avoid truncating the string potentially producing partial unicode
+ * characters at the end.
  */
-function stringifyPrefix(o, n) {
-  var buf = new Buffer(n);
-  var pos = 0;
-  var worklist = [];
-  function pushObject(o) {
-    var keys = Object.keys(o);
-    for (var i = Math.min(keys.length - 1, Math.floor(n/4)); i >= 0; i--) {
-      worklist.push((i === keys.length - 1) ? '}' : ',');
-      worklist.push(o[keys[i]]);
-      worklist.push(':');
-      worklist.push(keys[i]);
-    }
-    worklist.push('{');
+function truncate(string, length) {
+  if (Buffer.byteLength(string, 'utf8') <= length) {
+    return string;
   }
-  worklist.push(o);
-  while (worklist.length > 0) {
-    var elem = worklist.pop();
-    if (elem && typeof elem === 'object') {
-      pushObject(elem);
-    } else {
-      var val;
-      if (typeof elem === 'function') {
-        val = '[Function]';
-      } else if (typeof elem === 'string') {
-        val = elem;
-      } else {
-        // Undefined, Null, Boolean, Number, Symbol
-        val = String(elem);
-      }
-      pos += buf.write(val, pos);
-      if (buf.length === pos) {
-        buf.write('...', pos - 3);
-        break;
-      }
-    }
+  string = string.substr(0, length - 3);
+  while (Buffer.byteLength(string, 'utf8') > length - 3) {
+    string = string.substr(0, string.length - 1);
   }
-  return buf.toString('utf8', 0, pos);
+  return string + '...';
 }
 
 // Includes support for npm '@org/name' packages
@@ -128,7 +99,7 @@ function findModuleVersion(modulePath, load) {
 }
 
 module.exports = {
-  stringifyPrefix: stringifyPrefix,
+  truncate: truncate,
   packageNameFromPath: packageNameFromPath,
   findModulePath: findModulePath,
   findModuleVersion: findModuleVersion

test/plugins/test-trace-mongodb.js

@@ -201,25 +201,6 @@ describe('mongodb', function() {
           });
         });
       });
-
-      it('should limit result size', function(done) {
-        common.runInTransaction(agent, function(endTransaction) {
-          server.cursor('testdb.simples', {
-            find: 'testdb.simples',
-            query: {f1: 'sim'}
-          }).next(function(err, doc) {
-            endTransaction();
-            assert.ifError(err);
-            var trace = common.getMatchingSpan(
-                agent,
-                mongoPredicate.bind(null, 'mongo-cursor'));
-            var labels = trace.labels;
-            assert.equal(labels.result.length, RESULT_SIZE);
-            assert.equal(labels.result, '{_...');
-            done();
-          });
-        });
-      });
     });
   });
 });

test/plugins/test-trace-mysql.js

@@ -102,22 +102,6 @@ describe('test-trace-mysql', function() {
     });
   });
 
-  it('should limit result size', function(done) {
-    common.runInTransaction(agent, function(endRootSpan) {
-      connection.query('SELECT * FROM t', function(err, res) {
-        endRootSpan();
-        assert(!err);
-        var spans = common.getMatchingSpans(agent, function (span) {
-          return span.name === 'mysql-query';
-        });
-        var labels = spans[0].labels;
-        assert.equal(labels.result.length, RESULT_SIZE);
-        assert.equal(labels.result, '{0...');
-        done();
-      });
-    });
-  });
-
   it('should work with events', function(done) {
     common.runInTransaction(agent, function(endRootSpan) {
       var query = connection.query('SELECT * FROM t');

test/plugins/test-trace-redis.js

@@ -121,19 +121,6 @@ describe('redis', function() {
           });
         });
       });
-
-      it('should limit result size', function(done) {
-        common.runInTransaction(agent, function(endTransaction) {
-          client.get('key', function(err, res) {
-            endTransaction();
-            var trace = common.getMatchingSpan(agent, redisPredicate.bind(null, 'redis-get'));
-            var labels = trace.labels;
-            assert.equal(labels.result.length, RESULT_SIZE);
-            assert.equal(labels.result, 're...');
-            done();
-          });
-        });
-      });
     });
   });
 });

test/test-config-max-label-size.js

@@ -0,0 +1,37 @@
+/**
+ * Copyright 2017 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+'use strict';
+
+var assert = require('assert');
+var constants = require('../src/constants.js');
+var trace = require('..');
+
+var common = require('./plugins/common.js');
+
+describe('maximumLabelValueSize configuration', function() {
+  it('should not allow values above server maximum', function() {
+    var agent = trace.start({forceNewAgent_: true, maximumLabelValueSize: 1000000});
+    var valueMax = common.getConfig(agent).maximumLabelValueSize;
+    assert.strictEqual(valueMax, constants.TRACE_SERVICE_LABEL_VALUE_LIMIT);
+  });
+
+  it('should not modify values below server maximum', function() {
+    var agent = trace.start({forceNewAgent_: true, maximumLabelValueSize: 10});
+    var valueMax = common.getConfig(agent).maximumLabelValueSize;
+    assert.strictEqual(valueMax, 10);
+  });
+});