Skip to content

Commit

Permalink
Merge branch 'main' into feat/api-synchronous-gauge
Browse files Browse the repository at this point in the history
  • Loading branch information
pichlermarc committed Apr 10, 2024
2 parents 7951ab7 + 19660db commit b5e13e8
Show file tree
Hide file tree
Showing 16 changed files with 104 additions and 262 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,12 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/

### :rocket: (Enhancement)

feat(sdk-trace-base): log resource attributes in ConsoleSpanExporter [#4605](https://github.com/open-telemetry/opentelemetry-js/pull/4605) @pichlermarc

### :bug: (Bug Fix)

* fix(sdk-trace-web): fix invalid timings in span events [#4486](https://github.com/open-telemetry/opentelemetry-js/pull/4486) @Abinet18
* fix(resources): ensure BrowserDetector does not think Node.js v21 is a browser [#4561](https://github.com/open-telemetry/opentelemetry-js/issues/4561) @trentm

### :books: (Refine Doc)

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

### :bug: (Bug Fix)

### :books: (Refine Doc)
Expand All @@ -31,9 +40,6 @@ All notable changes to experimental packages in this project will be documented
* was used internally to keep track of the service client used by the exporter, as a side effect it allowed end-users to modify the gRPC service client that was used
* `compression`
* was used internally to keep track of the compression to use but was unintentionally exposed to the users. It allowed to read and write the value, writing, however, would have no effect.
* feat(api-events)!: removed domain from the Events API [#4569](https://github.com/open-telemetry/opentelemetry-js/pull/4569) @martinkuba
* fix(api-events)!: renamed EventEmitter to EventLogger in the Events API [#4569](https://github.com/open-telemetry/opentelemetry-js/pull/4568) @martinkuba
* feat(api-logs)!: changed LogRecord body data type to AnyValue and AnyValueMap types [#4575](https://github.com/open-telemetry/opentelemetry-js/pull/4575) @martinkuba

### :rocket: (Enhancement)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,9 +224,9 @@ const testCollectorExporter = (params: TestParams) => {
setTimeout(() => {
const result = responseSpy.args[0][0] as core.ExportResult;
assert.strictEqual(result.code, core.ExportResultCode.FAILED);
assert.strictEqual(
assert.match(
responseSpy.args[0][0].error.details,
'Deadline exceeded'
/Deadline exceeded.*/
);
done();
}, 300);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,9 +231,9 @@ const testCollectorExporter = (params: TestParams) => {
setTimeout(() => {
const result = responseSpy.args[0][0] as core.ExportResult;
assert.strictEqual(result.code, core.ExportResultCode.FAILED);
assert.strictEqual(
assert.match(
responseSpy.args[0][0].error.details,
'Deadline exceeded'
/Deadline exceeded.*/
);
done();
}, 300);
Expand Down
136 changes: 14 additions & 122 deletions experimental/packages/opentelemetry-sdk-node/src/sdk.ts
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
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
12 changes: 11 additions & 1 deletion experimental/packages/otlp-transformer/src/common/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,18 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import type { IAnyValue, IKeyValue } from './types';
import type { IAnyValue, IInstrumentationScope, IKeyValue } from './types';
import { Attributes } from '@opentelemetry/api';
import { InstrumentationScope } from '@opentelemetry/core';

export function createInstrumentationScope(
scope: InstrumentationScope
): IInstrumentationScope {
return {
name: scope.name,
version: scope.version,
};
}

export function toAttributes(attributes: Attributes): IKeyValue[] {
return Object.keys(attributes).map(key => toKeyValue(key, attributes[key]));
Expand Down

0 comments on commit b5e13e8

Please sign in to comment.