zalando / skipper

An HTTP router and reverse proxy for service composition, including use cases like Kubernetes Ingress
https://opensource.zalando.com/skipper/
Other
3.12k stars 351 forks source link

Use OpenTelemetry instead of OpenTracing #2104

Open AlexanderYastrebov opened 2 years ago

AlexanderYastrebov commented 2 years ago

OpenTracing is deprecated, see https://github.com/opentracing/specification/issues/163

Migration plan 1

Initial idea implemented by https://github.com/zalando/skipper/pull/2578 was to refactor all the code to use OpenTelemetry tracer and provide an adapter that implements OpenTelemetry API but uses OpenTracing tracer to send tracing data. Then in subsequent steps introduce first-class OpenTelemetry tracer and switch to it.

Migration plan 2

Plan 1 has two downsides:

I propose the following migration plan:

1. Enable OpenTelemetry SDK

This is a suggested path in the official migration guide. The idea is to make Skipper talk OpenTelemetry protocol to the telemetry backend (Lightstep in our case) and keep existing components intact and continue using OpenTracing API via bridge component. OpenTracing API is deprecated which in other words means it is stable and is very unlikely to be changed:

No more new pull requests or feature requests will be accepted, only security fixes (which are unlikely given the nature of the API libraries).

This will achieve the goal of using OpenTelemetry from external point of view without introducing breaking changes for users.

2. Use OpenTelemetry API in new components

For all new components use OpenTelemetry API. These new components will not work for users that use OpenTracing but this will be an incentive to migrate.

3. Refactor existing components one-by-one to use OpenTelemetry API

This should be done case-by-case and depends on the components. We may deprecated OpenTracing filters and add complementary OpenTelemtry filters that use OpenTelemetry API and proper naming (e.g. for tracingTag add tracingAttribute, etc)

We may refactor HTTP and Redis clients to use OpenTelemetry API or support both OpenTracing and OpenTelemetry tracers.

The most complex is the Proxy component which we may tackle the last after learning from migration of other components.

szuecs commented 2 years ago

@AlexanderYastrebov we don't need to break library users if we can abstract the interface and plug either opentracing or opentelemetry into it.

https://pkg.go.dev/go.opentelemetry.io example client: https://github.com/open-telemetry/opentelemetry-go/blob/example/http/v0.10.0/example/http/client/client.go trace: https://pkg.go.dev/go.opentelemetry.io/otel/trace opentracing bridge, which might be possible to use as compatibility layer: https://pkg.go.dev/go.opentelemetry.io/otel/bridge/opentracing#section-readme

fpiche commented 1 year ago

Hey there! Has there been any developments on this front ? I would love to be able to enable opentelemetry tracing for Skipper, since it would plug nicely into the rest of my observability stack.

szuecs commented 1 year ago

@fpiche yes we should do it also in our case, but we did not have the time yet. In general I think either we can make it the current tracer compliant or we create a separate module. The former will have much less code changes and will likely create less of a headache.

Or we do a major breaking change which we as a library don't want.