Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removing default span attributes #1342

Merged
merged 8 commits into from
Jul 27, 2020
5 changes: 5 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,5 +62,10 @@
"hooks": {
"commit-msg": "commitlint -E HUSKY_GIT_PARAMS"
}
},
aravinsiva marked this conversation as resolved.
Show resolved Hide resolved
"dependencies": {
"@opentelemetry/context-async-hooks": "^0.9.0",
"@opentelemetry/context-base": "^0.9.0",
"@opentelemetry/semantic-conventions": "^0.9.0"
}
}
23 changes: 0 additions & 23 deletions packages/opentelemetry-node/test/NodeTracerProvider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()', () => {
Expand Down Expand Up @@ -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(),
Expand Down
4 changes: 1 addition & 3 deletions packages/opentelemetry-tracing/src/Tracer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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 };
aravinsiva marked this conversation as resolved.
Show resolved Hide resolved
// make sampling decision
const samplingResult = this._sampler.shouldSample(
parentContext,
Expand Down
1 change: 0 additions & 1 deletion packages/opentelemetry-tracing/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
13 changes: 1 addition & 12 deletions packages/opentelemetry-tracing/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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.
*/
Expand Down
29 changes: 3 additions & 26 deletions packages/opentelemetry-tracing/test/BasicTracerProvider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,7 @@ describe('BasicTracerProvider', () => {
});

it('should construct an instance with default attributes', () => {
aravinsiva marked this conversation as resolved.
Show resolved Hide resolved
const tracer = new BasicTracerProvider({
defaultAttributes: {
region: 'eu-west',
asg: 'my-asg',
},
});
const tracer = new BasicTracerProvider();
assert.ok(tracer instanceof BasicTracerProvider);
});
});
Expand Down Expand Up @@ -143,25 +138,14 @@ describe('BasicTracerProvider', () => {
});

it('should start a span with defaultAttributes and spanoptions->attributes', () => {
aravinsiva marked this conversation as resolved.
Show resolved Hide resolved
const tracer = new BasicTracerProvider({
defaultAttributes: { foo: 'bar' },
}).getTracer('default');
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', {
Expand Down Expand Up @@ -318,16 +302,9 @@ describe('BasicTracerProvider', () => {
});

it('should set default attributes on span', () => {
aravinsiva marked this conversation as resolved.
Show resolved Hide resolved
const defaultAttributes = {
foo: 'bar',
};
const tracer = new BasicTracerProvider({
defaultAttributes,
}).getTracer('default');

const tracer = new BasicTracerProvider().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', () => {
Expand Down