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

[OpenTracing bridge] Support setting span kind tag after the creation of span #3997

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ChenX1993
Copy link
Contributor

@ChenX1993 ChenX1993 commented Apr 13, 2023

Resolves #3953

A few middleware frameworks (e.g https://github.com/yarpc/yarpc-go/blob/600cb49cc6331d8d15f65c058615a7649447efa7/api/transport/propagation.go#L67) that our company is using still set the OT span kind tag after a span is created.

Missing the span kind tags will impact a few functionalities in Jaeger (one of the OpenTracing implementations) that reply on the span kind tags.

Setting span kind tag in the bridge after a span is created cannot really set the SpanKind of OTel span, but adds the attribute instead which still helps in our case a lot.

@ChenX1993 ChenX1993 changed the title [OpenTracing bridge] Support setting span kind tag after the creation… [OpenTracing bridge] Support setting span kind tag after the creation of span Apr 13, 2023
@codecov
Copy link

codecov bot commented Apr 13, 2023

Codecov Report

Merging #3997 (b8e066b) into main (1b04bbc) will increase coverage by 0.0%.
The diff coverage is 100.0%.

❗ Current head b8e066b differs from pull request most recent head ad34f57. Consider uploading reports for the commit ad34f57 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #3997   +/-   ##
=====================================
  Coverage   82.2%   82.2%           
=====================================
  Files        175     175           
  Lines      13058   12987   -71     
=====================================
- Hits       10738   10681   -57     
+ Misses      2099    2084   -15     
- Partials     221     222    +1     
Impacted Files Coverage Δ
bridge/opentracing/bridge.go 65.2% <100.0%> (+1.4%) ⬆️
bridge/opentracing/internal/mock.go 73.7% <100.0%> (+6.6%) ⬆️

... and 24 files with indirect coverage changes

… of a span

Signed-off-by: Chen Xu <chen.x@uber.com>
// only add span kind tag when otel span kind is unset or bridge default (internal)
if span, ok := s.otelSpan.(readSpanKindSpan); ok {
if span.SpanKind() == trace.SpanKindUnspecified || span.SpanKind() == trace.SpanKindInternal {
s.otelSpan.SetAttributes(otTagToOTelAttr(key, value))

Choose a reason for hiding this comment

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

This will cause the span.kind to be duplicated at the exporter level. This will set it in the attributes, but the recordingSpan.spanKind will also be set to the default internal - both of these seem to get sent via the exporter. I've tested this with go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc sending to Jaeger collector.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[OpenTracing Bridge] Support setting span kind tag after span is created
3 participants