contrib/net/http: client and server both produce spans named http.request

[r3code]

I wonder why we do not split a http server and client requests span naming?

If we look at the grpc wrapper, we see that it has a name "grpc.client" and "grpc.server" But for http we see http.request for both.

My mind tells me all the time that client requests should have http.client naming. In a flame graph, I can only differentiate a server http request span from a client's one only by URL in the server span.
If we wrapped a http client that targets a different host, we won't have any information about which host the request was directed to.

I suggest we can use "http.client" for a http client requests and "http.server" (or http.request) for a server request

[felixge]

Thanks for the suggestion! cc-ing @DataDog/tracing-go to take a closer look.

[grosren]

We had to fork the Datadog tracing lib only because of this hard-coded http.request arg in roundertripper.go:l42 and trace.go:l66

We are using http.server and http.client respectively :)

Imho if a span is started by tracer.StartSpan(startOpts...) the operationName should be part of the StartSpanOptions or at least be able to be overwritten by StartSpanOptions.

[ajgajg1134]

Hello 👋 , this is something we're looking at doing for all language to drive better consistency. We'll keep in touch and leave this open :)

[ghost]

@ajgajg1134 In the meantime... can we at least have the option to customize the operation name in the setup of the client?

[ghost]

I think for now ive come up with a workaround.. i don't like it but needs must I guess..

func main() {
    tracer.Start(
        tracer.WithDebugMode(true),
        tracer.WithServiceName("custom-tripper"),
        tracer.WithEnv("braderz"),
        tracer.WithServiceVersion("1"),
    )

    if err := app(); err != nil {
        log.Fatal(err)
    }
    tracer.Flush()
    tracer.Stop()
}

func app() error {
    cl := http.Client{
        Timeout:   time.Hour,
        Transport: httptrace.WrapRoundTripper(&CustomTripper{http.DefaultTransport}),
    }
    resp, err := cl.Get("https://example.com")
    if err != nil {
        return err
    }
    defer resp.Body.Close()

    fmt.Printf("resp status: %s", resp.Status)
    return nil
}

func (tripper *CustomTripper) RoundTrip(req *http.Request) (*http.Response, error) {
    span, ok := tracer.SpanFromContext(req.Context())
    if !ok {
        return tripper.transport.RoundTrip(req)
    }
    span.SetOperationName("http.client")
    r2 := req.Clone(tracer.ContextWithSpan(req.Context(), span))
    return tripper.transport.RoundTrip(r2)
}

is there any update internally about the consistency of this?

[gaston-haro]

I've seen that some instrumentations are using web.request for incoming requests (a.k.a server) and http.request for outgoing (a.k.a client) requests. I personally found ok any of the naming conventions the problem for me is inconsistency.

[gaston-haro]

We have also ended up not relying on the native instrumentation for the http client due to this problem and have implemented our own which is against what we would like to do (we are currently using http.client.request as operation name)

[zarirhamza]

As an update, we do have an initiative to improve the span naming across the board for several libraries within the go tracer. Net/http is included in that and we do have an intention to provide distinct span names for http.server.request and http.client.request with a new version of span naming. @rarguelloF and I will keep this thread updated as we get more information.

[seanamos]

I went digging, and it does appear that there is a way to do this, but it's undocumented.

Internal to the dd-trace library, they have a concept of a namingschema and it is versioned. They use it to determine operation/span names, you can see it here: https://github.com/DataDog/dd-trace-go/blob/main/internal/namingschema/op.go

If you pay attention in there, for v0 you'll see http.request is used for both HTTPClient and HTTPServer. However, for v1 http.client.request is used for client and http.server.request for server.

To switch to the v1 naming schema you need to set an undocumented env var:

export DD_TRACE_SPAN_ATTRIBUTE_SCHEMA=v1