Add a span name getter to opencensus endpoint options #948
Conversation
|
What are your thoughts about this? Is this what you had in mind? |
sagikazarmark
force-pushed the
sagikazarmark:opencensus-options
branch
from
f972dce
to
e860742
|
@basvanbeek I've updated the PR based on your comments. However, I feel it would be better to split the function into two:
More functions, more work, but they are more expressive IMHO. |
|
We can do the split... we can also say that if a returned name is empty on a call to |
|
Updated the PR accordingly. If you have any ideas for better naming, let me know. |
sagikazarmark
force-pushed the
sagikazarmark:opencensus-options
branch
from
c999092
to
b27801a
|
Logic looks good to me with one nit as shown above. Also unsure of method naming. @peterbourgon any ideas? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
This is an experimental PR suggested by @basvanbeek in #946 (comment)
It adds a span name getter function to the opencensus endpoint tracer config.
Other options might worth adding:
See the opencensus ochttp plugin for more.