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

aio-flavor or azure.servicebus not working correctly with tracing spans #28738

Closed
futurwasfree opened this issue Feb 10, 2023 · 5 comments
Closed
Labels
customer-reported Issues that are reported by GitHub users external to the Azure organization. Messaging Messaging crew question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Service Bus

Comments

@futurwasfree
Copy link

futurwasfree commented Feb 10, 2023

  • Package Name: azure-servicebus
  • Package Version: 7.8.2
  • Operating System: Ubuntu 20.04
  • Python Version: 3.10

Describe the bug
I get the following warning for every received message on service bus:
/usr/local/lib/python3.10/site-packages/azure/servicebus/_common/utils.py:287: RuntimeWarning: coroutine 'BaseHandler._add_span_request_attributes' was never awaited

To Reproduce
Steps to reproduce the behavior:

import asyncio
from azure.servicebus.aio import ServiceBusClient
from azure.identity.aio import DefaultAzureCredential
from opentelemetry import trace
from opentelemetry.sdk.trace import TracerProvider
...
trace.set_tracer_provider(TracerProvider())
tracer = trace.get_tracer_provider().get_tracer(__name__)
...
  with tracer.start_as_current_span(name=endpoint_name):
                async with DefaultAzureCredential(
                    exclude_shared_token_cache_credential=True,
                    exclude_visual_studio_code_credential=True,
                ) as credential:
                    service_bus_client = ServiceBusClient(
                        fully_qualified_namespace=fully_qualified_service_bus_name,
                        credential=credential,
                    )
                    async with service_bus_client:
                        receiver = service_bus_client.get_subscription_receiver(
                            topic_name=topic_name
                            subscription_name=subscription_name,
                        )
                        async with receiver:
                            while True:
                                received_msgs = await receiver.receive_messages(
                                    max_message_count=1
                                )
 ...

Expected behavior
No warning given and trace recorded correctly.

Screenshots
If applicable, add screenshots to help explain your problem.

Additional context
servicebus/_common/utils.py always expects sync version of _add_span_request_attributes()

From 7a09a818c7c129fa122a886d36fb0137270702e0 Mon Sep 17 00:00:00 2001
From: Sergei Nikiforov <sergey.nikiforoff@gmail.com>
Date: Fri, 10 Feb 2023 16:27:20 +0100
Subject: [PATCH] servicebus/_common/utils expects sync version of
_add_span_request_attributes

---
.../azure/servicebus/aio/_base_handler_async.py               | 2 +-
.../azure/servicebus/aio/_servicebus_sender_async.py          | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_base_handler_async.py b/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_base_handler_async.py
index 328c69c881..284f41afed 100644
--- a/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_base_handler_async.py
+++ b/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_base_handler_async.py
@@ -387,7 +387,7 @@ class BaseHandler:  # pylint:disable=too-many-instance-attributes
            **kwargs
        )

-    async def _add_span_request_attributes(self, span):
+    def _add_span_request_attributes(self, span):
        return BaseHandlerSync._add_span_request_attributes(  # pylint: disable=protected-access
            self, span
        )
diff --git a/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_servicebus_sender_async.py b/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_servicebus_sender_async.py
index ccbdd6e4ab..b1d43f0f2a 100644
--- a/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_servicebus_sender_async.py
+++ b/sdk/servicebus/azure-servicebus/azure/servicebus/aio/_servicebus_sender_async.py
@@ -257,7 +257,7 @@ class ServiceBusSender(BaseHandler, SenderMixin):
                    schedule_time_utc, send_span, *obj_messages
                )
            if send_span:
-                await self._add_span_request_attributes(send_span)
+                self._add_span_request_attributes(send_span)
            return await self._mgmt_request_response_with_retry(
                REQUEST_RESPONSE_SCHEDULE_MESSAGE_OPERATION,
                request_body,
@@ -375,7 +375,7 @@ class ServiceBusSender(BaseHandler, SenderMixin):
                return  # Short circuit noop if an empty list or batch is provided.

            if send_span:
-                await self._add_span_request_attributes(send_span)
+                self._add_span_request_attributes(send_span)

            await self._do_retryable_operation(
                self._send,
-- 
2.34.1
@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 Feb 10, 2023
@github-actions github-actions bot added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Feb 10, 2023
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Feb 10, 2023
@kashifkhan kashifkhan added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. Messaging Messaging crew labels Feb 10, 2023
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Feb 10, 2023
@annatisch
Copy link
Member

Thanks for the report @futurwasfree!
I agree that it seems that there's no need for _add_span_request_attributes to be an async function, as it's simply calling into sync code.
The formatting of your patch was a bit lost - but I've attempted to recreate it in a PR here:
#28761

@futurwasfree
Copy link
Author

Wow, that was fast! Thanks @annatisch. I think you've managed to recreate my patch well!
Looking forward to new release with a fix! Had to revert to sync servicebus api for now.

P.S. Also I've fixed code layout in my report now..

@annatisch
Copy link
Member

Thanks for confirming @futurwasfree!
I've moved the PR out of drafts and tagged the messaging crew for review :)

cc @kashifkhan, @swathipil, @l0lawrence

@kashifkhan
Copy link
Member

Hi @futurwasfree, we released a new version of servicebus yesterday with a fix for this. Thank you for your PR :)

@futurwasfree
Copy link
Author

That's a great news to wrap up this week with! Thanks for sharing @kashifkhan

@github-actions github-actions bot locked and limited conversation to collaborators Jun 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
customer-reported Issues that are reported by GitHub users external to the Azure organization. Messaging Messaging crew question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Service Bus
Projects
None yet
Development

No branches or pull requests

3 participants