[ServiceBus] Update tracing (#29995)

* [ServiceBus] Update tracing - "Send" span now contains links to message spans. - Receive span is now kind CLIENT instead of CONSUMER. - Added span creation logic for settlement methods. - Attribute names were updated to align with distributed tracing conventions. - Some span named renamed to align with other SDKs. - Receive spans now have more accurate start times. Signed-off-by: Paul Van Eck <[email protected]> * Refactor tracing utils Signed-off-by: Paul Van Eck <[email protected]> * Remove unneeded arg from trace_message Signed-off-by: Paul Van Eck <[email protected]> * update changelog Signed-off-by: Paul Van Eck <[email protected]> * Remove use of `messaging.source.name` This is slated to be removed in favor of `messaging.destination.name` for everything. Here, we maintain use of the legacy attribute name `message_bus.destination`. Signed-off-by: Paul Van Eck <[email protected]> * remove test-resources.bicep from stress --------- Signed-off-by: Paul Van Eck <[email protected]> Co-authored-by: swathipil <[email protected]>
Azure · May 5, 2023 · 170fe74 · 170fe74
1 parent a75ad9f
commit 170fe74
Show file tree

Hide file tree

Showing 17 changed files with 570 additions and 453 deletions.
diff --git a/sdk/servicebus/azure-servicebus/CHANGELOG.md b/sdk/servicebus/azure-servicebus/CHANGELOG.md
@@ -10,6 +10,16 @@
 
 ### Other Changes
 
+ - Updated tracing ([#29995](https://github.com/Azure/azure-sdk-for-python/pull/29995)):
+   - Additional attributes added to existing spans:
+     - `messaging.system` - messaging system (i.e., `servicebus`)
+     - `messaging.operation` - type of operation (i.e., `publish`, `receive`, or `settle`)
+     - `messaging.batch.message_count` - number of messages sent or received (if more than one)
+   - A span will now be created upon calls to the service that settle messages.
+     - The span name will contain the settlement operation (e.g., `ServiceBus.complete`)
+     - The span will contain `az.namespace`, `messaging.destination.name`, `net.peer.name`, `messaging.system`, and `messaging.operation` attributes.
+   - All `send` spans now contain links to `message` spans. Now, `message` spans will no longer contain a link to the `send` span.
+
 ## 7.10.0b1 (2023-04-13)
 
 ### Features Added

diff --git a/sdk/servicebus/azure-servicebus/azure/servicebus/_base_handler.py b/sdk/servicebus/azure-servicebus/azure/servicebus/_base_handler.py
@@ -32,11 +32,6 @@
     TOKEN_TYPE_SASTOKEN,
     MGMT_REQUEST_OP_TYPE_ENTITY_MGMT,
     ASSOCIATEDLINKPROPERTYNAME,
-    TRACE_NAMESPACE_PROPERTY,
-    TRACE_COMPONENT_PROPERTY,
-    TRACE_COMPONENT,
-    TRACE_PEER_ADDRESS_PROPERTY,
-    TRACE_BUS_DESTINATION_PROPERTY,
 )
 
 if TYPE_CHECKING:
@@ -543,12 +538,6 @@ def _mgmt_request_response_with_retry(
             **kwargs
         )
 
-    def _add_span_request_attributes(self, span):
-        span.add_attribute(TRACE_COMPONENT_PROPERTY, TRACE_COMPONENT)
-        span.add_attribute(TRACE_NAMESPACE_PROPERTY, TRACE_NAMESPACE_PROPERTY)
-        span.add_attribute(TRACE_BUS_DESTINATION_PROPERTY, self._entity_path)
-        span.add_attribute(TRACE_PEER_ADDRESS_PROPERTY, self.fully_qualified_namespace)
-
     def _open(self):  # pylint: disable=no-self-use
         raise ValueError("Subclass should override the method.")
 

diff --git a/sdk/servicebus/azure-servicebus/azure/servicebus/_common/constants.py b/sdk/servicebus/azure-servicebus/azure/servicebus/_common/constants.py
@@ -137,31 +137,6 @@
     "ServiceBusDlqSupplementaryAuthorization"
 )
 
-# Distributed Tracing Constants
-
-TRACE_COMPONENT_PROPERTY = "component"
-TRACE_COMPONENT = "servicebus"
-
-TRACE_NAMESPACE_PROPERTY = "az.namespace"
-TRACE_NAMESPACE = "ServiceBus"
-
-SPAN_NAME_RECEIVE = TRACE_NAMESPACE + ".receive"
-SPAN_NAME_RECEIVE_DEFERRED = TRACE_NAMESPACE + ".receive_deferred"
-SPAN_NAME_PEEK = TRACE_NAMESPACE + ".peek"
-SPAN_NAME_SEND = TRACE_NAMESPACE + ".send"
-SPAN_NAME_SCHEDULE = TRACE_NAMESPACE + ".schedule"
-SPAN_NAME_MESSAGE = TRACE_NAMESPACE + ".message"
-
-TRACE_BUS_DESTINATION_PROPERTY = "message_bus.destination"
-TRACE_PEER_ADDRESS_PROPERTY = "peer.address"
-
-SPAN_ENQUEUED_TIME_PROPERTY = "enqueuedTime"
-
-TRACE_ENQUEUED_TIME_PROPERTY = b"x-opt-enqueued-time"
-TRACE_PARENT_PROPERTY = b"Diagnostic-Id"
-TRACE_PROPERTY_ENCODING = "ascii"
-
-
 MAX_MESSAGE_LENGTH_BYTES = 1024 * 1024  # Backcompat with uAMQP
 MESSAGE_PROPERTY_MAX_LENGTH = 128
 # .NET TimeSpan.MaxValue: 10675199.02:48:05.4775807

diff --git a/sdk/servicebus/azure-servicebus/azure/servicebus/_common/message.py b/sdk/servicebus/azure-servicebus/azure/servicebus/_common/message.py
@@ -46,9 +46,9 @@
 from .utils import (
     utc_from_timestamp,
     utc_now,
-    trace_message,
     transform_outbound_messages,
 )
+from .tracing import trace_message
 
 if TYPE_CHECKING:
     try:
@@ -60,7 +60,6 @@
     except ImportError:
         pass
     from .._pyamqp.performatives import TransferFrame
-    from azure.core.tracing import AbstractSpan
     from ..aio._servicebus_receiver_async import (
         ServiceBusReceiver as AsyncServiceBusReceiver,
     )
@@ -659,6 +658,7 @@ def __init__(
         **kwargs: Any
     ) -> None:
         self._amqp_transport = kwargs.pop("amqp_transport", PyamqpTransport)
+        self._tracing_attributes: Dict[str, Union[str, int]] = kwargs.pop("tracing_attributes", {})
 
         self._max_size_in_bytes = max_size_in_bytes or MAX_MESSAGE_LENGTH_BYTES
         self._message = self._amqp_transport.build_batch_message([])
@@ -676,15 +676,11 @@ def __repr__(self) -> str:
     def __len__(self) -> int:
         return self._count
 
-    def _from_list(self, messages: Iterable[ServiceBusMessage], parent_span: Optional["AbstractSpan"] = None) -> None:
+    def _from_list(self, messages: Iterable[ServiceBusMessage]) -> None:
         for message in messages:
-            self._add(message, parent_span)
+            self._add(message)
 
-    def _add(
-        self,
-        add_message: Union[ServiceBusMessage, Mapping[str, Any], AmqpAnnotatedMessage],
-        parent_span: Optional["AbstractSpan"] = None
-    ) -> None:
+    def _add(self, add_message: Union[ServiceBusMessage, Mapping[str, Any], AmqpAnnotatedMessage]) -> None:
         """Actual add implementation.  The shim exists to hide the internal parameters such as parent_span."""
         outgoing_sb_message = transform_outbound_messages(
             add_message, ServiceBusMessage, self._amqp_transport.to_outgoing_amqp_message
@@ -694,8 +690,8 @@ def _add(
         outgoing_sb_message._message = trace_message(
             outgoing_sb_message._message,
             amqp_transport=self._amqp_transport,
-            parent_span=parent_span
-        )  # parent_span is e.g. if built as part of a send operation.
+            additional_attributes=self._tracing_attributes
+        )
         message_size = self._amqp_transport.get_message_encoded_size(
             outgoing_sb_message._message  # pylint: disable=protected-access
         )