Skip to content

Support OpenTelemetry SDK - #3559

Closed
AlexanderYastrebov wants to merge 1 commit into
masterfrom
add-open-telemetry-2
Closed

Support OpenTelemetry SDK#3559
AlexanderYastrebov wants to merge 1 commit into
masterfrom
add-open-telemetry-2

Conversation

@AlexanderYastrebov

@AlexanderYastrebov AlexanderYastrebov commented Jul 14, 2025

Copy link
Copy Markdown
Contributor

When OpenTelemetry is enabled Skipper registers global OpenTelemetry TracerProvider and creates a bridge OpenTracing tracer.

This follows recommended migration strategy https://opentelemetry.io/docs/migration/opentracing/#migration-overview

OpenTelemetry Init reads configuration options from standard environment variables and allows additional options as a command line flag in yaml format to simplify parsing.

Updates #2104

@AlexanderYastrebov AlexanderYastrebov added the major moderate risk, for example new API, small filter changes that have no risk like refactoring or logs label Jul 14, 2025
Comment thread tracing/tracing.go Outdated
Comment thread config/config.go
AccessLogStripQuery bool `yaml:"access-log-strip-query"`
SuppressRouteUpdateLogs bool `yaml:"suppress-route-update-logs"`

OpenTelemetry *otel.Options `yaml:"open-telemetry"`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe better to tell it's OTelOptions or so

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like it to match the flag name which looks ok. Let me think about it and I'll change it later if needed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just not obvious so maybe change the flag.
It's also kind of new so I would prefer to change it if we have it already.

Comment thread otel/otel.go
Comment thread otel/otel.go Outdated
@szuecs

szuecs commented Jul 14, 2025

Copy link
Copy Markdown
Member

Would this configure some added span Tags or something like this?
I am a bit skeptical about everything "auto"-magic things, but likely it's fine as far as I understand.

AlexanderYastrebov added a commit to zalando-incubator/kubernetes-on-aws that referenced this pull request Sep 8, 2025
Add configuration to enable OpenTelemetry tracing instead of OpenTracing.

See zalando/skipper#3559

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit to zalando-incubator/kubernetes-on-aws that referenced this pull request Sep 8, 2025
Add configuration to enable OpenTelemetry tracing instead of OpenTracing.

See zalando/skipper#3559

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit to zalando-incubator/kubernetes-on-aws that referenced this pull request Sep 8, 2025
Add configuration to enable OpenTelemetry tracing instead of OpenTracing.

See zalando/skipper#3559

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit to zalando-incubator/kubernetes-on-aws that referenced this pull request Sep 8, 2025
Add configuration to enable OpenTelemetry tracing instead of OpenTracing.

See zalando/skipper#3559

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit to zalando-incubator/kubernetes-on-aws that referenced this pull request Sep 8, 2025
Add configuration to enable OpenTelemetry tracing instead of OpenTracing.

See zalando/skipper#3559

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit to zalando-incubator/kubernetes-on-aws that referenced this pull request Sep 8, 2025
Add configuration to enable OpenTelemetry tracing instead of OpenTracing.

See zalando/skipper#3559

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit to zalando-incubator/kubernetes-on-aws that referenced this pull request Sep 8, 2025
Add configuration to enable OpenTelemetry tracing instead of OpenTracing.

See zalando/skipper#3559

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit to zalando-incubator/kubernetes-on-aws that referenced this pull request Sep 8, 2025
Add configuration to enable OpenTelemetry tracing instead of OpenTracing.

See zalando/skipper#3559

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit to zalando-incubator/kubernetes-on-aws that referenced this pull request Sep 8, 2025
Add configuration to enable OpenTelemetry tracing instead of OpenTracing.

See zalando/skipper#3559

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit to zalando-incubator/kubernetes-on-aws that referenced this pull request Sep 9, 2025
Add configuration to enable OpenTelemetry tracing instead of OpenTracing.

See zalando/skipper#3559

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit to zalando-incubator/kubernetes-on-aws that referenced this pull request Sep 9, 2025
Add configuration to enable OpenTelemetry tracing instead of OpenTracing.

See zalando/skipper#3559

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
When OpenTelemetry is enabled Skipper registers global OpenTelemetry TracerProvider and
creates a bridge OpenTracing tracer.

This follows recommended migration strategy https://opentelemetry.io/docs/migration/opentracing/#migration-overview

OpenTelemetry Init reads configuration options from standard environment variables and
allows additional options as a command line flag in yaml format to simplify parsing.

Updates #2104

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
@AlexanderYastrebov
AlexanderYastrebov marked this pull request as ready for review September 9, 2025 15:11
AlexanderYastrebov added a commit to zalando-incubator/kubernetes-on-aws that referenced this pull request Sep 9, 2025
Add configuration to enable OpenTelemetry tracing instead of OpenTracing.

See zalando/skipper#3559

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
Comment thread skipper.go
Comment thread otel/otel.go
Comment thread skipper.go
Comment thread proxy/proxy.go
Comment on lines +1638 to +1641
// https://pkg.go.dev/go.opentelemetry.io/otel/bridge/opentracing#readme-interop-from-trace-context-from-opentracing-to-opentelemetry
if bridgeTracer, ok := p.tracing.tracer.(*otBridge.BridgeTracer); ok {
ctxWithSpan = bridgeTracer.ContextWithSpanHook(ctxWithSpan, span)
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm.. that's interesting.
I don't understand how it works now, when we provide bridgeTracer through skipper options.

@AlexanderYastrebov

Copy link
Copy Markdown
Contributor Author

Closing in favour of #3627 which supports environment variable overrides via config.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

major moderate risk, for example new API, small filter changes that have no risk like refactoring or logs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants