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

module/apmotel: setSpanAttributes may panic #1490

Closed
axw opened this issue Jul 14, 2023 · 1 comment · Fixed by #1512
Closed

module/apmotel: setSpanAttributes may panic #1490

axw opened this issue Jul 14, 2023 · 1 comment · Fixed by #1512
Assignees
Labels
agent-go bug Something isn't working

Comments

@axw
Copy link
Member

axw commented Jul 14, 2023

s.span.Type = "external"

panic({0x1c00800, 0x2fda980})                                                                                                                                                                                                                                                     
        runtime/panic.go:890 +0x263                                                                                                      
go.elastic.co/apm/module/apmotel/v2.(*span).setSpanAttributes(0xc00d5a5ee0)                                     
        go.elastic.co/apm/module/apmotel/[email protected]/span.go:252 +0x457                          
go.elastic.co/apm/module/apmotel/v2.(*span).End(0xc00d5a5ee0, {0x0?, 0x0, 0x40bb56?})                                          
        go.elastic.co/apm/module/apmotel/[email protected]/span.go:95 +0x485                                   
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp.(*wrappedBody).Close(0xc0115fe520)                                
        go.opentelemetry.io/contrib/instrumentation/net/http/[email protected]/transport.go:188 +0x30
github.com/elastic/elastic-transport-go/v8/elastictransport.(*Client).Perform(0xc011c123c0, 0xc00f186500)                  
        github.com/elastic/elastic-transport-go/[email protected]/elastictransport/elastictransport.go:413 +0xcb5      
github.com/elastic/go-elasticsearch/v8.(*BaseClient).Perform(0xc001d6e5f0, 0xc00f186500)
        github.com/elastic/go-elasticsearch/[email protected]/elasticsearch.go:321 +0x471

setSpanAttributes should only be called if the span is non-nil. I think this might happen if the span's End method is called twice, since the span field is never set to nil.

@axw axw added the bug Something isn't working label Jul 14, 2023
@vnarek
Copy link

vnarek commented Sep 22, 2023

I also have this issue and I can provide code that reproduces this if needed. I can probably also look into this if you are interested.

EDIT: here is the code

package main

import (
	"context"
	"io"
	"net/http"
	"net/http/httptrace"
	"net/url"

	"go.elastic.co/apm/module/apmotel/v2"
	"go.elastic.co/apm/v2"
	"go.elastic.co/apm/v2/transport"
	"go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace"
	"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
	"go.opentelemetry.io/otel/trace"
)

func main() {
	client := http.DefaultClient
	tp, err := createAPMTracerProvider(client, "development", "apm-url")
	if err != nil {
		panic(err)
	}
	client, err = enhanceHTTPClientWithOTEL(client, tp)
	if err != nil {
		panic(err)
	}

	ctx, span := tp.Tracer("todo").Start(context.Background(), "test span", trace.WithSpanKind(trace.SpanKindClient))
	defer span.End()
	//resp, httpResp, err := api.BookingFlowApi.GetBookingSession2(ctx, "4a071098-635a-4d4e-aa41-a5e7031cd8c9").Execute()
	req, err := http.NewRequestWithContext(ctx, http.MethodGet, "https://jsonplaceholder.typicode.com/todos/1", nil)
	if err != nil {
		panic(err)
	}
	resp, err := client.Do(req)
	if err != nil {
		panic(err)
	}
	io.ReadAll(resp.Body)
	resp.Body.Close()
}

func enhanceHTTPClientWithOTEL(client *http.Client, tp trace.TracerProvider) (*http.Client, error) {
	otelClient := *client
	otelClient.Transport = otelhttp.NewTransport(
		client.Transport,
		otelhttp.WithClientTrace(func(ctx context.Context) *httptrace.ClientTrace {
			return otelhttptrace.NewClientTrace(ctx, otelhttptrace.WithTracerProvider(tp))
		}),
	)
	return &otelClient, nil
}

func createAPMTracerProvider(client *http.Client, env, serverURL string) (trace.TracerProvider, error) {
	parsedURL, err := url.Parse(serverURL)
	if err != nil {
		return nil, err
	}
	httpTransport, err := transport.NewHTTPTransport(transport.HTTPTransportOptions{
		ServerURLs: []*url.URL{parsedURL},
	})
	if err != nil {
		return nil, err
	}
	httpTransport.Client = client

	tracer, err := apm.NewTracerOptions(apm.TracerOptions{
		ServiceName:        "b2b-platform",
		ServiceVersion:     "v2",
		Transport:          httpTransport,
		ServiceEnvironment: env,
	})
	if err != nil {
		return nil, err
	}

	tp, err := apmotel.NewTracerProvider(
		apmotel.WithAPMTracer(tracer),
	)
	return tp, err
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-go bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants