Skip to content
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

fix(bunny): headers property needs to exist #661

Merged
merged 2 commits into from
Sep 24, 2023
Merged

fix(bunny): headers property needs to exist #661

merged 2 commits into from
Sep 24, 2023

Conversation

maciekm
Copy link
Contributor

@maciekm maciekm commented Sep 7, 2023

bunny instrumentation errors when rabbitmq messages don't have headers property, e.g. published by an app that isn't instrumented yet.

E, [2023-09-07T12:01:06.688026 #1] ERROR -- #<Bunny::Session:0x29d88 hidden@hidden:5672, vhost=hidden, addresses=[hidden:5672]>: Uncaught exception from consumer #<Bunny::Consumer:101200 @channel_id=1 @queue=hidden @consumer_tag=hidden>: #<NoMethodError: undefined method `[]' for nil:NilClass 

carrier[key]
^^^^^> @ /gems/ruby/3.1.0/gems/opentelemetry-api-1.2.2/lib/opentelemetry/context/propagation/text_map_getter.rb:16:in `get'

you can reproduce the issue by spiting the example file into a publisher without instrumentation and a consumer with.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Sep 7, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@arielvalentin arielvalentin changed the title [bunny] fix: headers property needs to exist fix(bunny): headers property needs to exist Sep 19, 2023
Copy link
Collaborator

@arielvalentin arielvalentin left a comment

Choose a reason for hiding this comment

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

@maciekm I have not had much success in tracking down or getting feedback from the original contributors.

Would you be willing to take over as the maintainer of this instrumentation?

Also would you be able to look into why this instrumentation has multiple sets of Trace Context Propagation Carries in the properties i.e. headers and tracer_receive_headers?

@arielvalentin arielvalentin merged commit 0b560e2 into open-telemetry:main Sep 24, 2023
46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants