opentelemetry-collector
opentelemetry-collector copied to clipboard
[component] TelemetrySettings.MetricsLevel is experimental
The MetricsLevel field is marked as experimental https://pkg.go.dev/go.opentelemetry.io/collector/component#TelemetrySettings. We should decide what to do with it (stabilize it, remove it, change it in some way)
I expected TelemetrySettings to eventually include a LoggerProvider once the logging bridge API is implemented in otel-go. This makes me wonder if TelemetrySettings should be moved out of component
I expected TelemetrySettings to eventually include a LoggerProvider once the logging bridge API is implemented in otel-go.
I had assumed we would keep on using *zap.Logger with a zapcore.Core that uses the LoggerProvider behind the scenes.
This makes me wonder if TelemetrySettings should be moved out of component
But I am in favor of this regardless, I am just not sure where it should go (service/telemetry implies a dependency of the components on service)
I expected TelemetrySettings to eventually include a LoggerProvider once the logging bridge API is implemented in otel-go. This makes me wonder if TelemetrySettings should be moved out of component
Since we pass the TelemetrySettings in the start/creation of the components even if you move it, you depend on it so you have the same problem
We should decide what to do with it (stabilize it, remove it, change it in some way)
I want to remove it, since it was used to configure "high cardinality" metrics in the past and we should have views and other configs for that.
@codeboten Does the useOtelWithSDKConfigurationForInternalTelemetry feature gate still allow setting configtelemetry.Level? If so, I think we should remove its usage (at least once we add support for views).
It would also be interesting to tie ignoring this level in components that use it today with the useOtelWithSDKConfigurationForInternalTelemetry flag
@codeboten Could you take a look at https://github.com/open-telemetry/opentelemetry-collector/issues/9510#issuecomment-1934046671 ?
After discussing this at the 3-Apr-2024 SIG call, it was suggested that the use of a level is easier for users so we should keep it and remove the message about it being experimental.
More advanced use-cases will be able to enable all metrics and use views for maximum flexibility/control over what metrics are emitted.
I think we should distinguish service.Telemetry.Config.MetricsConfig.Level from component.TelemetrySettings.MetricsLevel. We agreed to keep service.Telemetry.Config.MetricsConfig.Level to keep user configuration easy, but we discussed several options for component.TelemetrySettings.MetricsLevel:
- Keep as is
- Have several meter providers for the different metrics levels
- Allow components to define views in addition to metrics, and tie said views to levels
(2) could look something like:
type LeveledMeterProvider interface {
MeterProvider(level configtelemetry.Level) metric.MeterProvider // noop/SDK provider depending on MetricsConfig.Level
}
or be at a lower level (when getting the meter or the instrument).