Skip to content

Commit

Permalink
feat(sdk-node): remove deprecated methods from NodeSDK (#4609)
Browse files Browse the repository at this point in the history
  • Loading branch information
@pichlermarc
pichlermarc committed Apr 10, 2024
1 parent 0b6463e commit 7fb673c
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 223 deletions.
7 changes: 7 additions & 0 deletions experimental/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,13 @@ All notable changes to experimental packages in this project will be documented

### :boom: Breaking Change

* feat(sdk-node)!: remove long deprecated methods in favor of constructor options [#4606](https://github.com/open-telemetry/opentelemetry-js/pull/4606) @pichlermarc
* `NodeSDK.configureTracerProvider()`, please use constructor options instead
* `NodeSDK.configureMeterProvider()`, please use constructor options instead
* `NodeSDK.configureLoggerProvider()`, please use constructor options instead
* `NodeSDK.addResource()`, please use constructor options instead
* `NodeSDK.detectResources()`, this is not necessary anymore, resources are now auto-detected on `NodeSDK.start()` if the constructor option `autoDetectResources` is unset, `undefined` or `true`.

### :rocket: (Enhancement)

* feat(otlp-transformer): consolidate scope/resource creation in transformer [#4600](https://github.com/open-telemetry/opentelemetry-js/pull/4600)
Expand Down
136 changes: 14 additions & 122 deletions experimental/packages/opentelemetry-sdk-node/src/sdk.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,19 +161,18 @@ export class NodeSDK {

const spanProcessors = configuration.spanProcessors ?? [spanProcessor];

this.configureTracerProvider(
tracerProviderConfig,
this._tracerProviderConfig = {
tracerConfig: tracerProviderConfig,
spanProcessors,
configuration.contextManager,
configuration.textMapPropagator
);
contextManager: configuration.contextManager,
textMapPropagator: configuration.textMapPropagator,
};
}

if (configuration.logRecordProcessor) {
const loggerProviderConfig: LoggerProviderConfig = {
this._loggerProviderConfig = {
logRecordProcessor: configuration.logRecordProcessor,
};
this.configureLoggerProvider(loggerProviderConfig);
}

if (configuration.metricReader || configuration.views) {
Expand All @@ -186,7 +185,7 @@ export class NodeSDK {
meterProviderConfig.views = configuration.views;
}

this.configureMeterProvider(meterProviderConfig);
this._meterProviderConfig = meterProviderConfig;
}

let instrumentations: InstrumentationOption[] = [];
Expand All @@ -196,119 +195,6 @@ export class NodeSDK {
this._instrumentations = instrumentations;
}

/**
*
* @deprecated Please pass {@code sampler}, {@code generalLimits}, {@code spanLimits}, {@code resource},
* {@code IdGenerator}, {@code spanProcessor}, {@code contextManager} and {@code textMapPropagator},
* to the constructor options instead.
*
* Set configurations needed to register a TracerProvider
*/
public configureTracerProvider(
tracerConfig: NodeTracerConfig,
spanProcessors: SpanProcessor[],
contextManager?: ContextManager,
textMapPropagator?: TextMapPropagator
): void {
this._tracerProviderConfig = {
tracerConfig,
spanProcessors,
contextManager,
textMapPropagator,
};
}

/**
* @deprecated Please pass {@code logRecordProcessor} to the constructor options instead.
*
* Set configurations needed to register a LoggerProvider
*/
public configureLoggerProvider(config: LoggerProviderConfig): void {
// nothing is set yet, we can set config and then return
if (this._loggerProviderConfig == null) {
this._loggerProviderConfig = config;
return;
}

// make sure we do not override existing logRecordProcessor with other logRecordProcessors.
if (
this._loggerProviderConfig.logRecordProcessor != null &&
config.logRecordProcessor != null
) {
throw new Error(
'LogRecordProcessor passed but LogRecordProcessor has already been configured.'
);
}

// set logRecordProcessor, but make sure we do not override existing logRecordProcessors with null/undefined.
if (config.logRecordProcessor != null) {
this._loggerProviderConfig.logRecordProcessor = config.logRecordProcessor;
}
}

/**
* @deprecated Please pass {@code views} and {@code reader} to the constructor options instead.
*
* Set configurations needed to register a MeterProvider
*/
public configureMeterProvider(config: MeterProviderConfig): void {
// nothing is set yet, we can set config and return.
if (this._meterProviderConfig == null) {
this._meterProviderConfig = config;
return;
}

// make sure we do not override existing views with other views.
if (this._meterProviderConfig.views != null && config.views != null) {
throw new Error('Views passed but Views have already been configured.');
}

// set views, but make sure we do not override existing views with null/undefined.
if (config.views != null) {
this._meterProviderConfig.views = config.views;
}

// make sure we do not override existing reader with another reader.
if (this._meterProviderConfig.reader != null && config.reader != null) {
throw new Error(
'MetricReader passed but MetricReader has already been configured.'
);
}

// set reader, but make sure we do not override existing reader with null/undefined.
if (config.reader != null) {
this._meterProviderConfig.reader = config.reader;
}
}

/**
* @deprecated Resources are detected automatically on {@link NodeSDK.start()}, when the {@code autoDetectResources}
* constructor option is set to {@code true} or left {@code undefined}.
*
* Detect resource attributes
*/
public detectResources(): void {
if (this._disabled) {
return;
}

const internalConfig: ResourceDetectionConfig = {
detectors: this._resourceDetectors,
};

this.addResource(detectResourcesSync(internalConfig));
}

/**
* @deprecated Please pre-merge resources and pass them to the constructor
*
* Manually add a Resource
* @param resource
*/
public addResource(resource: IResource): void {
this._resource = this._resource.merge(resource);
}

/**
* Call this method to construct SDK components and register them with the OpenTelemetry API.
*/
Expand All @@ -322,7 +208,13 @@ export class NodeSDK {
});

if (this._autoDetectResources) {
this.detectResources();
const internalConfig: ResourceDetectionConfig = {
detectors: this._resourceDetectors,
};

this._resource = this._resource.merge(
detectResourcesSync(internalConfig)
);
}

this._resource =
Expand Down
101 changes: 0 additions & 101 deletions experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -409,107 +409,6 @@ describe('Node SDK', () => {
delete env.OTEL_TRACES_EXPORTER;
});

it('should throw error when calling configureMeterProvider when views are already configured', () => {
const exporter = new InMemoryMetricExporter(
AggregationTemporality.CUMULATIVE
);
const metricReader = new PeriodicExportingMetricReader({
exporter: exporter,
exportIntervalMillis: 100,
exportTimeoutMillis: 100,
});

const sdk = new NodeSDK({
metricReader: metricReader,
views: [
new View({
name: 'test-view',
instrumentName: 'test_counter',
instrumentType: InstrumentType.COUNTER,
}),
],
autoDetectResources: false,
});

assert.throws(
() => {
sdk.configureMeterProvider({
reader: metricReader,
views: [
new View({
name: 'test-view',
instrumentName: 'test_counter',
instrumentType: InstrumentType.COUNTER,
}),
],
});
},
(error: Error) => {
return error.message.includes(
'Views passed but Views have already been configured'
);
}
);
});

it('should throw error when calling configureMeterProvider when metricReader is already configured', () => {
const exporter = new InMemoryMetricExporter(
AggregationTemporality.CUMULATIVE
);
const metricReader = new PeriodicExportingMetricReader({
exporter: exporter,
exportIntervalMillis: 100,
exportTimeoutMillis: 100,
});

const sdk = new NodeSDK({
metricReader: metricReader,
views: [
new View({
name: 'test-view',
instrumentName: 'test_counter',
instrumentType: InstrumentType.COUNTER,
}),
],
autoDetectResources: false,
});

assert.throws(
() => {
sdk.configureMeterProvider({
reader: metricReader,
});
},
(error: Error) => {
return error.message.includes(
'MetricReader passed but MetricReader has already been configured.'
);
}
);
});

it('should throw error when calling configureLoggerProvider when logRecordProcessor is already configured', () => {
const logRecordExporter = new InMemoryLogRecordExporter();
const logRecordProcessor = new SimpleLogRecordProcessor(logRecordExporter);
const sdk = new NodeSDK({
logRecordProcessor: logRecordProcessor,
autoDetectResources: false,
});

assert.throws(
() => {
sdk.configureLoggerProvider({
logRecordProcessor: logRecordProcessor,
});
},
(error: Error) => {
return error.message.includes(
'LogRecordProcessor passed but LogRecordProcessor has already been configured.'
);
}
);
});

describe('detectResources', async () => {
beforeEach(() => {
process.env.OTEL_RESOURCE_ATTRIBUTES =
Expand Down

0 comments on commit 7fb673c

Please sign in to comment.