Migrate collector's metrics from OC to OpenTelemetry

[pavolloffay]

Currently, the collector uses OpenCensus API for internal observability

opentelemetry-collector/observability/observability.go

Line 25 in 0e1b2e3

"go.opencensus.io/stats"

.

These metrics should be probably migrated to OpenTelemetry API.

Jaeger storage components use abstract metrics API and so we need to export these metrics in Jaeger OTEL collector e.g.

https://github.com/jaegertracing/jaeger/blob/bf870d9e2bffaf4758045919446be75ac9fc9783/cmd/opentelemetry-collector/app/exporter/cassandra/exporter.go#L31.

Are there plans to migrate to OpenTelemetry metrics? In Jaeger we want to avoid first creating OC adapter and then OTEL once this repo migrates. I can submit a PR to port the metrics to OTEL API.

[yurishkuro]

This would also provide the needed dogfooding for OTel SDK.

[bogdandrutu]

@yurishkuro completely agree, we are waiting for 2 things:

1. The ability to use labels from the correlation-context.
2. The ability to configure Views - allow us to configure which labels from the correlation-context to use.

If we don't have these two things we need to manually propagate/retrieve these values every time we do a report. Which may be possible if we do have couple of helper functions. So probably the main missing feature is to support changing the default aggregations for some instruments like for distributions we don't want just minmaxsumcount we want a histogram.

• @jmacd to help correct me if we can already do the histogram thing.

[yurishkuro]

Why do you need context labels in the collector? Sounds a bit like nice to have, not a showstopper.

[bogdandrutu]

I think I also tried to explain myself that we have the solution to use helper functions, but let me describe the design a bit:

• In the pipeline we want to record at every step metric broken down by the "receiver" so we add receiver to the context to propagate it in the pipeline instead of having it as an explicit argument. So we can have helper functions that just retrieve that from the context and use it in the metrics as labels.

And as I explained is not a showstopper, the current blocker is the ability to configure the aggregation for the distribution to be histogram vs minmaxsumcount, and probably a nice to have is the ability to select the labels for the aggregator (again not a showstopper).

[bogdandrutu]

@pavolloffay to answer your question, we do want to use opentelemetry metrics. probably tracing as well :)

[pavolloffay]

thanks @bogdandrutu. I can open a PR to port the metrics.

Is there any issue in opentelemetry-go for:

So probably the main missing feature is to support changing the default aggregations for some instruments like for distributions we don't want just minmaxsumcount we want a histogram.

[bogdandrutu]

I think the missing thing is this otep open-telemetry/oteps#89

[tigrannajaryan]

@bogdandrutu I think I remember some recent work that you did on this and changed telemetry to use Otel API instead of OpenCensus. Is that correct? If so I can close this.

[nilebox]

Collector is still using OpenCensus in its self-observability pipeline:

opentelemetry-collector/service/telemetry.go

Line 25 in c8aac9e

"go.opencensus.io/stats/view"

There is #1833 switching traces to Otel API (not merged yet), but that still doesn't include metrics.

[mxiamxia]

The scope to resolve this issue is vague. Are we planing to only remove the OC metric reference in public API for unblocking Tracing GA? Because it's hard to replace all OC metrics in all those metrics components in core and contrib repos before tracing GA timeframe.

Need help to elaborate the exit criteria/scope on this issue? And we can contribute for the changes :)

[alolita]

@bogdandrutu since you mentioned that this issue is not a showstopper can it be moved to a post-GA phase 3 backlog?

[alolita]

@bogdandrutu: We will evaluate all the core components for any public API dependence on OC metrics references and respond on issue.

[mxiamxia]

Discussed it in SIG with @tigrannajaryan and @bogdandrutu , this issue is currently blocked by open-telemetry/oteps#89.

[bogdandrutu]

Block by open-telemetry/opentelemetry-go#2204

[yurishkuro]

@bogdandrutu open-telemetry/opentelemetry-go#2204 is about OpenCensus bridge, why is it a concern? I think the path would be to rewrite instrumentation to natively talk to OTEL-Go API, not to continue going through OC API.

[fatsheep9146]

@bogdandrutu open-telemetry/opentelemetry-go#2204 is about OpenCensus bridge, why is it a concern? I think the path would be to rewrite instrumentation to natively talk to OTEL-Go API, not to continue going through OC API.

@yurishkuro

Since @dashpole provided a mature propose that the metrics migration would use OpenCensus bridge for OpenTelemetry, like trace.
#3816 (review)

As an alternative, I would propose that we use both opencensus and opentelemetry client libraries at the same time while we migrate. We can use the OpenCensus bridge for OpenTelemetry to make both opencensus and opentelemetry clients serve metrics on the same prometheus endpoint.
We could roll this out with the following steps:

• Switch from the OpenCensus prometheus exporter to a "wrapped" OpenTelemetry prometheus exporter. Verify that self obs metrics on the prometheus endpoint remain the same.
• Add the OpenTelemetry API/SDK and use the same prometheus exporter that OC uses.
• Migrate each metric from OC to OTel individually, and verify that the metric doesn't change.

But this propose is blocked by issue #2204, the prometheus exporter does not satisfy the requirement for OpenCensus metric bridge for OpenTelemetry, which is described in https://pkg.go.dev/go.opentelemetry.io/otel/bridge/[email protected]#readme-metrics

[jpkrohling]

We discussed this a couple of SIG calls ago, and @jaronoff97 was set to check what's our current state by working on a couple of components first and reporting back.

[jaronoff97]

@jpkrohling Our team is currently looking in to this. @paivagustavo has been working on what a migration may look like and has already confirmed that we can provide the same prometheus metrics (and histogram buckets) with the latest Go SDK release. I'll let Gustavo provide some more details.

[paivagustavo]

I've been working on testing the latest released otel go sdk and verifying that it would met the requirements here. As part of this I've instrumented the existing receiver metrics from obsreport.Receiver(#6222) and the loadbalancingexporter from contrib, more specifically testing that we could use different histogram buckets for specific metrics. Using the views api, we are able to accomplish that.

About the bridge, as it is in review at the moment (open-telemetry/opentelemetry-go#3207), I would suggest us to start to double instrumenting the collector with otel go in the meantime. If the bridge is merged and released before we finish migrating the current metrics, we can enable it and start removing oc metrics that have already being instrumented with otel-go.