Specify tracing span kind on creation #3039
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -456,7 +456,6 @@ func (ac *admissionControl) startSpan(ctx context.Context) (span opentracing.Spa | |||
| if parent != nil { | |||
| span = ac.tracer.StartSpan(admissionControlSpanName, opentracing.ChildOf(parent.Context())) | |||
| ext.Component.Set(span, "skipper") | |||
| ext.SpanKind.Set(span, "shedder") | |||
There was a problem hiding this comment.
I am not sure if we really need non-standard kind here so decided to drop it.
And this would break in OpenTelemetry, see
https://github.com/open-telemetry/opentelemetry-go/blob/9656d0afa72646101e859bf8a1c6d05b73ee094d/trace/trace.go#L460-L474
There was a problem hiding this comment.
Can you check if we have streams that might select it and propose a change?
There was a problem hiding this comment.
Found one that also uses operation IN ("admission_control") which should be enough.
There was a problem hiding this comment.
Reached out to the owner and this was fixed.
There was a problem hiding this comment.
Can you add this to the release notes, too?
I don't know who will hit by that but that we do tell about.
szuecs
reviewed
Apr 24, 2024
AlexanderYastrebov
force-pushed
the
specify-span-kind-on-creation
branch
from
99fea88 to
e05c8b8
Compare
|
let's land it, no? |
|
👍 |
AlexanderYastrebov
force-pushed
the
specify-span-kind-on-creation
branch
from
e05c8b8 to
c3d7298
Compare
|
Rebased and fixed span creation added by #3034 |
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
force-pushed
the
specify-span-kind-on-creation
branch
from
c3d7298 to
406a3ff
Compare
|
👍 |
1 similar comment
|
👍 |
AlexanderYastrebov
added a commit
that referenced
this pull request
Apr 26, 2024
Use setTag helper to enable tag exclusion This is follow up fix for #3039 Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov
added a commit
that referenced
this pull request
Apr 26, 2024
Use setTag helper to enable tag exclusion This is follow up fix for #3039 Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
JanardhanSharma
pushed a commit
to JanardhanSharma/skipper
that referenced
this pull request
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>
This was referenced Apr 26, 2024
JanardhanSharma
pushed a commit
to JanardhanSharma/skipper
that referenced
this pull request
Jul 19, 2024
Use setTag helper to enable tag exclusion This is follow up fix for zalando#3039 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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