open-telemetry / opentelemetry-collector

OpenTelemetry Collector
https://opentelemetry.io
Apache License 2.0
4.53k stars 1.48k forks source link

remove `MetricsLevel` from `TelemetrySettings` #11061

Open codeboten opened 3 months ago

codeboten commented 3 months ago

A discussion on 4-Sept-2024 lead to the agreement that the struct field MetricsLevel will be removed from TelemetrySettings. This field is there to allow component authors to check a level before deciding whether to incur the cost of calculating an expensive operation.

The alternative for component authors will be to check whether an instrument is enabled or not prior to calculating the metric. This is dependent on the PR in otel go to implement this and on the spec to stabilize this feature.

The decision was that if the change would take too long to implement upstream, then we would remove this field and move any component to configure their own metric level instead. This will allow the unblocking of the component module stabilization work.

mx-psi commented 2 months ago

@bogdandrutu Are you going to submit any changes to the spec related to this? (Maybe related to open-telemetry/opentelemetry-specification/issues/4200?)

mx-psi commented 2 months ago

Given that stabilization of Enabled is not on September's release (see open-telemetry/opentelemetry-specification/pull/4204), I think we should go with the alternative plan, since mid-October would delay us significantly

mx-psi commented 1 month ago

Summary of current state: open-telemetry/opentelemetry-specification/issues/4256 and open-telemetry/opentelemetry-specification/issues/4207 are blocking stabilization in the spec. There is a PR for 4256: open-telemetry/opentelemetry-specification/pull/4262 that is blocked by adding a prototype. Nobody is working on a prototype. For 4207, things seem to be moving forward.

This is one of the last issues blocking component 1.0. I have been focusing on #11499 and related issues in case it changes the roadmap and this becomes less important, but given the state above, we may want to go with the approach of "remove MetricsLevel and use feature gates for current use cases"/

mx-psi commented 2 weeks ago

There is also now an OTEP related to one of the issues above: open-telemetry/opentelemetry-specification/pull/4290

mx-psi commented 2 weeks ago

We discussed open-telemetry/opentelemetry-specification/issues/4256 at the 2024-11-19 spec meeting. We will close it on 2024-12-03 unless there are use case brought up based on https://github.com/open-telemetry/opentelemetry-specification/issues/4256#issuecomment-2486242726.

This also would mean that https://github.com/open-telemetry/opentelemetry-specification/issues/4207 is not blocking stabilization, so we may (fingers crossed) be able to merge the stabilization PR.

mx-psi commented 1 week ago

This will be unblocked by open-telemetry/opentelemetry-go/issues/6002