Updates Client to create Bunny::Consumer with queue obj, not string #30
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change should not result in any change in functionality. The motivation behind introducing this change is two-fold: 1. The official Bunny documentation shows multiple examples where a Bunny::Consumer object is initialized with a Bunny::Queue object as the queue parameter, and none where the queue parameter is a string literal. Although the string parameter is documented and supported by the bunny gem, the examples given and community usage seems to show a preference for passing a Queue object. 2. The OpenTelemetry bunny instrumentation gem is currently incompatible with the current version of bunny_burrow. The OTel gem works by patching a number of methods in Bunny classes, and expects to be able to call `queue.channel` inside its patch of Bunny::Consumer#call`. This results in a runtime error with the current versions of bunny_burrow and the OTel gem. See https://github.com/open-telemetry/opentelemetry-ruby-contrib/blob/main/instrumentation/bunny/lib/opentelemetry/instrumentation/bunny/patches/consumer.rb#L14 Regarding point 2 above, I acknowledge that the "better" course of action would probably be to file a bug report with the OTel bunny instrumentation gem. In my opinion, the authors of the gem made an incorrect assumption about the underlying Bunny gem that contradicts the latter's own documentation. That said, I don't believe there's any harm in making this change to bunny_burrow AND it is the quicker course of action. Since this issue is currently a blocker to being able to use the OTel bunny instrumentation gem with a project that uses bunny_burrow, I decided to propose this fix.
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.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change should not result in any change in functionality.
The motivation behind introducing this change is two-fold:
queue.channelinside its patch ofBunny::Consumer#call. This results in a runtime error with the current versions of bunny_burrow and the OTel gem. See https://github.com/open-telemetry/opentelemetry-ruby-contrib/blob/main/instrumentation/bunny/lib/opentelemetry/instrumentation/bunny/patches/consumer.rb#L14Regarding point 2 above, I acknowledge that the "better" course of action would probably be to file a bug report with the OTel bunny instrumentation gem. In my opinion, the authors of the gem made an incorrect assumption about the underlying Bunny gem that contradicts the latter's own documentation.
That said, I don't believe there's any harm in making this change to bunny_burrow AND it is the quicker option. Since this issue is currently a blocker to being able to use the OTel bunny instrumentation gem with a project that uses bunny_burrow, I decided to propose this
fixchange.