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

Eventhub Opentelemetry Trace exception: "'MessageProperties' object has no attribute 'setdefault'" #27986

Closed
erewok opened this issue Dec 16, 2022 · 7 comments
Assignees
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Event Hubs issue-addressed The Azure SDK team member assisting with this issue believes it to be addressed and ready to close.
Milestone

Comments

@erewok
Copy link

erewok commented Dec 16, 2022

  • Package Name: azure-eventhub
  • Package Version: 5.10.1
  • Operating System: Linux-Debian (docker container)
  • Python Version: 3.11.1

Describe the bug
Today in an experiment with adding tracing spans to our EventhubProducerClient calls, I followed a helpful example provided by the package azure-core-tracing-opentelemetry.

After some tracing spans were produced, I noticed an exception in the logs (but these did not prevent events from being published):

Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/opentelemetry/trace/__init__.py", line 573, in use_span
    yield span
  File "/usr/local/lib/python3.11/site-packages/azure/eventhub/_utils.py", line 181, in trace_message
    event.properties.setdefault(
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'MessageProperties' object has no attribute 'setdefault'

In the _utils.py module, there's a function called trace_message with a few lines like this:

                if not event.properties:
                    event.properties = dict()
                event.properties.setdefault(
                    b"Diagnostic-Id", message_span.get_trace_parent().encode("ascii")
                )

Here, if event.properties is truthy, then it will not be a Python dict (as in my case), and instead it's an instance of MessageProperties from the uamqp library, which has no setdefault function.

To fix it, we could indent setdefault under the if not event.properties and add an else that sets the property differently for MessageProperties.

I would be happy to submit a PR for this change if it is useful.

To Reproduce
Steps to reproduce the behavior:

  1. Follow the example for instrumenting an EventhubProducerClient
  2. Produce some events.
  3. Load tracing span waterfall graph matching this trace id.
  4. See exception screenshot below

Screenshots

Screenshot 2022-12-16 at 12 04 03 PM

Expected behavior
No exceptions when instrumenting Eventhub calls with opentelemetry tracing spans.

@ghost ghost added customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Dec 16, 2022
@github-actions github-actions bot added the needs-triage This is a new issue that needs to be triaged to the appropriate team. label Dec 16, 2022
@kristapratico kristapratico added Event Hubs Client This issue points to a problem in the data-plane of the library. and removed needs-triage This is a new issue that needs to be triaged to the appropriate team. labels Dec 16, 2022
@ghost ghost added the needs-team-attention This issue needs attention from Azure service team or SDK team label Dec 16, 2022
@kristapratico
Copy link
Member

Hi @erewok thanks for the detailed issue, we'll take a look and get back to you as soon as possible.

@swathipil swathipil added bug This issue requires a change to an existing behavior in the product in order to be resolved. and removed question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Dec 19, 2022
@swathipil swathipil added this to the 2023-01 milestone Dec 19, 2022
@swathipil
Copy link
Member

swathipil commented Dec 21, 2022

Hi @erewok - Thanks for the investigation! It was very helpful in helping reproduce/track down the bug. We've made a few big changes recently to the SDK, so the fix will be a little bit more complex. We're working on it right now and will keep you posted on any updates. Thanks!

@erewok
Copy link
Author

erewok commented Dec 21, 2022

@swathipil, great! Glad to hear it and glad that this issue was useful. Thanks for following up.

ghost pushed a commit that referenced this issue Jan 6, 2023
fixes bug in: #27986

changes:
- removed duplicate tracing which was adding the `Azure.EventHubs.Message` span twice when sending a batch message/list of messages.
- adding the tracing info the uamqp/pyamqp.Message application_properties rather than event_data. Ensures that the diagnostic ID is actually being added on the outgoing Message, rather than the EventData after outgoing Message has already been created.
- Adding the FakeSpan class to `tests` folder to test telemetry.

TODO:
- follow-up issue created to address implementation so it behaves correctly in a separate PR: #28141
@swathipil
Copy link
Member

Hi @erewok - The fix for this bug has been merged and will be included in the upcoming release: azure-eventhub 5.11.0.

If you'd like to test the fix in the meantime, you can build and install the wheel by:

  1. Cloning the azure-sdk-for-python repo.
  2. Running pip install wheel
> cd azure-sdk-for-python/sdk/eventhub/azure-eventhub
azure-sdk-for-python/sdk/eventhub/azure-eventhub> python .\setup.py bdist_wheel -d <dest_folder>

Thanks!

@swathipil swathipil added the issue-addressed The Azure SDK team member assisting with this issue believes it to be addressed and ready to close. label Jan 6, 2023
@ghost
Copy link

ghost commented Jan 6, 2023

Hi @erewok. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text “/unresolve” to remove the “issue-addressed” label and continue the conversation.

@ghost ghost removed the needs-team-attention This issue needs attention from Azure service team or SDK team label Jan 6, 2023
@erewok
Copy link
Author

erewok commented Jan 6, 2023

Thanks for the update, @swathipil! I will wait until the latest version is up on PyPi. I appreciate the help.

@ghost
Copy link

ghost commented Jan 13, 2023

Hi @erewok, since you haven’t asked that we “/unresolve” the issue, we’ll close this out. If you believe further discussion is needed, please add a comment “/unresolve” to reopen the issue.

@ghost ghost closed this as completed Jan 13, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Apr 13, 2023
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. Event Hubs issue-addressed The Azure SDK team member assisting with this issue believes it to be addressed and ready to close.
Projects
None yet
Development

No branches or pull requests

4 participants