-
Notifications
You must be signed in to change notification settings - Fork 35
[OTel] Pika instrumentation #614
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
Conversation
50fa3c1 to
9059ca4
Compare
820b226 to
2b4a16f
Compare
9059ca4 to
2b37e3b
Compare
2b37e3b to
d4026eb
Compare
GSVarsha
left a comment
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.
Few requests.
d4026eb to
55d3cda
Compare
GSVarsha
left a comment
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.
Can we move all the imports inside the try block so that if pika is not present we can just completely avoid importing everything else?
CagriYonca
left a comment
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.
LGTM
All of them are inside the try-except block. Could you please re-check it? |
55d3cda to
84f195f
Compare
I meant these ones. |
Got it. We did not follow this pattern at the beginning of the instrumentation migration, but I guess we can follow it now. I will change it and let you know. |
84f195f to
bbf443a
Compare
Signed-off-by: Paulo Vital <paulo.vital@ibm.com>
Signed-off-by: Paulo Vital <paulo.vital@ibm.com>
bbf443a to
c175b4d
Compare
GSVarsha
left a comment
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.
Looks good to me.
Test coverage: 98%