zipkin-go icon indicating copy to clipboard operation
zipkin-go copied to clipboard

Change SpanName to depend on the request

Open jcchavezs opened this issue 3 years ago • 3 comments

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

jcchavezs avatar Nov 23 '20 21:11 jcchavezs

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)

basvanbeek avatar Nov 24 '20 09:11 basvanbeek

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 avatar Nov 24 '20 23:11 codefromthecrypt

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

codefromthecrypt avatar Nov 24 '20 23:11 codefromthecrypt