Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use OpenTelemetry instead of OpenTracing #2104

Open
AlexanderYastrebov opened this issue Oct 14, 2022 · 3 comments
Open

Use OpenTelemetry instead of OpenTracing #2104

AlexanderYastrebov opened this issue Oct 14, 2022 · 3 comments
Assignees
Labels
breaks for library users enhancement tracing tracing or opentracing related issue

Comments

@AlexanderYastrebov
Copy link
Member

AlexanderYastrebov commented Oct 14, 2022

OpenTracing is deprecated, see opentracing/specification#163

Migration plan 1

Initial idea implemented by #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:

  • refactors a lot of code and hence increases chances to introduce bugs
  • does not use OpenTelemetry protocol to talk to telemetry backend, i.e. does not migrate to OpenTelemetry from external point of view.

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
Copy link
Member

szuecs commented Oct 14, 2022

@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
Copy link

fpiche commented Apr 12, 2023

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
Copy link
Member

szuecs commented Apr 12, 2023

@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.

AlexanderYastrebov added a commit that referenced this issue Apr 24, 2024
OpenTelemetry-OpenTracing bridge span kind can not be changed after creation,
see open-telemetry/opentelemetry-go#3953

The workaround is to specify span kind on creation which works for
both Open Tracing and Open Telemetry bridge spans.

For #2104

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit that referenced this issue Apr 24, 2024
OpenTelemetry-OpenTracing bridge span kind can not be changed after creation,
see open-telemetry/opentelemetry-go#3953

The workaround is to specify span kind on creation which works for
both Open Tracing and Open Telemetry bridge spans.

For #2104

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit that referenced this issue Apr 24, 2024
OpenTelemetry-OpenTracing bridge span kind can not be changed after creation,
see open-telemetry/opentelemetry-go#3953

The workaround is to specify span kind on creation which works for
both Open Tracing and Open Telemetry bridge spans.

For #2104

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit that referenced this issue Apr 26, 2024
OpenTelemetry-OpenTracing bridge span kind can not be changed after creation,
see open-telemetry/opentelemetry-go#3953

The workaround is to specify span kind on creation which works for
both Open Tracing and Open Telemetry bridge spans.

For #2104

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit that referenced this issue Apr 26, 2024
OpenTelemetry-OpenTracing bridge span kind can not be changed after creation,
see open-telemetry/opentelemetry-go#3953

The workaround is to specify span kind on creation which works for
both Open Tracing and Open Telemetry bridge spans.

For #2104

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit that referenced this issue Apr 26, 2024
OpenTelemetry-OpenTracing bridge span kind can not be changed after creation,
see open-telemetry/opentelemetry-go#3953

The workaround is to specify span kind on creation which works for
both Open Tracing and Open Telemetry bridge spans.

Note that this change removes non-standard "shedder" kind from spans created by `admissionControl` filter.
Use operation name "admission_control" to query its spans instead if needed.

For #2104

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit that referenced this issue Apr 26, 2024
Deprecates opentracing getters and removes their usage from filters.

For #2104

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit that referenced this issue Apr 26, 2024
Deprecate OpenTracing FilterContext getters and remove their usage from filters.

For #2104

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit that referenced this issue Apr 26, 2024
Deprecate OpenTracing FilterContext getters and remove their usage from filters.

For #2104

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
@AlexanderYastrebov AlexanderYastrebov self-assigned this Apr 26, 2024
AlexanderYastrebov added a commit that referenced this issue Apr 26, 2024
Deprecate OpenTracing FilterContext getters and remove their usage from filters.

For #2104

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
JanardhanSharma pushed a commit to JanardhanSharma/skipper that referenced this issue Apr 26, 2024
OpenTelemetry-OpenTracing bridge span kind can not be changed after creation,
see open-telemetry/opentelemetry-go#3953

The workaround is to specify span kind on creation which works for
both Open Tracing and Open Telemetry bridge spans.

Note that this change removes non-standard "shedder" kind from spans created by `admissionControl` filter.
Use operation name "admission_control" to query its spans instead if needed.

For zalando#2104

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
Signed-off-by: Janardhan Sharma <janardhan.sharma@zalando.de>
AlexanderYastrebov added a commit that referenced this issue May 7, 2024
YAML flag parses (preferably flow-style) YAML value from command line
or unmarshals object yaml value from config file.

Example value:
```
bin/skipper -foo-flag='{foo: hello, bar: [world, "1"], baz: 2, qux: {baz: 3}}'
```

and equivalent branch in config yaml:
```yaml
foo-flag:
  foo: hello
  bar:
    - world
    - "1"
  baz: 2
  qux:
    baz: 3
```

This will be useful for #2104

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit that referenced this issue May 7, 2024
YAML flag parses (preferably flow-style) YAML value from the command line
or unmarshals object yaml value from the yaml config file.

Example value:
```
bin/skipper -foo-flag='{foo: hello, bar: [world, "1"], baz: 2, qux: {baz: 3}}'
```

and equivalent branch in config yaml:
```yaml
foo-flag:
  foo: hello
  bar:
    - world
    - "1"
  baz: 2
  qux:
    baz: 3
```

This will be useful for #2104

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit that referenced this issue May 7, 2024
YAML flag parses (preferably flow-style) YAML value from the command line
or unmarshals object yaml value from the yaml config file.

Example value:
```
bin/skipper -foo-flag='{foo: hello, bar: [world, "1"], baz: 2, qux: {baz: 3}}'
```

and equivalent branch in config yaml:
```yaml
foo-flag:
  foo: hello
  bar:
    - world
    - "1"
  baz: 2
  qux:
    baz: 3
```

This will be useful for #2104

It is also a better alternative to manual parsing of micro-syntax like
e.g. implemented in #2888

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit that referenced this issue May 7, 2024
YAML flag parses (preferably flow-style) YAML value from the command line
or unmarshals object yaml value from the yaml config file.

Example value:
```
bin/skipper -foo-flag='{foo: hello, bar: [world, "1"], baz: 2, qux: {baz: 3}}'
```

and equivalent branch in config yaml:
```yaml
foo-flag:
  foo: hello
  bar:
    - world
    - "1"
  baz: 2
  qux:
    baz: 3
```

This will be useful for #2104

It is also a better alternative to manual parsing of micro-syntax like
e.g. implemented in #2888

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit that referenced this issue May 8, 2024
YAML flag parses (preferably flow-style) YAML value from the command line
or unmarshals object yaml value from the yaml config file.

Example value:
```
bin/skipper -foo-flag='{foo: hello, bar: [world, "1"], baz: 2, qux: {baz: 3}}'
```

and equivalent branch in config yaml:
```yaml
foo-flag:
  foo: hello
  bar:
    - world
    - "1"
  baz: 2
  qux:
    baz: 3
```

This will be useful for #2104

It is also a better alternative to manual parsing of micro-syntax like
e.g. implemented in #2888

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaks for library users enhancement tracing tracing or opentracing related issue
Projects
None yet
Development

No branches or pull requests

3 participants