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.

Sign up for GitHub

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

Jump to bottom

[OpenTracing Bridge] Support setting span kind tag after span is created #3953

Open
ChenX1993 opened this issue Mar 31, 2023 · 2 comments · May be fixed by #3997
Open

[OpenTracing Bridge] Support setting span kind tag after span is created #3953

ChenX1993 opened this issue Mar 31, 2023 · 2 comments · May be fixed by #3997
Labels
enhancement New feature or request

Comments

@ChenX1993
Copy link
Contributor

ChenX1993 commented Mar 31, 2023 •
edited
Loading

Problem Statement

Currently in OT Bridge, it does nothing when setting span kind tag.

opentelemetry-go/bridge/opentracing/bridge.go

Lines 158 to 170 in 271df1d

func (s *bridgeSpan) SetTag(key string, value interface{}) ot.Span {
switch key {
case string(otext.SpanKind):
// TODO: Should we ignore it?
case string(otext.Error):
if b, ok := value.(bool); ok && b {
s.otelSpan.SetStatus(codes.Error, "")
}
default:
s.otelSpan.SetAttributes(otTagToOTelAttr(key, value))
}
return s
}

This cause some problem because the middle layers of some framework set span kind tag after a span is created, and we don't have control on those framework. Hence the spans are missing the span kind tag.
e.g. YAPRC: https://github.com/yarpc/yarpc-go/blob/600cb49cc6331d8d15f65c058615a7649447efa7/api/transport/propagation.go#L67

span := c.Tracer.StartSpan(
		req.Procedure,
		opentracing.StartTime(c.StartTime),
		opentracing.ChildOf(parent),
		tags,
)
ext.PeerService.Set(span, req.Service)
ext.SpanKindRPCClient.Set(span)

Is there any strong reason not to set span kind tags?

Proposed Solution

Support setting span kind tag.
@ChenX1993 ChenX1993 added the enhancement New feature or request label Mar 31, 2023
@ChenX1993
Copy link
Contributor Author

Hi, can I pick this up if this request makes sense?

@ChenX1993 ChenX1993 linked a pull request Apr 13, 2023 that will close this issue
Open
@AlexanderYastrebov
Copy link

The workaround when sources can be changed is to specify span kind tag on creation which works for both Open Tracing and Open Telemetry bridge spans:

opts := []ot.StartSpanOption{ot.Tags{
  "span.kind":       "client",
  // etc
}}
if parentSpan := ot.SpanFromContext(req.Context()); parentSpan != nil {
  opts = append(opts, ot.ChildOf(parentSpan.Context()))
}
span := tracer.StartSpan(spanName, opts...)

AlexanderYastrebov added a commit to zalando/skipper that referenced this issue Apr 24, 2024
@AlexanderYastrebov
proxy: specify span kind on creation
95397f8 
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 <[email protected]>
AlexanderYastrebov added a commit to zalando/skipper that referenced this issue Apr 24, 2024
@AlexanderYastrebov
Specify tracing span kind on creation
99fea88 
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 <[email protected]>
@AlexanderYastrebov AlexanderYastrebov mentioned this issue Apr 24, 2024
Merged
AlexanderYastrebov added a commit to zalando/skipper that referenced this issue Apr 24, 2024
@AlexanderYastrebov
Specify tracing span kind on creation
e05c8b8 
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 <[email protected]>
AlexanderYastrebov added a commit to zalando/skipper that referenced this issue Apr 26, 2024
@AlexanderYastrebov
Specify tracing span kind on creation
c3d7298 
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 <[email protected]>
AlexanderYastrebov added a commit to zalando/skipper that referenced this issue Apr 26, 2024
@AlexanderYastrebov
Specify tracing span kind on creation
406a3ff 
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 <[email protected]>
AlexanderYastrebov added a commit to zalando/skipper that referenced this issue Apr 26, 2024
@AlexanderYastrebov
Specify tracing span kind on creation (#3039)
b02d681 
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 <[email protected]>
JanardhanSharma pushed a commit to JanardhanSharma/skipper that referenced this issue Apr 26, 2024
@AlexanderYastrebov
Specify tracing span kind on creation (zalando#3039)
bb88e1f 
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 <[email protected]>
Signed-off-by: Janardhan Sharma <[email protected]>
@dkrizic dkrizic mentioned this issue Sep 8, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
2 participants
@AlexanderYastrebov @ChenX1993