Removing default span attributes (open-telemetry#1342)

* refactor(opentelemetry-tracing): removing default span attributes Signed-off-by: Aravin Sivakumar <aravinarjunn@gmail.com> * refactor(opentelemetry-tracing): removing default span attributed from tracer object Signed-off-by: Aravin Sivakumar <aravinarjunn@gmail.com> * refactor(opentelemetry-tracing): removing accidental add to package.json Signed-off-by: Aravin Sivakumar <aravinarjunn@gmail.com> * refactor(opentelemetry-tracing): removing redundant test and fixing suggestions by Shawn and Daniel Signed-off-by: Aravin Sivakumar <aravinarjunn@gmail.com>
jonahrosenblum · Aug 6, 2020 · 6f4de1e · 6f4de1e
1 parent 44a7275
commit 6f4de1e
Show file tree

Hide file tree

Showing 5 changed files with 6 additions and 72 deletions.
diff --git a/packages/opentelemetry-node/test/NodeTracerProvider.test.ts b/packages/opentelemetry-node/test/NodeTracerProvider.test.ts
@@ -114,16 +114,6 @@ describe('NodeTracerProvider', () => {
       require('http');
       assert.strictEqual(plugins.length, 3);
     });
-
-    it('should construct an instance with default attributes', () => {
-      provider = new NodeTracerProvider({
-        defaultAttributes: {
-          region: 'eu-west',
-          asg: 'my-asg',
-        },
-      });
-      assert.ok(provider instanceof NodeTracerProvider);
-    });
   });
 
   describe('.startSpan()', () => {
@@ -195,19 +185,6 @@ describe('NodeTracerProvider', () => {
       assert.strictEqual(span.isRecording(), true);
     });
 
-    it('should set default attributes on span', () => {
-      const defaultAttributes = {
-        foo: 'bar',
-      };
-      provider = new NodeTracerProvider({
-        defaultAttributes,
-      });
-
-      const span = provider.getTracer('default').startSpan('my-span') as Span;
-      assert.ok(span instanceof Span);
-      assert.deepStrictEqual(span.attributes, defaultAttributes);
-    });
-
     it('should assign resource to span', () => {
       provider = new NodeTracerProvider({
         logger: new NoopLogger(),

diff --git a/packages/opentelemetry-tracing/src/Tracer.ts b/packages/opentelemetry-tracing/src/Tracer.ts
@@ -36,7 +36,6 @@ import { mergeConfig } from './utility';
  * This class represents a basic tracer.
  */
 export class Tracer implements api.Tracer {
-  private readonly _defaultAttributes: api.Attributes;
   private readonly _sampler: api.Sampler;
   private readonly _traceParams: TraceParams;
   readonly resource: Resource;
@@ -52,7 +51,6 @@ export class Tracer implements api.Tracer {
     private _tracerProvider: BasicTracerProvider
   ) {
     const localConfig = mergeConfig(config);
-    this._defaultAttributes = localConfig.defaultAttributes;
     this._sampler = localConfig.sampler;
     this._traceParams = localConfig.traceParams;
     this.resource = _tracerProvider.resource;
@@ -83,7 +81,7 @@ export class Tracer implements api.Tracer {
     }
     const spanKind = options.kind ?? api.SpanKind.INTERNAL;
     const links = options.links ?? [];
-    const attributes = { ...this._defaultAttributes, ...options.attributes };
+    const attributes = options.attributes ?? {};
     // make sampling decision
     const samplingResult = this._sampler.shouldSample(
       parentContext,

diff --git a/packages/opentelemetry-tracing/src/config.ts b/packages/opentelemetry-tracing/src/config.ts
@@ -30,7 +30,6 @@ export const DEFAULT_MAX_LINKS_PER_SPAN = 32;
  * used to extend the default value.
  */
 export const DEFAULT_CONFIG = {
-  defaultAttributes: {},
   logLevel: LogLevel.INFO,
   sampler: new AlwaysOnSampler(),
   traceParams: {

diff --git a/packages/opentelemetry-tracing/src/types.ts b/packages/opentelemetry-tracing/src/types.ts
@@ -14,12 +14,7 @@
  * limitations under the License.
  */
 
-import {
-  Attributes,
-  HttpTextPropagator,
-  Logger,
-  Sampler,
-} from '@opentelemetry/api';
+import { HttpTextPropagator, Logger, Sampler } from '@opentelemetry/api';
 import { LogLevel } from '@opentelemetry/core';
 import { ContextManager } from '@opentelemetry/context-base';
 import { Resource } from '@opentelemetry/resources';
@@ -28,12 +23,6 @@ import { Resource } from '@opentelemetry/resources';
  * TracerConfig provides an interface for configuring a Basic Tracer.
  */
 export interface TracerConfig {
-  /**
-   * Attributes that will be applied on every span created by Tracer.
-   * Useful to add infrastructure and environment information to your spans.
-   */
-  defaultAttributes?: Attributes;
-
   /**
    * User provided logger.
    */

diff --git a/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts b/packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts
@@ -119,13 +119,8 @@ describe('BasicTracerProvider', () => {
       });
     });
 
-    it('should construct an instance with default attributes', () => {
-      const tracer = new BasicTracerProvider({
-        defaultAttributes: {
-          region: 'eu-west',
-          asg: 'my-asg',
-        },
-      });
+    it('should construct an instance of BasicTracerProvider', () => {
+      const tracer = new BasicTracerProvider();
       assert.ok(tracer instanceof BasicTracerProvider);
     });
   });
@@ -159,26 +154,15 @@ describe('BasicTracerProvider', () => {
       span.end();
     });
 
-    it('should start a span with defaultAttributes and spanoptions->attributes', () => {
-      const tracer = new BasicTracerProvider({
-        defaultAttributes: { foo: 'bar' },
-      }).getTracer('default');
+    it('should start a span with given attributes', () => {
+      const tracer = new BasicTracerProvider().getTracer('default');
       const span = tracer.startSpan('my-span', {
         attributes: { foo: 'foo', bar: 'bar' },
       }) as Span;
       assert.deepStrictEqual(span.attributes, { bar: 'bar', foo: 'foo' });
       span.end();
     });
 
-    it('should start a span with defaultAttributes and undefined spanoptions->attributes', () => {
-      const tracer = new BasicTracerProvider({
-        defaultAttributes: { foo: 'bar' },
-      }).getTracer('default');
-      const span = tracer.startSpan('my-span', {}) as Span;
-      assert.deepStrictEqual(span.attributes, { foo: 'bar' });
-      span.end();
-    });
-
     it('should start a span with spanoptions->attributes', () => {
       const tracer = new BasicTracerProvider().getTracer('default');
       const span = tracer.startSpan('my-span', {
@@ -334,19 +318,6 @@ describe('BasicTracerProvider', () => {
       assert.strictEqual(span.isRecording(), true);
     });
 
-    it('should set default attributes on span', () => {
-      const defaultAttributes = {
-        foo: 'bar',
-      };
-      const tracer = new BasicTracerProvider({
-        defaultAttributes,
-      }).getTracer('default');
-
-      const span = tracer.startSpan('my-span') as Span;
-      assert.ok(span instanceof Span);
-      assert.deepStrictEqual(span.attributes, defaultAttributes);
-    });
-
     it('should assign a resource', () => {
       const tracer = new BasicTracerProvider().getTracer('default');
       const span = tracer.startSpan('my-span') as Span;