feat(rum): deprecate addTags in favor of addLabels (#270)

* feat(rum): deprecate addTags in favor of addLabels * add review comments and fix lint * depreacte tags on span-base
elastic · Jun 4, 2019 · 3e313d3 · 3e313d3
1 parent f2b2562
commit 3e313d3
Show file tree

Hide file tree

Showing 14 changed files with 85 additions and 56 deletions.
diff --git a/docs/agent-api.asciidoc b/docs/agent-api.asciidoc
@@ -68,16 +68,22 @@ TIP: Before using custom context, ensure you understand the different types of
 [[apm-add-tags]]
 ==== `apm.addTags()`
 
+deprecated[4.1.x,Tags have been replaced with labels], Please use <<apm-add-labels,`apm.addLabels()`>> instead
+
+[float]
+[[apm-add-labels]]
+==== `apm.addLabels()`
+
 [source,js]
 ----
-apm.addTags({ [name]: value })
+apm.addLabels({ [name]: value })
 ----
 
-Set tags on transactions and errors.
+Set labels on transactions and errors.
 
-Tags are key/value pairs that are indexed by Elasticsearch and therefore searchable (as opposed to data set via `setCustomContext()`). You can set multiple tags.
+Labels are key/value pairs that are indexed by Elasticsearch and therefore searchable (as opposed to data set via <<apm-set-custom-context,`apm.setCustomContext()`>>). You can set multiple labels.
 
-TIP: Before using custom tags, ensure you understand the different types of
+TIP: Before using custom labels, ensure you understand the different types of
 {apm-overview-ref-v}/metadata.html[metadata] that are available.
 
 Arguments:
@@ -86,7 +92,7 @@ Arguments:
 
 * `value` - Any string. If a non-string data type is given, it's converted to a string before being sent to the APM Server.
 
-WARNING: Avoid defining too many user-specified tags.
+WARNING: Avoid defining too many user-specified labels.
 Defining too many unique fields in an index is a condition that can lead to a
 {ref}/mapping.html#mapping-limit-settings[mapping explosion].
 

diff --git a/docs/span-api.asciidoc b/docs/span-api.asciidoc
@@ -42,13 +42,20 @@ the following are standardized across all Elastic APM agents:
 [[span-add-tags]]
 ==== `span.addTags()`
 
+deprecated[4.1.x,Tags have been replaced with labels], Please use <<span-add-labels,`span.addLabels()`>> instead
+
+
+[float]
+[[span-add-labels]]
+==== `span.addLabels()`
+
 [source,js]
 ----
-span.addTags({ [name]: value })
+span.addLabels({ [name]: value })
 ----
 
-Add several tags on the span. If an error happens during the span,
-it will also get tagged with the same tags.
+Add several labels on the span. If an error happens during the span,
+it will also get tagged with the same labels.
 
 Arguments:
 

diff --git a/docs/transaction-api.asciidoc b/docs/transaction-api.asciidoc
@@ -77,18 +77,23 @@ See <<span-api,Span API>> docs for details on how to use custom spans.
 [[transaction-add-tags]]
 ==== `transaction.addTags()`
 
+deprecated[4.1.x,Tags have been replaced with labels], Please use <<transaction-add-labels,`transaction.addLabels()`>> instead
+
+[float]
+[[transaction-add-labels]]
+==== `transaction.addLabels()`
+
 [source,js]
 ----
-transaction.addTags({ [name]: value })
+transaction.addLabels({ [name]: value })
 ----
 
-Add several tags on the transaction.
-If an error happens during the transaction,
-it will also get tagged with the same tags.
+Add several labels on the transaction. If an error happens during the transaction,
+it will also get tagged with the same labels.
 
-Tags are key/value pairs that are indexed by Elasticsearch and therefore searchable (as opposed to data set via <<apm-set-custom-context,`apm.setCustomContext()`>>).
+Labels are key/value pairs that are indexed by Elasticsearch and therefore searchable (as opposed to data set via <<apm-set-custom-context,`apm.setCustomContext()`>>).
 
-TIP: Before using custom tags, ensure you understand the different types of
+TIP: Before using custom labels, ensure you understand the different types of
 {apm-overview-ref-v}/metadata.html[metadata] that are available.
 
 Arguments:

diff --git a/packages/rum-core/src/common/config-service.js b/packages/rum-core/src/common/config-service.js
@@ -23,7 +23,7 @@
  *
  */
 
-import { getCurrentScript, setTag, merge, getDtHeaderValue } from './utils'
+import { getCurrentScript, setLabel, merge, getDtHeaderValue } from './utils'
 import Subscription from '../common/subscription'
 
 function getConfigFromScript() {
@@ -200,12 +200,12 @@ class Config {
     }
   }
 
-  addTags(tags) {
-    if (!this.config.context.tags) {
-      this.config.context.tags = {}
+  addLabels(labels) {
+    if (!this.config.context.labels) {
+      this.config.context.labels = {}
     }
-    var keys = Object.keys(tags)
-    keys.forEach(k => setTag(k, tags[k], this.config.context.tags))
+    var keys = Object.keys(labels)
+    keys.forEach(k => setLabel(k, labels[k], this.config.context.labels))
   }
 
   setConfig(properties = {}) {

diff --git a/packages/rum-core/src/common/utils.js b/packages/rum-core/src/common/utils.js
@@ -135,14 +135,14 @@ function isPlatformSupported() {
 }
 
 /**
- * Convert values of the tag to be string to be compatible
+ * Convert values of the Tag/Label to be string to be compatible
  * with the apm server prior to <6.7 version
  *
  * TODO: Remove string conversion in the next major release since
  * support for boolean and number in the APM server has landed in 6.7
  * https://github.com/elastic/apm-server/pull/1712/
  */
-function setTag(key, value, obj) {
+function setLabel(key, value, obj) {
   if (!obj || !key) return
   var skey = removeInvalidChars(key)
   if (value) {
@@ -347,7 +347,7 @@ export {
   generateRandomId,
   rng,
   checkSameOrigin,
-  setTag,
+  setLabel,
   stripQueryStringFromUrl,
   find,
   removeInvalidChars

diff --git a/packages/rum-core/src/opentracing/span.js b/packages/rum-core/src/opentracing/span.js
@@ -77,7 +77,7 @@ class Span extends otSpan {
       }
     }
 
-    this.span.addTags(tags)
+    this.span.addLabels(tags)
   }
   // eslint-disable-next-line
   _log(log, timestamp) {

diff --git a/packages/rum-core/src/performance-monitoring/span-base.js b/packages/rum-core/src/performance-monitoring/span-base.js
@@ -23,7 +23,7 @@
  *
  */
 
-import { isUndefined, generateRandomId, setTag, merge } from '../common/utils'
+import { isUndefined, generateRandomId, setLabel, merge } from '../common/utils'
 
 class SpanBase {
   // context
@@ -49,15 +49,18 @@ class SpanBase {
   }
 
   addTags(tags) {
+    console.warn('addTags deprecated, please use addLabels')
+    this.addLabels(tags)
+  }
+
+  addLabels(labels) {
     this.ensureContext()
     var ctx = this.context
-    if (!ctx.tags) {
-      ctx.tags = {}
+    if (!ctx.labels) {
+      ctx.labels = {}
     }
-    var keys = Object.keys(tags)
-    keys.forEach(function(k) {
-      setTag(k, tags[k], ctx.tags)
-    })
+    var keys = Object.keys(labels)
+    keys.forEach(k => setLabel(k, labels[k], ctx.labels))
   }
 
   addContext(context) {

diff --git a/packages/rum-core/test/common/config-service.spec.js b/packages/rum-core/test/common/config-service.spec.js
@@ -143,18 +143,18 @@ describe('ConfigService', function() {
     expect(result).toBe(true)
   })
 
-  it('should addTags', function() {
+  it('should addLabels', function() {
     var date = new Date()
-    const tags = {
+    const labels = {
       test: 'test',
       no: 1,
       'test.test': 'test',
       obj: { just: 'object' },
       date
     }
-    configService.addTags(tags)
-    const contextTags = configService.get('context.tags')
-    expect(contextTags).toEqual({
+    configService.addLabels(labels)
+    const contextLabels = configService.get('context.labels')
+    expect(contextLabels).toEqual({
       test: 'test',
       no: '1',
       test_test: 'test',

diff --git a/packages/rum-core/test/common/utils.spec.js b/packages/rum-core/test/common/utils.spec.js
@@ -233,19 +233,19 @@ describe('lib/utils', function() {
     expect(result).toBeLessThanOrEqual(now)
   })
 
-  it('should setTag', function() {
+  it('should setLabel', function() {
     var date = new Date()
-    var tags = {}
-    utils.setTag('key', 'value', undefined)
-    utils.setTag(undefined, 'value', tags)
-    utils.setTag('test', 'test', tags)
-    utils.setTag('no', 1, tags)
-    utils.setTag('test.test', 'passed', tags)
-    utils.setTag('date', date, tags)
-    utils.setTag()
-    utils.setTag('removed', undefined, tags)
-    utils.setTag('obj', {}, tags)
-    expect(tags).toEqual({
+    var labels = {}
+    utils.setLabel('key', 'value', undefined)
+    utils.setLabel(undefined, 'value', labels)
+    utils.setLabel('test', 'test', labels)
+    utils.setLabel('no', 1, labels)
+    utils.setLabel('test.test', 'passed', labels)
+    utils.setLabel('date', date, labels)
+    utils.setLabel()
+    utils.setLabel('removed', undefined, labels)
+    utils.setLabel('obj', {}, labels)
+    expect(labels).toEqual({
       test: 'test',
       no: '1',
       test_test: 'passed',

diff --git a/packages/rum-core/test/opentracing/opentracing.spec.js b/packages/rum-core/test/opentracing/opentracing.spec.js
@@ -96,7 +96,7 @@ describe('OpenTracing API', function() {
         username: 'test-username',
         email: 'test-email'
       },
-      tags: { another_tag: 'test-tag' }
+      labels: { another_tag: 'test-tag' }
     })
 
     var testError = new Error('OpenTracing test error')
@@ -126,7 +126,7 @@ describe('OpenTracing API', function() {
 
     expect(childSpan.span.type).toBe('new-type')
     expect(childSpan.span.context).toEqual({
-      tags: {
+      labels: {
         another_tag: 'test-tag',
         user_id: 'test-id',
         user_username: 'test-username',

diff --git a/packages/rum-core/test/performance-monitoring/span-base.spec.js b/packages/rum-core/test/performance-monitoring/span-base.spec.js
@@ -26,10 +26,12 @@
 import SpanBase from '../../src/performance-monitoring/span-base'
 
 describe('SpanBase', function() {
-  it('should addTags', function() {
+  it('should addLabels', function() {
     var span = new SpanBase()
-    span.addTags({ test: 'passed', 'test.new': 'new' })
-    expect(span.context).toEqual({ tags: { test: 'passed', test_new: 'new' } })
+    span.addLabels({ test: 'passed', 'test.new': 'new' })
+    expect(span.context).toEqual({
+      labels: { test: 'passed', test_new: 'new' }
+    })
   })
 
   it('should addContext', function() {

diff --git a/packages/rum/src/apm-base.js b/packages/rum/src/apm-base.js
@@ -115,8 +115,14 @@ class ApmBase {
   }
 
   addTags(tags) {
+    const loggingService = this.serviceFactory.getService('LoggingService')
+    loggingService.warn('addTags deprecated, please use addLabels')
+    this.addLabels(tags)
+  }
+
+  addLabels(labels) {
     var configService = this.serviceFactory.getService('ConfigService')
-    configService.addTags(tags)
+    configService.addLabels(labels)
   }
 
   // Should call this method before 'load' event on window is fired

diff --git a/packages/rum/test/e2e/general-usecase/app.js b/packages/rum/test/e2e/general-usecase/app.js
@@ -54,7 +54,7 @@ elasticApm.setUserContext({
   email: 'email'
 })
 elasticApm.setCustomContext({ testContext: 'testContext' })
-elasticApm.addTags({ testTagKey: 'testTagValue' })
+elasticApm.addLabels({ testTagKey: 'testTagValue' })
 
 elasticApm.addFilter(function(payload) {
   if (payload.errors) {

diff --git a/packages/rum/test/specs/index.spec.js b/packages/rum/test/specs/index.spec.js
@@ -76,7 +76,7 @@ describe('index', function() {
       email: 'email'
     })
     apmBase.setCustomContext({ testContext: 'testContext' })
-    apmBase.addTags({ testTagKey: 'testTagValue' })
+    apmBase.addLabels({ testTagKey: 'testTagValue' })
 
     try {
       throw new Error('ApmBase test error')