Skip to content
This repository has been archived by the owner on May 23, 2024. It is now read-only.

Support externally-provided trace/span IDs #397

Closed
xbsura opened this issue Jul 1, 2019 · 8 comments · Fixed by #411
Closed

Support externally-provided trace/span IDs #397

xbsura opened this issue Jul 1, 2019 · 8 comments · Fixed by #411

Comments

@xbsura
Copy link

xbsura commented Jul 1, 2019

Problem

I need to use jaeger-client-go daeling with some existed trace Log and uload them to jager agent

As exiting Log has had traceId, spanId and so on, so I need to NewTrace and NewSpan all by myself, in Python, I can create a span with a existed spanContex, but in Go, I can not find such method

jaeger-client-go has a NewSpanContext method, but not method named: Span.SetContext() Or NewSpan()

So could you please add a method: Span.SetContext or Newspan(SpanContext, Tracer, operationName, tags, startTime, references)?

thanks

@yurishkuro yurishkuro changed the title client has problem when dealing with log tracer Support externally-provided trace/span IDs Jul 5, 2019
dm03514 added a commit to dm03514/jaeger-client-go that referenced this issue Aug 2, 2019
@dm03514
Copy link
Contributor

dm03514 commented Aug 2, 2019

@yurishkuro Would love your feedback:

dm03514@efd30c9

POC just to get design talk going.

I appreciate your time.


So far this pulls out the references building and parent detection into filterReferences

It exposes another span constructor (StartSpanFromContext ) on the tracer.

@yurishkuro
Copy link
Member

@dm03514 this looks like its copying a bunch of code that already exists in this repo

I think a better approach is to introduce a new non-standard type of span reference SELF (discussed here opentracing/specification#81). If such reference is passed to the through the child and ID generation logic.

dm03514 added a commit to dm03514/jaeger-client-go that referenced this issue Aug 3, 2019
@dm03514
Copy link
Contributor

dm03514 commented Aug 3, 2019

@yurishkuro another POC, would love your feedback.

dm03514@bf5f342

Thank you

@yurishkuro
Copy link
Member

looks reasonable. It's hard to review, could you create a WIP PR?

@dm03514 dm03514 mentioned this issue Aug 3, 2019
5 tasks
dm03514 added a commit to dm03514/jaeger-client-go that referenced this issue Aug 4, 2019
refs jaegertracing#397

Signed-off-by: Daniel Mican <dm03514@gmail.com>
yurishkuro pushed a commit that referenced this issue Aug 7, 2019
* POC: Self Referenced Span

refs #397

Signed-off-by: Daniel Mican <dm03514@gmail.com>

* Self Ref Span: Moves SelfRef -> constants

Signed-off-by: Daniel Mican <dm03514@gmail.com>

* SelfRef: removes extra pointer

Signed-off-by: Daniel Mican <dm03514@gmail.com>

* SelfRef: README

Signed-off-by: Daniel Mican <dm03514@gmail.com>

* SelfRef: README

Signed-off-by: Daniel Mican <dm03514@gmail.com>

* SelfRef: startSpan unit test

Signed-off-by: Daniel Mican <dm03514@gmail.com>

* SelfRef: startSpanWithOptions check for selRef before tracking references.

Provides a selfRef factory method hidningn constant/implementation details

Signed-off-by: Daniel Mican <dm03514@gmail.com>

* SelfRef: Lint

Signed-off-by: Daniel Mican <dm03514@gmail.com>

* SelfRef: README update

Signed-off-by: Daniel Mican <dm03514@gmail.com>

* SelfRef: Rebuild, github timeout

Signed-off-by: Daniel Mican <dm03514@gmail.com>

* SelfRef: Rebuild

Signed-off-by: Daniel Mican <dm03514@gmail.com>
@dm03514
Copy link
Contributor

dm03514 commented Aug 13, 2019

👋 @yurishkuro

Just to update this is working perfectly for my use case! Thank you again. My use case is long lived (hours, days, weeks) of low traffic (100's to 1000's / day) spans. Because of how long they live I needed to shuffle them out of memory. Right now I am exporting the operation name, SpanContext ID as string, tags and start time:

...
		s, err := json.Marshal(SerializableJaegerSpan{
			OperationName: span.OperationName(),
			ID:            span.String(),
			Tags:          span.Tags(),
			StartTime:     span.StartTime(),
		})
...
		ss.SpanContext = s
		ss.Backend = Jaeger
...

Then when a span needs to be finished/updated it is loaded back and the context is rebuilt:

func (s SerializableJaegerSpan) Context() (jaeger.SpanContext, error) {
	return jaeger.ContextFromString(s.ID)
}
...

js := SerializableJaegerSpan{}
...
ctx, err := js.Context()
...
span := tracer.StartSpan(
			js.OperationName,
			jaeger.SelfRef(ctx),
			opentracing.StartTime(js.StartTime),
		)
		for k, v := range js.Tags {
			span.SetTag(k, v)
		}

Is there anything I could do to help with opentracing/specification#81 ? Is it possible to revisit it? I personally could benefit from SELF being part of the spec (for lightstep & dd-trace). I read that adriancole said the issue predates the current process, so i'm not sure what step to take next. Thanks again: Danny

@yurishkuro
Copy link
Member

Have you tried actually finishing the span more than once? Jaeger should be able to handle that and merge multiple span records into one (not sure what it would do with timestamps). This way you wouldn't need to save the tags, and in fact you might be able to operate purely in the OpenTracing API (where things like span.Tags() are not accessible).

Regarding the process in OpenTracing, I think there needs to be an RFC proposed. But so far you only solved your use case by accessing a lot of Jaeger-specific methods, so simply adding SELF to OpenTracing wouldn't solve the issue without requiring the additional merging behavior.

@dm03514
Copy link
Contributor

dm03514 commented Aug 13, 2019

I will experiment with the multiple finishes.

I noticed that issue with Tags() when looking at other Go tracing libraries. I'm using jaeger as backend so it isn't super huge for me right now, but would def be awesome to have a solution that was fully opentracing compliant. I see what you're saying: there's really no generic SELF benefit for this use case since the serialize/deserialize method requires jaeger specific calls.

@yurishkuro
Copy link
Member

Resolved by #411.

yurishkuro added a commit that referenced this issue Feb 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants