search

open-telemetry / opentelemetry-collector

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

Move telemetry initialization logic to `service/telemetry` #8170

Open mx-psi opened 1 year ago

mx-psi commented 1 year ago

Telemetry initialization is currently split between the service/telemetry package and the service package in the telemetry.go package. This causes some issues:

  1. The telemetry.TracerProvider method returns a tracer provider that is never used in practice. It is used here https://github.com/open-telemetry/opentelemetry-collector/blob/3089ea878eef022913a053600f3599b3fddffb95/service/service.go#L103 and then immediately overridden by the telemetry initializer one. https://github.com/open-telemetry/opentelemetry-collector/blob/3089ea878eef022913a053600f3599b3fddffb95/service/service.go#L115 The telemetry initializer tracer provider is built from scratch https://github.com/open-telemetry/opentelemetry-collector/blob/3089ea878eef022913a053600f3599b3fddffb95/service/telemetry.go#L101-L111 without using telemetry.TracerProvider. I believe because of this the sampling configuration set on service/telemetry is ignored. https://github.com/open-telemetry/opentelemetry-collector/blob/3089ea878eef022913a053600f3599b3fddffb95/service/telemetry/telemetry.go#L47-L50
  2. Parts of telemetry are initialized in service/telemetry, while others are initialized in service. In particular, the logger is initialized in service/telemetry, the tracer provider can be initialized in both but only the service one is used, and the metrics provider is only initialized in service.

I think we should consolidate telemetry initialization to the service/telemetry package and have methods to generate the telemetry providers from config as part of the public API. This would mean adding a new method to telemetry.Telemetry to handle registration of process metrics, currently done in the service via the telemetry initializer https://github.com/open-telemetry/opentelemetry-collector/blob/3089ea878eef022913a053600f3599b3fddffb95/service/service.go#L210-L211

One challenge here is that proctelemetry depends on service/telemetry for the generated configuration; if we don't want to expose the OpenCensus registry we need to refactor the current package structure (e.g. by having a new service/telemetry/config package).

cc @codeboten since this may overlap with the work on #7532

mx-psi commented 3 months ago

Removed this from Collector v1 project board based on #10643