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

Expose TraceID and SpanID from SpanContext #188

Open
gianarb opened this Issue Jun 7, 2018 · 12 comments

Comments

Projects
None yet
6 participants
@gianarb
Copy link

gianarb commented Jun 7, 2018

Hello folks! Thanks for all the work you did.

Use Case

I would like to expose the trace_id out from a Span mainly over HTTP header, and I would also like to attach it to the logs that my application generates.

Mainly for support purpose. A developer or a system integrator will be able to communicate the trace_id back to the support team, and it will easy for them to understand what is going on.

Problem

I looked around, and it seems like this information is not accessible from the SpanContext but I had a quick chat with @yurishkuro in the mailing list, and it looks like the specification now covers this use case:

https://github.com/opentracing/specification/blob/master/rfc/trace_identifiers.md#specification-changes

Proposal

I opened this issue to figure out together how we should update the SpanContext interface to cover this use case.

// SpanContext represents Span state that must propagate to descendant Spans and across process
// boundaries (e.g., a <trace_id, span_id, sampled> tuple).
type SpanContext interface {
	// ForeachBaggageItem grants access to all baggage items stored in the
	// SpanContext.
	// The handler function will be called for each baggage key/value pair.
	// The ordering of items is not guaranteed.
	//
	// The bool return value indicates if the handler wants to continue iterating
	// through the rest of the baggage items; for example if the handler is trying to
	// find some baggage item by pattern matching the name, it can return false
	// as soon as the item is found to stop further iterations.
	ForeachBaggageItem(handler func(k, v string) bool)

        SpanID() string
        TraceID() string
}

We can probably add two simple method SpanID() and TraceID() to the SpanContext.

@jcchavezs

This comment has been minimized.

Copy link
Contributor

jcchavezs commented Jun 7, 2018

@gianarb

This comment has been minimized.

Copy link
Author

gianarb commented Jun 7, 2018

That's a good idea @jcchavezs thanks for sharing your experience. I am gonna do that.
But we probably still need to implement the updated spec I think.

Btw storing another tag that behaves like the trace_id is gonna double the size of the index, so at the moment I don't care about the format over to efficiency in the storage. (this obviously depends on storage and tracer but opentracing is ahead of them).

@rafaeljesus

This comment has been minimized.

Copy link

rafaeljesus commented Jun 7, 2018

Some api gateways like traefik for ex opted for using opentracing/jaeger rather than X-Request-Id when proxying request downstream. In my case I want to retrieve the trace ID in the header response making pretty easy to search in Jaeger UI. Here is the work around I came up with:

func NewTracerRequest(next http.Handler) http.Handler {
	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		var span opentracing.Span
		// Attempt to join a trace by getting trace context from the headers.
		carrier := opentracing.HTTPHeadersCarrier(r.Header)
		wireContext, err := opentracing.GlobalTracer().Extract(opentracing.HTTPHeaders, carrier)
		if err != nil {
			span = opentracing.StartSpan(r.URL.Path)
		} else {
			span = opentracing.StartSpan(r.URL.Path, opentracing.ChildOf(wireContext))
		}
		defer span.Finish()

                // code omitted

                span.SetTag("request.id", r.Header.Get("X-Request-Id"))

                // it adds the trace ID to the http headers                
		if err := span.Tracer().Inject(span.Context(), opentracing.HTTPHeaders, carrier); err != nil {
			ext.Error.Set(span, true)
		} else {
			r = r.WithContext(opentracing.ContextWithSpan(r.Context(), span))
		}

		traceID := r.Header.Get("Uber-Trace-Id")
		if traceID != "" {
			traceID = strings.Split(traceID, ":")[0]
			w.Header().Set("X-Trace-Id", traceID)
                         // adds traceID to a context and get from it latter
			r = r.WithContext(tracer.WithContextValue(r.Context(), traceID))
		}

		next.ServeHTTP(w, r)
	})
}

With this approach, being able to join the trace or not the caller will always have the traceID in the response Header

@rafaeljesus

This comment has been minimized.

Copy link

rafaeljesus commented Jun 7, 2018

You can also use @opentracing/basictracer-go

		if sc, ok := span.Context().(basictracer.SpanContext); ok {
			 sc.TraceID
		}

Update: https://godoc.org/github.com/opentracing/basictracer-go#SpanContext

@gianarb

This comment has been minimized.

Copy link
Author

gianarb commented Jun 7, 2018

Probably I am lost, I am just trying to upgrade the opentracing-go lib to catch up with the specification.
In this way, I am good without adding any library.

@rafaeljesus

This comment has been minimized.

Copy link

rafaeljesus commented Jun 7, 2018

sounds reasonable, I am neither adding any other lib just to grab the traceID, I wasn't aware of the spec docs you've shared, sound good tho

@yurishkuro

This comment has been minimized.

Copy link
Member

yurishkuro commented Jun 7, 2018

The main concern is that extending the SpanContext interface in this way is a breaking change that requires a major rev of OpenTracing API. We could implement the identifiers via a separate interface to which the span context could be cast dynamically.

@rafaeljesus

This comment has been minimized.

Copy link

rafaeljesus commented Jun 7, 2018

This is also possible if one is using jaeger and for me works pretty well

		if sc, ok := span.Context().(jaeger.SpanContext); ok {
			sc.TraceID()
		}
@gianarb

This comment has been minimized.

Copy link
Author

gianarb commented Jun 7, 2018

I understand your concern but doing that we are kind of allowing to have 2 version of the ScanContext probably but it is not what the spec says.

But I don't know how you manage this thing. I thought for a second that opentracing-go should be the representation of the spec. But I also understand your concern @yurishkuro .

@yyt030

This comment has been minimized.

Copy link

yyt030 commented Jun 16, 2018

@gianarb @yurishkuro
So now, I also have the require to print TraceID and SpanID into log files, but not any method or property to do it.

@tedsuo

This comment has been minimized.

Copy link
Member

tedsuo commented Jul 11, 2018

If TraceId and SpanID are already in use by jaeger, but with different return types, then we have to pick a different name for the equivalent methods we add to SpanContext. Since this is a cross-language decision, we should continue the debate in the specification repo (and on gitter):
opentracing/specification#123

@jcchavezs

This comment has been minimized.

Copy link
Contributor

jcchavezs commented Jul 11, 2018

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