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

Allow customizing the span name #4

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

pdobacz
Copy link

@pdobacz pdobacz commented Jan 26, 2021

Hello, thank you for sharing the library.

I'm proposing a slight addition, whereby the middleware client can override the span name to a fixed struct, instead of the request path, which remains the default.

This is useful, when requests being made have IDs in the paths, and those end up in span names. E.g. Jaeger will interpret this as the operation, which can be filtered on, but it is rendered useless if there's an operation per every ID.

@pdobacz
Copy link
Author

pdobacz commented Jan 27, 2021

@tsloughter, I forgot to tag, please take a look, when you have a moment, thank you 🙇

@tsloughter
Copy link
Member

Should this be a function? It is set when the middleware is added, right? So you'd want a call like span_name_override(path) to be called that returns the span name.

@pdobacz
Copy link
Author

pdobacz commented Jan 27, 2021

Should this be a function? It is set when the middleware is added, right? So you'd want a call like span_name_override(path) to be called that returns the span name.

yea, makes sense, I think I over-simplified this, lemme make the necessary changes

@pdobacz
Copy link
Author

pdobacz commented Feb 1, 2021

@tsloughter Hello again, I did the modification requested. I also did a very simple test for this option. It's still pretty rudimentary, but I hope enough for the time being.

@garthk
Copy link

garthk commented Jun 28, 2021

Thanks for adding a test! Could we call the function with env instead so we can configure it to output "HTTP #{env.method}" matching the OpenTelemetry semantic attributes spec?

Therefore, HTTP client spans SHOULD be using conservative, low cardinality names formed from the available parameters of an HTTP request, such as "HTTP {METHOD_NAME}". Instrumentation MUST NOT default to using URI path as span name, but MAY provide hooks to allow custom logic to override the default span name.

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.

3 participants