Skip to content

Conversation

@pvital
Copy link
Member

@pvital pvital commented Sep 10, 2024

Test coverage: 98%

@pvital pvital self-assigned this Sep 10, 2024
@pvital pvital added the OTel_migration Migration the code dependency from OpenTracing to OpenTelemetry label Sep 10, 2024
@pvital pvital force-pushed the otel_instrumentation_pika branch 2 times, most recently from 50fa3c1 to 9059ca4 Compare September 12, 2024 09:47
@pvital pvital force-pushed the otel_instrumentation_pika branch from 9059ca4 to 2b37e3b Compare September 12, 2024 13:53
@pvital pvital force-pushed the otel_instrumentation_pika branch from 2b37e3b to d4026eb Compare September 17, 2024 01:42
@pvital pvital requested a review from CagriYonca September 17, 2024 01:42
Copy link
Contributor

@GSVarsha GSVarsha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few requests.

@pvital pvital force-pushed the otel_instrumentation_pika branch from d4026eb to 55d3cda Compare September 17, 2024 07:47
@pvital pvital requested a review from GSVarsha September 17, 2024 07:47
Copy link
Contributor

@GSVarsha GSVarsha left a 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?

Copy link
Contributor

@CagriYonca CagriYonca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pvital
Copy link
Member Author

pvital commented Sep 18, 2024

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?

All of them are inside the try-except block. Could you please re-check it?

@pvital pvital force-pushed the otel_instrumentation_pika branch from 55d3cda to 84f195f Compare September 18, 2024 12:56
@GSVarsha
Copy link
Contributor

All of them are inside the try-except block. Could you please re-check it?

I meant these ones.

@pvital
Copy link
Member Author

pvital commented Sep 19, 2024

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.

@pvital pvital force-pushed the otel_instrumentation_pika branch from 84f195f to bbf443a Compare September 19, 2024 05:24
@pvital pvital requested a review from GSVarsha September 19, 2024 05:27
Signed-off-by: Paulo Vital <paulo.vital@ibm.com>
Signed-off-by: Paulo Vital <paulo.vital@ibm.com>
@pvital pvital force-pushed the otel_instrumentation_pika branch from bbf443a to c175b4d Compare September 19, 2024 12:32
Copy link
Contributor

@GSVarsha GSVarsha left a 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.

@pvital pvital merged commit b55e3fe into otel_migration Sep 20, 2024
@pvital pvital deleted the otel_instrumentation_pika branch September 20, 2024 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

OTel_migration Migration the code dependency from OpenTracing to OpenTelemetry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants