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

fix(sdk-node): Allow Defining Sampler with Exporter in Env #4394

Merged
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
3800ac8
fix(sdk-node): Allow tracerProvider to be created when exporter is de…
JacksonWeber Dec 29, 2023
971b5c9
fix(node-sdk): Update to not accept when exporter is set to none.
JacksonWeber Dec 29, 2023
26c83e3
fix(sdk-node): Update Changelog.
JacksonWeber Dec 29, 2023
2f998f5
fix(sdk-node): Fix Changelog.
JacksonWeber Dec 29, 2023
8e3f266
fix(sdk-node): cleanup changelog.
JacksonWeber Dec 29, 2023
306e08d
fix(sdk-node): lint fix
JacksonWeber Dec 29, 2023
7eca942
fix(sdk-node): Fix logic for creating tracerProviders.
JacksonWeber Dec 29, 2023
0d5545b
Merge branch 'main' into jacksonweber/sampler-without-exporter
JacksonWeber Jan 3, 2024
02574b3
Fix lint.
JacksonWeber Jan 3, 2024
6302111
Update experimental/CHANGELOG.md
JacksonWeber Jan 10, 2024
8414831
Merge branch 'main' into jacksonweber/sampler-without-exporter
pichlermarc Jan 12, 2024
f0df5c6
Merge branch 'main' into jacksonweber/sampler-without-exporter
JacksonWeber Jan 15, 2024
a7bc759
Fix manual sampler and environment exporter case.
JacksonWeber Jan 15, 2024
f62d05f
Update logic to check for a defined traceExporter on the config befor…
JacksonWeber Jan 16, 2024
996172f
Fix equality check.
JacksonWeber Jan 16, 2024
685dd6c
Update env exporter configuration logic and add tests.
JacksonWeber Jan 16, 2024
2c3e88a
Update experimental/CHANGELOG.md
JacksonWeber Jan 16, 2024
6c1f226
Fix changelog issues.
JacksonWeber Jan 16, 2024
a0f2dd9
Merge branch 'jacksonweber/sampler-without-exporter' of https://githu…
JacksonWeber Jan 16, 2024
392a8ce
Merge branch 'main' into jacksonweber/sampler-without-exporter
JacksonWeber Jan 16, 2024
ad5ec30
Clean up tracerProvider logic.
JacksonWeber Jan 16, 2024
fad032a
Merge branch 'main' into jacksonweber/sampler-without-exporter
JacksonWeber Jan 17, 2024
da94a6e
Merge branch 'main' into jacksonweber/sampler-without-exporter
JacksonWeber Jan 18, 2024
88aa3a0
Update sdk.ts
JacksonWeber Jan 18, 2024
1d01797
Update sdk.ts
JacksonWeber Jan 18, 2024
e0bd102
Merge branch 'main' into jacksonweber/sampler-without-exporter
JacksonWeber Jan 24, 2024
e4eb1a1
Merge branch 'main' into jacksonweber/sampler-without-exporter
JacksonWeber Jan 29, 2024
35b889e
Update experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts
JacksonWeber Jan 30, 2024
5c65cc0
Update experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts
JacksonWeber Jan 30, 2024
a639e1c
Update experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts
JacksonWeber Jan 30, 2024
7bd5316
Merge branch 'main' into jacksonweber/sampler-without-exporter
pichlermarc Jan 31, 2024
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
1 change: 1 addition & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ All notable changes to experimental packages in this project will be documented

### :bug: (Bug Fix)

* fix(sdk-node): allow using samplers when the exporter is defined in the environment [#4394](https://github.com/open-telemetry/opentelemetry-js/pull/4394) @JacksonWeber
* fix(instrumentation): use caret range on import-in-the-middle [#4380](https://github.com/open-telemetry/opentelemetry-js/pull/4380) @pichlermarc
* fix(instrumentation): do not import 'path' in browser runtimes [#4378](https://github.com/open-telemetry/opentelemetry-js/pull/4378) @pichlermarc
* Fixes a bug where bundling for web would fail due to `InstrumentationNodeModuleDefinition` importing `path`
Expand Down
12 changes: 11 additions & 1 deletion experimental/packages/opentelemetry-sdk-node/src/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@ export class NodeSDK {
public constructor(configuration: Partial<NodeSDKConfiguration> = {}) {
const env = getEnv();
const envWithoutDefaults = getEnvWithoutDefaults();
const envExporterWithConfiguration =
env.OTEL_TRACES_EXPORTER &&
(configuration.sampler ||
configuration.spanLimits ||
configuration.idGenerator);

if (env.OTEL_SDK_DISABLED) {
this._disabled = true;
Expand All @@ -126,7 +131,12 @@ export class NodeSDK {

this._autoDetectResources = configuration.autoDetectResources ?? true;

if (configuration.spanProcessor || configuration.traceExporter) {
// If there are configuration options to read, we need to create a new TracerProvider even if the exporter is configured in the environment
if (
configuration.traceExporter ||
configuration.spanProcessor ||
envExporterWithConfiguration
JacksonWeber marked this conversation as resolved.
Show resolved Hide resolved
) {
const tracerProviderConfig: NodeTracerConfig = {};

if (configuration.sampler) {
Expand Down
23 changes: 23 additions & 0 deletions experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,29 @@ describe('Node SDK', () => {
assert.ok(apiTracerProvider.getDelegate() instanceof NodeTracerProvider);
});

it('should register a tracer provider if an exporter is provided via env', async () => {
env.OTEL_TRACES_EXPORTER = 'console';
const sdk = new NodeSDK({
autoDetectResources: false,
});

sdk.start();

assert.ok(!(metrics.getMeterProvider() instanceof MeterProvider));

assert.ok(
context['_getContextManager']().constructor.name ===
DefaultContextManager.name
);
assert.ok(
propagation['_getGlobalPropagator']() instanceof CompositePropagator
);
const apiTracerProvider =
trace.getTracerProvider() as ProxyTracerProvider;
assert.ok(apiTracerProvider.getDelegate() instanceof NodeTracerProvider);
delete env.OTEL_TRACES_EXPORTER;
});

it('should register a tracer provider if a span processor is provided', async () => {
const exporter = new ConsoleSpanExporter();
const spanProcessor = new SimpleSpanProcessor(exporter);
Expand Down