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

offer TracedContext(ctx context.Context) bool func #5348

Closed
someview opened this issue May 11, 2024 · 3 comments
Closed

offer TracedContext(ctx context.Context) bool func #5348

someview opened this issue May 11, 2024 · 3 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@someview
Copy link

someview commented May 11, 2024

I have tried some way to infer if we should trace the request before using tracer.Start method.

const currentSpanKey = 0
func TracedContext(ctx context.Context) bool {
        val := ctx.Value(currentSpanKey)
	_, ok := val.(Span)
	return ok
}

But currentSpanKey is not exposed. How about expose func to do this?

@someview someview added the enhancement New feature or request label May 11, 2024
@someview
Copy link
Author

someview commented May 13, 2024

There is some benchmark result about trace func:

func TracedContext(ctx context.Context) bool {
	return trace.SpanContextFromContext(ctx).IsSampled()
}

// goos: windows
// 2024/05/11 17:38:50 maxprocs: Leaving GOMAXPROCS=4: CPU quota undefined
// goarch: amd64
// pkg: gitlab.dev.wiqun.com/tl/goserver/chat/l1/tl.goapm.git/v2
// cpu: Intel(R) Core(TM) i3-9100F CPU @ 3.60GHz
// BenchmarkTracedContext
// BenchmarkTracedContext-4        127979276                9.480 ns/op
// PASS
func BenchmarkTracedContext(b *testing.B) {
	ctx := context.Background()
	for i := 0; i < b.N; i++ {
		_ = TracedContext(ctx)
	}
}

// goos: windows
// goarch: amd64
// pkg: gitlab.dev.wiqun.com/tl/goserver/chat/l1/tl.goapm.git/v2
// cpu: Intel(R) Core(TM) i3-9100F CPU @ 3.60GHz
// BenchmarkTracedContext_Nested
// BenchmarkTracedContext_Nested-4         77304644                14.95 ns/op
// PASS
func BenchmarkTracedContext_Nested(b *testing.B) {
	root := context.Background()
	p1, _ := context.WithTimeout(root, time.Second*3)
	p2 := context.WithValue(p1, 0, "123")
	p3, _ := context.WithTimeout(p2, time.Second*3)
	p4 := context.WithValue(p3, 1, "123")
	b.ResetTimer()
	for i := 0; i < b.N; i++ {
		_ = TracedContext(p4)
	}
}

// goos: windows
// goarch: amd64
// pkg: gitlab.dev.wiqun.com/tl/goserver/chat/l1/tl.goapm.git/v2
// cpu: Intel(R) Core(TM) i3-9100F CPU @ 3.60GHz
// 2024/05/11 17:32:20 maxprocs: Leaving GOMAXPROCS=4: CPU quota undefined
// BenchmarkTracer_Start
// BenchmarkTracer_Start-4          9982496               114.9 ns/op
// PASS
func BenchmarkTracer_StartEnd(b *testing.B) {
	ins := &tracer{Tracer: newNoopTracer()}
	for i := 0; i < b.N; i++ {
		_, span := ins.Start(context.Background(), "test")
		span.End()
	}
}

But if we using this method to infer traceable:

const currentSpanKey = 0
func TracedContext(ctx context.Context) bool {
        val := ctx.Value(currentSpanKey)
	_, ok := val.(Span)
	return ok
}
``
the op/s is just 0.3op/s, 1/300 of noopTracer.  1/30 of  `trace.SpanContextFromContext(ctx).IsSampled()`
In most case, the trace ratio would not be little. We do not need use `start-end` pattern in every request.  
If(TracedContext(ctx context.Context)) {
    ctx,span:=tracer.Start(ctx,"opname")
    span.XXX  // span method
    defer span.close()
}


@dmathieu
Copy link
Member

You're looking for trace.SpanFromContext.

if trace.SpanFromContext(ctx).IsRecording() {
    // Parent span is recording
    // Which does not mean a newly created one will be recording too. Sampling decisions could be made differently.
}

@MrAlias MrAlias added the question Further information is requested label May 13, 2024
@MrAlias
Copy link
Contributor

MrAlias commented May 13, 2024

You're looking for trace.SpanFromContext.

if trace.SpanFromContext(ctx).IsRecording() {
    // Parent span is recording
    // Which does not mean a newly created one will be recording too. Sampling decisions could be made differently.
}

Closing as this appears to answer this.

@MrAlias MrAlias closed this as not planned Won't fix, can't repro, duplicate, stale May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants