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

Use the request's uri as operation name for Span objects. #23

Conversation

carlosalberto
Copy link

Proposal for the uri as operation name ;)

@jakerobb
Copy link

jakerobb commented May 8, 2017

Thanks! I've been meaning to submit an issue for this. I think it would be better to provide an interface and a default implementation for determining an operation name. In the Zipkin world, this is what Brave does (see https://github.com/openzipkin/brave/blob/master/brave-http/src/main/java/com/github/kristofa/brave/http/DefaultSpanNameProvider.java and related).

My personal preference for HTTP calls is to provide method + the path portion of the URI, e.g. "GET /users/123".

@sjoerdtalsma
Copy link
Contributor

@jakerobb Surely /users/123 should be reduced to either something like /users/{id} or get_user?

From https://github.com/opentracing/specification/blob/master/specification.md#tracer:

An operation name, a human-readable string which concisely represents the work done by the Span (for example, an RPC method name, a function name, or the name of a subtask or stage within a larger computation). The operation name should be the most general string that identifies a (statistically) interesting class of Span instances. That is, "get_user" is better than "get_user/314159".

@objectiser
Copy link
Contributor

This issue is being more generally discussed here: opentracing-contrib/meta#25

Would be good to add any suggestions there, so they can be applied to all relevant framework integrations.

@pavolloffay
Copy link
Collaborator

@tedsuo can this be closed?

@tedsuo
Copy link

tedsuo commented Jun 5, 2017

Yes, closing as URI's turn out to be bad for operation names.

@tedsuo tedsuo closed this Jun 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants