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.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
docs: add ioredis example #665
docs: add ioredis example #665
Changes from 5 commits
c0c64f5
065c198
dc678d9
ee14dd3
aa877f4
9a7c7ec
8861816
0143bc0
b9d544b
5c86eec
4a097ca
eae9b86
a712024
e11fd09
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dyladan should this now be
module.exports = opentelemetry.getTracerRegistry();
?Should the file be renamed to
tracer-registry.js
in this case?As a side note I find this whole tracer registry/factory thing to be confusing for the end user. I wish the focus was that libs just not have broken implementations of tracers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that means it becomes tracer registry.
"just don't have bugs" is not a feasible answer to the problem unfortunately.
Fortunately, most end users won't have to worry about it too much, just library developers. The only change for most end users is the name of the global object they need to create. Most users will never call any methods on the tracer registry manually.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for explaining.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dyladan I had not yet made the above fix. Do you want a follow up PR for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can make a follow up PR to handle it, but you're not actually using the exported value in your index, so it shouldn't matter.