knative / eventing

Event-driven application platform for Kubernetes
https://knative.dev/docs/eventing
Apache License 2.0
1.4k stars 590 forks source link

Port opencensus integration to opentelemetry #3126

Open slinkydeveloper opened 4 years ago

slinkydeveloper commented 4 years ago

Problem Now we use opencensus, which is going to be deprecated, we need to switch to https://github.com/open-telemetry/opentelemetry-go

Persona: Knative contributor

Exit Criteria We don't depend anymore on opencensus

Time Estimate (optional): :man_shrugging:

Additional context (optional) We also need to handle this: https://github.com/knative/eventing-contrib/pull/1155#issuecomment-623294931

lionelvillard commented 4 years ago

Related to https://github.com/knative/pkg/issues/855

slinkydeveloper commented 4 years ago

Oh thanks for spotting it!

grantr commented 4 years ago

Looks like OpenTelemetry Go SDK is in beta now: https://github.com/open-telemetry/opentelemetry-go. So the next step is making sure that OT meets all our requirements.

slinkydeveloper commented 4 years ago

@grantr do we have some document with these requirements defined? How would you proceed?

vaikas commented 4 years ago

@evankanderson @anniefu would you mind sharing what you had in mind? I know you had done a lot of work here.

anniefu commented 4 years ago

The upcoming metrics plan that Evan wrote up https://github.com/knative/pkg/blob/master/metrics/README.md is primarily about moving to a centralized collector service model for metrics.

Since, the OpenCensus Collector Service agent and the OpenTelemetry Collector Service agent will both support the OpenCensus client libraries, we can port from opencensus client libraries to the opentelemetry client libraries independently of the new metrics plan.

The last time we talked to OpenTelemetry back in february, their metrics models were still in flux and they recommended @evankanderson make changes to opencensus library to support Knative rather than trying to early-migrate to Otel client libraries.

Given that, I think we want the metrics plan to land and maybe wait beyond Otel beta before considering porting Knative libraries.

anniefu commented 4 years ago

In terms of https://github.com/knative/eventing-contrib/pull/1155, tracing and metrics in Knative are handled completely separately right now, so it is possible to migrate to OpenTelemetry for tracing, but not metrics.

I'm not very familiar with the world of tracing, so I'll leave that judgment call to others.

slinkydeveloper commented 4 years ago

Links to https://github.com/cloudevents/sdk-go/issues/554

github-actions[bot] commented 3 years ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

slinkydeveloper commented 3 years ago

/remove-lifecycle stale

skonto commented 3 years ago

We discussed this a bit yesterday (meeting minutes here), I will take a look by porting the Api Server source to the new model and then we can discuss the next steps.

devguyio commented 3 years ago

@skonto that sounds great, can you please create an issue for that ?

skonto commented 3 years ago

Sure @devguyio

slinkydeveloper commented 3 years ago

@skonto is this issue still useful? Or is there an updated one and we can close this one?

skonto commented 3 years ago

@slinkydeveloper let's keep this is an umbrella ticket for the Eventing migration work. The other ticket is just a sub-task.

github-actions[bot] commented 3 years ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

ustiugov commented 3 years ago

hi all, I wonder when we could expect eventing to support OpenTelemetry.

skonto commented 3 years ago

@ustiugov if you are referring to the OTEL client go lib afaik it is not ready from a metrics perspective. Tracing is in better shape upstream. Integrating with the OTEL collector is possible as described in the docs.

ustiugov commented 3 years ago

@ustiugov if you are referring to the OTEL client go lib afaik it is not ready from a metrics perspective. Tracing is in better shape upstream. Integrating with the OTEL collector is possible as described in the docs.

@skonto thank you. OpenTelemetry traces for Serving and our gRPC based functions work well.

With eventing, we are able to visualize Knative components but we do not know how to add the functions (also deployed with Serving), which generate ClouEvents and get triggered by them, to the same traces.

I wonder if there are any examples for that.

skonto commented 3 years ago

@ustiugov cloudevents client is observability aware so if you try setup tracing as in https://knative.dev/docs/eventing/accessing-traces/ you should see them for existing components. Same applies to user services that utilize the same api and setup tracing accordingly: https://github.com/cloudevents/sdk-go/blob/release-2.1/samples/http/receiver-traced/main.go. For a concrete trace handler using opencensus check: https://github.com/knative/eventing/blob/2fcbc0ac500dc06433a851e242fa57a23dedbdfa/pkg/adapter/v2/cloudevents.go#L60. Not sure if there are more samples maybe @slinkydeveloper has more pointers.

github-actions[bot] commented 2 years ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

ustiugov commented 2 years ago

/remove-lifecycle stale

github-actions[bot] commented 2 years ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

ustiugov commented 2 years ago

/remove-lifecycle stale