From d714733afd9c5b787643ed7599781552c15f6473 Mon Sep 17 00:00:00 2001 From: Vignesh Shanmugam Date: Thu, 23 May 2019 19:27:25 +0200 Subject: [PATCH 1/3] feat(rum): deprecate addTags in favor of addLabels --- docs/agent-api.asciidoc | 16 +++++++++---- .../rum-core/src/common/config-service.js | 12 +++++----- packages/rum-core/src/common/utils.js | 6 ++--- packages/rum-core/src/opentracing/span.js | 2 +- .../src/performance-monitoring/span-base.js | 14 +++++------ .../test/common/config-service.spec.js | 10 ++++---- packages/rum-core/test/common/utils.spec.js | 24 +++++++++---------- .../test/opentracing/opentracing.spec.js | 4 ++-- .../performance-monitoring/span-base.spec.js | 8 ++++--- packages/rum/src/apm-base.js | 8 ++++++- packages/rum/test/e2e/general-usecase/app.js | 2 +- packages/rum/test/specs/apm-base.spec.js | 1 + packages/rum/test/specs/index.spec.js | 2 +- 13 files changed, 61 insertions(+), 48 deletions(-) diff --git a/docs/agent-api.asciidoc b/docs/agent-api.asciidoc index 7f2b5be38..00a41ebbf 100644 --- 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, Please use <> 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 <>). 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/packages/rum-core/src/common/config-service.js b/packages/rum-core/src/common/config-service.js index eca40ce4e..ccc86e231 100644 --- 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 index 4f18101dd..5fb490488 100644 --- 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 index a6c4af90d..4b3f58ed4 100644 --- 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 index 575c36a8f..25f1d5f07 100644 --- 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 @@ -48,16 +48,14 @@ class SpanBase { } } - addTags(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 index 31e5d4388..bd654737d 100644 --- 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 index 54704cb03..565a81dba 100644 --- 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 index ea34be50a..ea86dcf14 100644 --- 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 index 9d3b95e44..0eaaff1f9 100644 --- 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 index 590752a48..54a05d13a 100644 --- 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 index d1eccea80..07d6ec1a1 100644 --- 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/apm-base.spec.js b/packages/rum/test/specs/apm-base.spec.js index 6842aa95e..b0f27c8db 100644 --- a/packages/rum/test/specs/apm-base.spec.js +++ b/packages/rum/test/specs/apm-base.spec.js @@ -26,6 +26,7 @@ import ApmBase from '../../src/apm-base' import { createServiceFactory } from '@elastic/apm-rum-core' import bootstrap from '../../src/bootstrap' +import { apm } from '../../src' var enabled = bootstrap() diff --git a/packages/rum/test/specs/index.spec.js b/packages/rum/test/specs/index.spec.js index bd6cc025d..d4d47c2c9 100644 --- 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') From ac640807cb08942d83fba90eb27a6f3ca29aec14 Mon Sep 17 00:00:00 2001 From: Vignesh Shanmugam Date: Fri, 24 May 2019 10:09:45 +0200 Subject: [PATCH 2/3] add review comments and fix lint --- docs/agent-api.asciidoc | 2 +- packages/rum/test/specs/apm-base.spec.js | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/agent-api.asciidoc b/docs/agent-api.asciidoc index 00a41ebbf..e879f1fdc 100644 --- a/docs/agent-api.asciidoc +++ b/docs/agent-api.asciidoc @@ -68,7 +68,7 @@ TIP: Before using custom context, ensure you understand the different types of [[apm-add-tags]] ==== `apm.addTags()` -deprecated, Please use <> instead +deprecated[4.1.x,Tags have been replaced with labels], Please use <> instead [float] [[apm-add-labels]] diff --git a/packages/rum/test/specs/apm-base.spec.js b/packages/rum/test/specs/apm-base.spec.js index b0f27c8db..6842aa95e 100644 --- a/packages/rum/test/specs/apm-base.spec.js +++ b/packages/rum/test/specs/apm-base.spec.js @@ -26,7 +26,6 @@ import ApmBase from '../../src/apm-base' import { createServiceFactory } from '@elastic/apm-rum-core' import bootstrap from '../../src/bootstrap' -import { apm } from '../../src' var enabled = bootstrap() From 4f60ba586d7351f4baf16ca9b2343fe240291d78 Mon Sep 17 00:00:00 2001 From: Vignesh Shanmugam Date: Tue, 28 May 2019 16:41:56 +0200 Subject: [PATCH 3/3] depreacte tags on span-base --- docs/span-api.asciidoc | 13 ++++++++++--- docs/transaction-api.asciidoc | 17 +++++++++++------ .../src/performance-monitoring/span-base.js | 5 +++++ 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/docs/span-api.asciidoc b/docs/span-api.asciidoc index d1bfb7fad..3d507a92c 100644 --- 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 <> 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 index 00399f7b9..f798ffb59 100644 --- a/docs/transaction-api.asciidoc +++ b/docs/transaction-api.asciidoc @@ -77,18 +77,23 @@ See <> 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 <> 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 <>). +Labels are key/value pairs that are indexed by Elasticsearch and therefore searchable (as opposed to data set via <>). -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/performance-monitoring/span-base.js b/packages/rum-core/src/performance-monitoring/span-base.js index 25f1d5f07..e2ee791e8 100644 --- a/packages/rum-core/src/performance-monitoring/span-base.js +++ b/packages/rum-core/src/performance-monitoring/span-base.js @@ -48,6 +48,11 @@ class SpanBase { } } + addTags(tags) { + console.warn('addTags deprecated, please use addLabels') + this.addLabels(tags) + } + addLabels(labels) { this.ensureContext() var ctx = this.context