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 27 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
2 changes: 2 additions & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ 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

### :books: (Refine Doc)

### :house: (Internal)
Expand Down
10 changes: 8 additions & 2 deletions experimental/packages/opentelemetry-sdk-node/src/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ export class NodeSDK {
private _loggerProvider?: LoggerProvider;
private _meterProvider?: MeterProvider;
private _serviceName?: string;
private _configuration?: Partial<NodeSDKConfiguration>;

private _disabled?: boolean;

Expand All @@ -116,6 +117,8 @@ export class NodeSDK {
});
}

this._configuration = configuration;

this._resource = configuration.resource ?? new Resource({});
this._resourceDetectors = configuration.resourceDetectors ?? [
envDetector,
Expand All @@ -126,7 +129,8 @@ export class NodeSDK {

this._autoDetectResources = configuration.autoDetectResources ?? true;

if (configuration.spanProcessor || configuration.traceExporter) {
// If a tracer provider can be created from manual configuration, create it
if (configuration.traceExporter || configuration.spanProcessor) {
const tracerProviderConfig: NodeTracerConfig = {};

if (configuration.sampler) {
Expand Down Expand Up @@ -316,12 +320,14 @@ export class NodeSDK {
})
);

// if there is a tracerProviderConfig (traceExporter/spanProcessor was set manually) or the traceExporter is set manually, use NodeTracerProvider
const Provider = this._tracerProviderConfig
? NodeTracerProvider
: TracerProviderWithEnvExporters;

// If the Provider is configured with Env Exporters, we need to check if the SDK had any manual configurations and set them here
const tracerProvider = new Provider({
...this._tracerProviderConfig?.tracerConfig,
...this._configuration,
resource: this._resource,
});

Expand Down
41 changes: 41 additions & 0 deletions experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import {
BatchSpanProcessor,
NoopSpanProcessor,
IdGenerator,
AlwaysOffSampler,
} from '@opentelemetry/sdk-trace-base';
import * as assert from 'assert';
import * as semver from 'semver';
Expand Down Expand Up @@ -176,6 +177,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 Expand Up @@ -873,6 +897,23 @@ describe('setup exporter from env', () => {
assert(listOfProcessors[0] instanceof BatchSpanProcessor);
delete env.OTEL_TRACES_EXPORTER;
});
it('should only create one span processor when configured using env vars and config', async () => {
env.OTEL_TRACES_EXPORTER = 'console';
const sdk = new NodeSDK({
sampler: new AlwaysOffSampler(),
});
sdk.start();
const listOfProcessors =
sdk['_tracerProvider']!['_registeredSpanProcessors']!;
assert(
sdk['_tracerProvider'] instanceof TracerProviderWithEnvExporters === true
);
JacksonWeber marked this conversation as resolved.
Show resolved Hide resolved
assert(
sdk['_tracerProvider']!['_config']?.sampler instanceof AlwaysOffSampler
);
JacksonWeber marked this conversation as resolved.
Show resolved Hide resolved
assert(listOfProcessors.length === 1);
JacksonWeber marked this conversation as resolved.
Show resolved Hide resolved
delete env.OTEL_TRACES_EXPORTER;
});
it('use otlp exporter and defined exporter protocol env value', async () => {
env.OTEL_TRACES_EXPORTER = 'otlp';
env.OTEL_EXPORTER_OTLP_TRACES_PROTOCOL = 'grpc';
Expand Down