openzipkin / zipkin-go

Zipkin distributed tracing library for go.
Apache License 2.0
612 stars 114 forks source link

Change SpanName to depend on the request #183

Closed jcchavezs closed 1 year ago

jcchavezs commented 3 years ago

Currently the ServerOption named SpanName accepts a span name. While this serves for the purpose, I think accepting a function that accepts the request would allow users to do things like this when using mux:

        spanName = r.Method
    route := mux.CurrentRoute(r)
    if route != nil {
        var err error
        spanName, err = route.GetPathTemplate()
                ...
    }

In addition, accepting such function can still allow you to return a fixed name.

Ping @basvanbeek

basvanbeek commented 3 years ago

I'd much rather introduce a server option that allows to do more than just span name changes. We might want to include the possibility to influence sampling decision based on request details.

For the above example you already have the option to update the span name span.SetName(spanName)

codefromthecrypt commented 3 years ago

to do that bas, you'd need to collect all potential inputs to the span name function and send that to the server, right? in brave, we don't special case span name, it is part of overall request -> span parsing. to simplify things, we make it easy to write tag parsers and have people write their own request -> span parser instead of punching holes just for span name. OTOH, if folks both want to and can afford to send more tags to make a late span name decision out of process, they can also do that with the same function.

codefromthecrypt commented 3 years ago

so I would suggest that while I do feel adding more options for span-name-only is a bit of digging a hole (as would be different for messaging etc and better to focus on req/res), it would also be decoupled any change here. for example, with a more generic req/res function even you can send reformatted data to SaaS you can't control, but have naming policies ex aws and gcp