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

Can't mix native transaction with Otel's trace.SpanFromContext #1449

Closed
wzy9607 opened this issue May 30, 2023 · 3 comments · Fixed by #1450
Closed

Can't mix native transaction with Otel's trace.SpanFromContext #1449

wzy9607 opened this issue May 30, 2023 · 3 comments · Fixed by #1450
Assignees
Labels
agent-go enhancement New feature or request

Comments

@wzy9607
Copy link

wzy9607 commented May 30, 2023

Describe the bug

The OTel bridge have the same problem that the opentracing bridge once faced (#235 and opentracing/opentracing-go#192)

apm’s native transaction can't be retrieved by Otel’s trace.SpanFromContext(https://github.com/open-telemetry/opentelemetry-go/blob/b2246d58650bc6db04459353843250ee3bba77fc/trace/context.go#L48).

Since goredis (https://github.com/redis/go-redis/blob/dc8c93ba66777b8f9866542b47cd102f591b5984/extra/redisotel/tracing.go#L90) and maybe other libs use trace.SpanFromContext before starting a Span,
currently it is not possible to mix native apm instrumentation with those lib’s OTel instrumentation.

To Reproduce

package main

import (
	"context"
	"fmt"

	"go.elastic.co/apm/module/apmotel/v2"
	"go.elastic.co/apm/v2"
	"go.elastic.co/apm/v2/apmtest"
	"go.opentelemetry.io/otel"
	"go.opentelemetry.io/otel/trace"
)

func main() {
	tracer := apmtest.NewRecordingTracer()
	provider, err := apmotel.NewTracerProvider(apmotel.WithAPMTracer(tracer.Tracer))
	if err != nil {
		panic(err)
	}
	otel.SetTracerProvider(provider)

	ctx := context.Background()
	tx := tracer.StartTransaction("test1", "test")
	ctx = apm.ContextWithTransaction(ctx, tx)

	fmt.Printf("apm transaction: %#v\n", apm.TransactionFromContext(ctx))
	sp := trace.SpanFromContext(ctx)
	fmt.Printf("OTel span: %#v\n", sp) // expect to not be otel trace.noopSpan{}
}

Expected behavior

Should be able to retrieve Span using Otel’s trace.SpanFromContext from an apm transaction.

@axw axw added the enhancement New feature or request label May 30, 2023
@axw
Copy link
Member

axw commented May 30, 2023

Thanks for opening this issue @wzy9607, I agree this would be ideal - I have added it to our backlog of things to look into.

@wzy9607
Copy link
Author

wzy9607 commented May 31, 2023

I found that apm.TransactionFromContextapm.SpanFromContext also can't retrieve an Otel span started with by apmotel tracer.

To Reproduce

package main

import (
	"context"
	"fmt"

	"go.elastic.co/apm/module/apmotel/v2"
	"go.elastic.co/apm/v2"
	"go.elastic.co/apm/v2/apmtest"
	"go.opentelemetry.io/otel"
	"go.opentelemetry.io/otel/trace"
)

func main() {
	tracer := apmtest.NewRecordingTracer()
	provider, err := apmotel.NewTracerProvider(apmotel.WithAPMTracer(tracer.Tracer))
	if err != nil {
		panic(err)
	}
	otel.SetTracerProvider(provider)

	ctx := context.Background()
	ctx, _ = otel.Tracer("test").Start(ctx, "test1")

	fmt.Printf("apm transaction: %#v\n", apm.TransactionFromContext(ctx)) // expect to not be (*apm.Transaction)(nil)
	fmt.Printf("apm span: %#v\n", apm.SpanFromContext(ctx))               // expect to not be (*apm.Span)(nil)
	fmt.Printf("OTel span: %#v\n", trace.SpanFromContext(ctx))            // is &apmotel.span, as expected
}

@dmathieu
Copy link
Member

dmathieu commented Jun 2, 2023

This should be fully fixed by #1450.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-go enhancement New feature or request
Projects
None yet
3 participants