ignore labels unless it is an object (#352)

README.md

@@ -161,6 +161,8 @@ Once your work is complete, you can end a child span with `agent.endSpan`. You c
   agent.endSpan(span, {label2: 'value'});
 ```
 
+Note that the labels parameter must be an object. Other types (e.g. string) will be silently ignored.
+
 ### Run in span
 
 `agent.runInSpan` takes a function to execute inside a custom child span with the given name. The function may be synchronous or asynchronous. If it is asynchronous, it must accept a 'endSpan' function as an argument that should be called once the asynchronous work has completed.

package.json

@@ -40,6 +40,7 @@
     "@google-cloud/common": "^0.11.0",
     "continuation-local-storage": "^3.1.4",
     "gcp-metadata": "^0.1.0",
+    "is": "^3.2.0",
     "lodash.findindex": "^4.4.0",
     "lodash.isequal": "^4.0.0",
     "methods": "^1.1.1",

src/span-data.js

@@ -16,6 +16,7 @@
 
 'use strict';
 
+var is = require('is');
 var TraceSpan = require('./trace-span.js');
 var TraceLabels = require('./trace-labels.js');
 
@@ -94,6 +95,21 @@ SpanData.prototype.addLabel = function(key, value) {
   this.span.setLabel(key, value);
 };
 
+/**
+ * Add properties from provided `labels` param to the span as labels.
+ * 
+ * @param {Object<string, string}>=} labels Labels to be attached to the newly 
+ *   created span. Non-object data types are silently ignored.
+ */
+SpanData.prototype.addLabels = function(labels) {
+  var that = this;
+  if (is.object(labels)) {
+    Object.keys(labels).forEach(function(key) {
+      that.addLabel(key, labels[key]);
+    });
+  }
+};
+
 /**
  * Closes the span and queues it for publishing if it is a root.
  */
@@ -143,6 +159,7 @@ function StackFrame(className, methodName, fileName, lineNumber, columnNumber) {
 SpanData.nullSpan = {
   createChildSpanData: function() { return SpanData.nullSpan; },
   addLabel: function() {},
+  addLabels: function() {},
   close: function() {}
 };
 

src/trace-agent.js

@@ -85,7 +85,7 @@ TraceAgent.prototype.config = function() {
  * Begin a new custom span.
  * @param {string} name The name of the span.
  * @param {Object<string, string}>=} labels Labels to be attached
- *   to the newly created span.
+ *   to the newly created span. Non-object data types are silently ignored.
  * @param {number=} skipFrames The number of caller frames to eliminate from
  *                            stack traces.
  * @return {SpanData} The newly created span.
@@ -95,11 +95,7 @@ TraceAgent.prototype.startSpan = function(name, labels, skipFrames) {
   skipFrames = skipFrames || 0;
   if (rootSpan) {
     var newSpan = rootSpan.createChildSpanData(name, skipFrames + 1);
-    if (labels) {
-      Object.keys(labels).forEach(function(key) {
-        newSpan.addLabel(key, labels[key]);
-      });
-    }
+    newSpan.addLabels(labels);
     return newSpan;
   } else {
     this.logger.error
@@ -112,14 +108,10 @@ TraceAgent.prototype.startSpan = function(name, labels, skipFrames) {
  * Close the provided span.
  * @param {SpanData} spanData The span to be ended.
  * @param {Object<string, string}>=} labels Labels to be attached
- *   to the terminated span.
+ *   to the terminated span. Non-object data types are silently ignored.
  */
 TraceAgent.prototype.endSpan = function(spanData, labels) {
-  if (labels) {
-    Object.keys(labels).forEach(function(key) {
-      spanData.addLabel(key, labels[key]);
-    });
-  }
+  spanData.addLabels(labels);
   spanData.close();
 };
 
@@ -130,7 +122,7 @@ TraceAgent.prototype.endSpan = function(spanData, labels) {
  * work is completed.
  * @param {string} name The name of the resulting span.
  * @param {Object<string, string}>=} labels Labels to be attached
- *   to the resulting span.
+ *   to the resulting span. Non-object data types are silently ignored.
  * @param {function(function()=)} fn The function to trace.
  */
 TraceAgent.prototype.runInSpan = function(name, labels, fn) {
@@ -169,11 +161,7 @@ TraceAgent.prototype.runInRootSpan = function(name, labels, fn) {
       return;
     }
     var span = self.createRootSpanData(name, null, null, 3, 'SPAN_KIND_UNSPECIFIED');
-    if (labels) {
-      Object.keys(labels).forEach(function(key) {
-        span.addLabel(key, labels[key]);
-      });
-    }
+    span.addLabels(labels);
     if (fn.length === 0) {
       fn();
       span.close();

test/standalone/test-index.js

@@ -24,6 +24,7 @@ if (!process.env.GCLOUD_PROJECT) {
 var assert = require('assert');
 var agent = require('../..');
 var cls = require('../../src/cls.js');
+var TraceLabels = require('../../src/trace-labels.js');
 
 describe('index.js', function() {
 
@@ -164,6 +165,51 @@ describe('index.js', function() {
     });
   });
 
+  describe('labels', function(){
+    it('should add labels to spans', function() {
+      agent.start();
+      cls.getNamespace().run(function() {
+        agent.private_().createRootSpanData('root', 1, 2);
+        var spanData = agent.startSpan('sub', {test1: 'value'});
+        agent.endSpan(spanData);
+        var traceSpan = spanData.span;
+        assert.equal(traceSpan.name, 'sub');
+        assert.ok(traceSpan.labels);
+        assert.equal(traceSpan.labels.test1, 'value');
+      });
+    });
+
+    it('should ignore non-object labels', function() {
+      agent.start();
+      cls.getNamespace().run(function() {
+        agent.private_().createRootSpanData('root', 1, 2);
+
+        var testLabels = [
+          'foo',
+          5,
+          undefined,
+          null,
+          true,
+          false,
+          [4,5,6],
+          function () {}
+        ];
+
+        testLabels.forEach(function(labels) {
+          var spanData = agent.startSpan('sub', labels);
+          agent.endSpan(spanData);
+          var spanLabels = spanData.span.labels;
+          // Only the default labels should be there.
+          var keys = Object.keys(spanLabels);
+          assert.equal(keys.length, 1, 'should have only 1 key');
+          assert.equal(keys[0], TraceLabels.STACK_TRACE_DETAILS_KEY);
+        });
+      });
+    });
+
+  });
+
+
   it('should produce real spans runInSpan sync', function() {
     agent.start();
     cls.getNamespace().run(function() {