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
Add Jaeger exporter #534
Add Jaeger exporter #534
Changes from 6 commits
7cd462f
62746e5
9fd15a4
2081396
fcb82b7
eacbe0b
5c09997
02b3bbd
a3a08ad
5d8140b
ca66fed
8a7fa9f
f7f6d79
64672ba
dbed4aa
4bde77a
0a64e80
6cbffcf
d09aa9b
fd3d26f
04581ee
0d74fac
f337341
e58b00b
843e2c6
accce34
05a7f42
adb49c4
b443819
8e6c3a1
9a980ad
aeadf81
e1b93df
3d9597d
d7c3498
2027eb9
ec6ee47
6d09836
c962561
3f2ef72
e38dec6
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.
Nit: Not something which needs immediate attention, but probably we should have recordable names after transport types - eg
TUDPRecordable
. This is an issue with Zipkin exporter too.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.
Agree. I logged the same comment for ETW exporter. I think we could create a task for the cleanup for all the exporters? I'll try the renaming anyway.
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.
+1
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.
Created issue #735 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.
Why are these shared_ptr?
There's ref-counting overhead vs. unique_ptr. Are these actually shared?
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 Josh. Yes, I also prefer
unique_ptr
if possible. For the cases of usingshared_ptr
, that would be needed as it is required to be passed to other functions asshared_ptr
, so I just created it asshared_ptr
in the first place.