-
Notifications
You must be signed in to change notification settings - Fork 368
add trace_id to goroutine context #3620
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
base: master
Are you sure you want to change the base?
Conversation
proxy/proxy.go
Outdated
traceID := tracing.GetTraceID(span) | ||
|
||
gCtx := stdlibcontext.Background() | ||
|
||
defer pprof.SetGoroutineLabels(gCtx) | ||
labels := pprof.Labels("trace_id", traceID) | ||
gCtx = pprof.WithLabels(gCtx, labels) | ||
pprof.SetGoroutineLabels(gCtx) | ||
|
||
ctx.goCtx = &gCtx | ||
|
||
traceIDCtx, _ := pprof.Label(gCtx, "trace_id") | ||
ctx.Logger().Debugf("trace id in goroutine context: %s", traceIDCtx) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't need to store ctx.goCtx, i.e. just do:
labels := pprof.Labels("trace_id", tracing.GetTraceID(span))
pprof.SetGoroutineLabels(pprof.WithLabels(stdlibcontext.Background(), labels))
defer pprof.SetGoroutineLabels(stdlibcontext.Background())
From https://pkg.go.dev/runtime/pprof#SetGoroutineLabels we read
SetGoroutineLabels sets the current goroutine's labels to match ctx. A new goroutine inherits the labels of the goroutine that created it.
So to test this we can create a testproxy with a mock tracer and a test filter that spawns a goroutine and verifies it can obtain trace_id via https://pkg.go.dev/runtime/pprof#Label. See TestFilterPanic for how to create and register a test filter.
Signed-off-by: Greeshma Mathew <[email protected]>
Signed-off-by: greeshma1196 <[email protected]>
Signed-off-by: greeshma1196 <[email protected]>
Updates: #3596