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.

Sign up for GitHub

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

feat(sdk-trace-*):! drop unintentional/unnecessary exports #5405

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

feat(sdk-trace-*):! drop unintentional/unnecessary exports #5405

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a957215
feat(sdk-trace-*):! drop unintentional exports
pichlermarc Jan 30, 2025
68f0ab0
Merge branch 'main' into feat/sdk-trace-base-exports
pichlermarc Jan 30, 2025
9214adc
Update CHANGELOG.md
pichlermarc Jan 30, 2025
e2bb994
fixup! Update CHANGELOG.md
pichlermarc Jan 30, 2025
53192bf
Merge branch 'main' into feat/sdk-trace-base-exports
pichlermarc Jan 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,13 @@ For semantic convention package changes, see the [semconv CHANGELOG](packages/se
* (user-facing): only a non-env-var based default is now used on `WebTracerProvider#register()`.
* propagators can now not be configured via `window.OTEL_PROPAGATORS` anymore, please pass the propagator to `WebTracerProvider#register()` instead.
* if not configured via code, `WebTracerProvider#register()` will now fall back to defaults (`tracecontext` and `baggage`)
* feat(sdk-trace-*)!: drop unnecessary exports [#????](https://github.com/open-telemetry/opentelemetry-js/pull/????) @pichlermarc
pichlermarc marked this conversation as resolved.
Show resolved Hide resolved
* (user-facing): `EXPORTER_FACTORY` is not used anymore and has been removed
* (user-facing): `PROPAGATOR_FACTORY` is not used anymore and has been removed
* (user-facing): `ForceFlushState` was intended for internal use and has been removed
* (user-facing): the `Tracer` class was unintentionally exported and has been removed
* to obtain a `Tracer`, please use `BasicTracerProvider#getTracer()`, `NodeTracerProvider#getTracer()` or `WebTracerProvider#getTracer()`
* to reference a `Tracer`, please use the `Tracer` type from `@opentelemetry/api`

### :rocket: (Enhancement)

Expand Down
17 changes: 12 additions & 5 deletions experimental/packages/shim-opencensus/test/ShimTracer.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
* limitations under the License.
*/

import { Tracer as Tracer } from '@opentelemetry/sdk-trace-base';
import * as assert from 'assert';
import * as sinon from 'sinon';
import * as oc from '@opencensus/core';
Expand All @@ -25,26 +24,34 @@ import {
SpanKind,
context,
createContextKey,
Tracer,
} from '@opentelemetry/api';
import { withTestTracer, setupNodeContextManager } from './util';

function createStubTracer(): Tracer {
return {
startSpan: sinon.stub(),
startActiveSpan: sinon.stub(),
} as unknown as Tracer;
}

describe('ShimTracer', () => {
setupNodeContextManager(before, after);

it('should initially be inactive', () => {
const shimTracer = new ShimTracer(sinon.createStubInstance(Tracer));
const shimTracer = new ShimTracer(createStubTracer());
assert.ok(!shimTracer.active);
});
describe('start', () => {
it('should set the tracer as active', () => {
const shimTracer = new ShimTracer(sinon.createStubInstance(Tracer));
const shimTracer = new ShimTracer(createStubTracer());
shimTracer.start({});
assert.ok(shimTracer.active);
});
});
describe('stop', () => {
it('should set the tracer as inactive', () => {
const shimTracer = new ShimTracer(sinon.createStubInstance(Tracer));
const shimTracer = new ShimTracer(createStubTracer());
shimTracer.start({});
assert.ok(shimTracer.active);
});
Expand Down Expand Up @@ -170,7 +177,7 @@ describe('ShimTracer', () => {

describe('wrap', () => {
it('should bind the provided function to active context at time of wrapping', () => {
const shimTracer = new ShimTracer(sinon.createStubInstance(Tracer));
const shimTracer = new ShimTracer(createStubTracer());
const key = createContextKey('key');
const fnToWrap = () =>
assert.strictEqual(context.active().getValue(key), 'value');
Expand Down
7 changes: 2 additions & 5 deletions packages/opentelemetry-sdk-trace-base/src/BasicTracerProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
TextMapPropagator,
trace,
TracerProvider,
Tracer as ApiTracer,
} from '@opentelemetry/api';
import {
CompositePropagator,
Expand All @@ -33,12 +34,8 @@ import { Tracer } from './Tracer';
import { loadDefaultConfig } from './config';
import { MultiSpanProcessor } from './MultiSpanProcessor';
import { SDKRegistrationConfig, TracerConfig } from './types';
import { SpanExporter } from './export/SpanExporter';
import { reconfigureLimits } from './utility';

export type PROPAGATOR_FACTORY = () => TextMapPropagator;
export type EXPORTER_FACTORY = () => SpanExporter;

export enum ForceFlushState {
'resolved',
'timeout',
Expand Down Expand Up @@ -84,7 +81,7 @@ export class BasicTracerProvider implements TracerProvider {
name: string,
version?: string,
options?: { schemaUrl?: string }
): Tracer {
): ApiTracer {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: if there's time, I'll also follow-up with renaming the now internal tracer to SdkTracer to avoid having to do this and to have the code be more readable.

I've opted to not do this in this PR to keep the diff somewhat contained.

const key = `${name}@${version || ''}:${options?.schemaUrl || ''}`;
if (!this._tracers.has(key)) {
this._tracers.set(
Expand Down
8 changes: 1 addition & 7 deletions packages/opentelemetry-sdk-trace-base/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,7 @@
* limitations under the License.
*/

export { Tracer } from './Tracer';
export {
BasicTracerProvider,
EXPORTER_FACTORY,
ForceFlushState,
PROPAGATOR_FACTORY,
} from './BasicTracerProvider';
export { BasicTracerProvider } from './BasicTracerProvider';
export { BatchSpanProcessor, RandomIdGenerator } from './platform';
export { ConsoleSpanExporter } from './export/ConsoleSpanExporter';
export { InMemorySpanExporter } from './export/InMemorySpanExporter';
Expand Down
71 changes: 48 additions & 23 deletions packages/opentelemetry-sdk-trace-base/test/common/BasicTracerProvider.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import {
} from '../../src';
import { SpanImpl } from '../../src/Span';
import { MultiSpanProcessor } from '../../src/MultiSpanProcessor';
import { Tracer } from '../../src/Tracer';

class DummyPropagator implements TextMapPropagator {
inject(context: Context, carrier: any, setter: TextMapSetter<any>): void {
Expand Down Expand Up @@ -137,7 +138,9 @@ describe('BasicTracerProvider', () => {
describe('generalLimits', () => {
describe('when not defined default values', () => {
it('should have tracer with default values', () => {
const tracer = new BasicTracerProvider({}).getTracer('default');
const tracer = new BasicTracerProvider({}).getTracer(
Copy link
Member Author

@pichlermarc pichlermarc Jan 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note for reviewers: there are quite a few of these as we assert on internals.

'default'
) as Tracer;
assert.deepStrictEqual(tracer.getGeneralLimits(), {
attributeValueLengthLimit: Infinity,
attributeCountLimit: 128,
Expand All @@ -151,7 +154,7 @@ describe('BasicTracerProvider', () => {
generalLimits: {
attributeCountLimit: 100,
},
}).getTracer('default');
}).getTracer('default') as Tracer;
const generalLimits = tracer.getGeneralLimits();
assert.strictEqual(generalLimits.attributeCountLimit, 100);
});
Expand All @@ -163,7 +166,7 @@ describe('BasicTracerProvider', () => {
generalLimits: {
attributeValueLengthLimit: 10,
},
}).getTracer('default');
}).getTracer('default') as Tracer;
const generalLimits = tracer.getGeneralLimits();
assert.strictEqual(generalLimits.attributeValueLengthLimit, 10);
});
Expand All @@ -173,7 +176,7 @@ describe('BasicTracerProvider', () => {
generalLimits: {
attributeValueLengthLimit: -10,
},
}).getTracer('default');
}).getTracer('default') as Tracer;
const generalLimits = tracer.getGeneralLimits();
assert.strictEqual(generalLimits.attributeValueLengthLimit, -10);
});
Expand All @@ -183,7 +186,9 @@ describe('BasicTracerProvider', () => {
describe('spanLimits', () => {
describe('when not defined default values', () => {
it('should have tracer with default values', () => {
const tracer = new BasicTracerProvider({}).getTracer('default');
const tracer = new BasicTracerProvider({}).getTracer(
'default'
) as Tracer;
assert.deepStrictEqual(tracer.getSpanLimits(), {
attributeValueLengthLimit: Infinity,
attributeCountLimit: 128,
Expand All @@ -201,7 +206,7 @@ describe('BasicTracerProvider', () => {
spanLimits: {
attributeCountLimit: 100,
},
}).getTracer('default');
}).getTracer('default') as Tracer;
const spanLimits = tracer.getSpanLimits();
assert.strictEqual(spanLimits.attributeCountLimit, 100);
});
Expand All @@ -213,7 +218,7 @@ describe('BasicTracerProvider', () => {
spanLimits: {
attributeValueLengthLimit: 10,
},
}).getTracer('default');
}).getTracer('default') as Tracer;
const spanLimits = tracer.getSpanLimits();
assert.strictEqual(spanLimits.attributeValueLengthLimit, 10);
});
Expand All @@ -223,7 +228,7 @@ describe('BasicTracerProvider', () => {
spanLimits: {
attributeValueLengthLimit: -10,
},
}).getTracer('default');
}).getTracer('default') as Tracer;
const spanLimits = tracer.getSpanLimits();
assert.strictEqual(spanLimits.attributeValueLengthLimit, -10);
});
Expand All @@ -232,14 +237,18 @@ describe('BasicTracerProvider', () => {
describe('when attribute value length limit is defined via env', () => {
it('should have general attribute value length limits value as defined with env', () => {
envSource.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT = '115';
const tracer = new BasicTracerProvider().getTracer('default');
const tracer = new BasicTracerProvider().getTracer(
'default'
) as Tracer;
const generalLimits = tracer.getGeneralLimits();
assert.strictEqual(generalLimits.attributeValueLengthLimit, 115);
delete envSource.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT;
});
it('should have span attribute value length limit value same as general limit value', () => {
envSource.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT = '125';
const tracer = new BasicTracerProvider().getTracer('default');
const tracer = new BasicTracerProvider().getTracer(
'default'
) as Tracer;
const generalLimits = tracer.getGeneralLimits();
const spanLimits = tracer.getSpanLimits();
assert.strictEqual(generalLimits.attributeValueLengthLimit, 125);
Expand All @@ -249,7 +258,9 @@ describe('BasicTracerProvider', () => {
it('should have span and general attribute value length limits as defined in env', () => {
envSource.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT = '125';
envSource.OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT = '109';
const tracer = new BasicTracerProvider().getTracer('default');
const tracer = new BasicTracerProvider().getTracer(
'default'
) as Tracer;
const spanLimits = tracer.getSpanLimits();
const generalLimits = tracer.getGeneralLimits();
assert.strictEqual(generalLimits.attributeValueLengthLimit, 125);
Expand All @@ -260,7 +271,9 @@ describe('BasicTracerProvider', () => {
it('should have span attribute value length limit as default of Infinity', () => {
envSource.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT = '125';
envSource.OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT = 'Infinity';
const tracer = new BasicTracerProvider().getTracer('default');
const tracer = new BasicTracerProvider().getTracer(
'default'
) as Tracer;
const spanLimits = tracer.getSpanLimits();
const generalLimits = tracer.getGeneralLimits();
assert.strictEqual(generalLimits.attributeValueLengthLimit, 125);
Expand All @@ -272,7 +285,9 @@ describe('BasicTracerProvider', () => {

describe('when attribute value length limit is not defined via env', () => {
it('should use default value of Infinity', () => {
const tracer = new BasicTracerProvider().getTracer('default');
const tracer = new BasicTracerProvider().getTracer(
'default'
) as Tracer;
const spanLimits = tracer.getSpanLimits();
const generalLimits = tracer.getGeneralLimits();
assert.strictEqual(generalLimits.attributeValueLengthLimit, Infinity);
Expand All @@ -283,14 +298,18 @@ describe('BasicTracerProvider', () => {
describe('when attribute count limit is defined via env', () => {
it('should general attribute count limit as defined with env', () => {
envSource.OTEL_ATTRIBUTE_COUNT_LIMIT = '25';
const tracer = new BasicTracerProvider({}).getTracer('default');
const tracer = new BasicTracerProvider({}).getTracer(
'default'
) as Tracer;
const generalLimits = tracer.getGeneralLimits();
assert.strictEqual(generalLimits.attributeCountLimit, 25);
delete envSource.OTEL_ATTRIBUTE_COUNT_LIMIT;
});
it('should have span attribute count limit value same as general limit value', () => {
envSource.OTEL_ATTRIBUTE_COUNT_LIMIT = '20';
const tracer = new BasicTracerProvider().getTracer('default');
const tracer = new BasicTracerProvider().getTracer(
'default'
) as Tracer;
const generalLimits = tracer.getGeneralLimits();
const spanLimits = tracer.getSpanLimits();
assert.strictEqual(generalLimits.attributeCountLimit, 20);
Expand All @@ -300,7 +319,9 @@ describe('BasicTracerProvider', () => {
it('should have span and general attribute count limits as defined in env', () => {
envSource.OTEL_ATTRIBUTE_COUNT_LIMIT = '20';
envSource.OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT = '35';
const tracer = new BasicTracerProvider().getTracer('default');
const tracer = new BasicTracerProvider().getTracer(
'default'
) as Tracer;
const spanLimits = tracer.getSpanLimits();
const generalLimits = tracer.getGeneralLimits();
assert.strictEqual(generalLimits.attributeCountLimit, 20);
Expand All @@ -311,7 +332,9 @@ describe('BasicTracerProvider', () => {
it('should have span attribute count limit as default of 128', () => {
envSource.OTEL_ATTRIBUTE_COUNT_LIMIT = '20';
envSource.OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT = '128';
const tracer = new BasicTracerProvider().getTracer('default');
const tracer = new BasicTracerProvider().getTracer(
'default'
) as Tracer;
const spanLimits = tracer.getSpanLimits();
const generalLimits = tracer.getGeneralLimits();
assert.strictEqual(generalLimits.attributeCountLimit, 20);
Expand All @@ -323,7 +346,9 @@ describe('BasicTracerProvider', () => {

describe('when attribute count limit is not defined via env', () => {
it('should use default value of 128', () => {
const tracer = new BasicTracerProvider().getTracer('default');
const tracer = new BasicTracerProvider().getTracer(
'default'
) as Tracer;
const spanLimits = tracer.getSpanLimits();
const generalLimits = tracer.getGeneralLimits();
assert.strictEqual(generalLimits.attributeCountLimit, 128);
Expand All @@ -337,7 +362,7 @@ describe('BasicTracerProvider', () => {
spanLimits: {
eventCountLimit: 300,
},
}).getTracer('default');
}).getTracer('default') as Tracer;
const spanLimits = tracer.getSpanLimits();
assert.strictEqual(spanLimits.eventCountLimit, 300);
});
Expand All @@ -349,7 +374,7 @@ describe('BasicTracerProvider', () => {
spanLimits: {
linkCountLimit: 10,
},
}).getTracer('default');
}).getTracer('default') as Tracer;
const spanLimits = tracer.getSpanLimits();
assert.strictEqual(spanLimits.linkCountLimit, 10);
});
Expand All @@ -362,7 +387,7 @@ describe('BasicTracerProvider', () => {
attributeValueLengthLimit: 100,
attributeCountLimit: 200,
},
}).getTracer('default');
}).getTracer('default') as Tracer;
const spanLimits = tracer.getSpanLimits();
assert.strictEqual(spanLimits.attributeValueLengthLimit, 100);
assert.strictEqual(spanLimits.attributeCountLimit, 200);
Expand All @@ -380,7 +405,7 @@ describe('BasicTracerProvider', () => {
attributeValueLengthLimit: 10,
attributeCountLimit: 20,
},
}).getTracer('default');
}).getTracer('default') as Tracer;
const spanLimits = tracer.getSpanLimits();
assert.strictEqual(spanLimits.attributeValueLengthLimit, 10);
assert.strictEqual(spanLimits.attributeCountLimit, 20);
Expand Down Expand Up @@ -458,7 +483,7 @@ describe('BasicTracerProvider', () => {

it('should propagate resources', () => {
const tracerProvider = new BasicTracerProvider();
const tracer = tracerProvider.getTracer('default');
const tracer = tracerProvider.getTracer('default') as Tracer;
const span = tracer.startSpan('my-span') as Span;
assert.strictEqual(tracer['_resource'], tracerProvider['_resource']);
assert.strictEqual(span.resource, tracerProvider['_resource']);
Expand Down
Loading