fix docstring, add conversion in otel

Author: roger-zhangg

aws_lambda_powertools/tracing/provider/base.py

@@ -4,22 +4,19 @@
 from typing import AsyncGenerator, Generator, Sequence, Union
 
 
-## TO-Discuss how to refactor this one. Segment seems X-ray exclusive concept
 class BaseSpan(abc.ABC):
-    """Holds common properties and methods on segment and subsegment."""
+    """Holds common properties and methods on span."""
 
     @abc.abstractmethod
     def set_attribute(self, key: str, value: Union[str, numbers.Number, bool], **kwargs) -> None:
-        """Annotate segment or subsegment with a key-value pair.
-
-        Note: Annotations will be indexed for later search query.
+        """set attribute for span with a key-value pair.
 
         Parameters
         ----------
         key: str
-            Metadata key
+            Attribute key
         value: Union[str, numbers.Number, bool]
-            Annotation value
+            Attribute value
         """
 
     @abc.abstractmethod
@@ -63,16 +60,14 @@ def trace_async(self, name: str, **kwargs) -> AsyncGenerator[BaseSpan, None]:
 
     @abc.abstractmethod
     def set_attribute(self, key: str, value: Union[str, numbers.Number, bool], **kwargs) -> None:
-        """Annotate current active trace entity with a key-value pair.
-
-        Note: Annotations will be indexed for later search query.
+        """set attribute on current active trace entity with a key-value pair.
 
         Parameters
         ----------
         key: str
-            Metadata key
+            attribute key
         value: Union[str, numbers.Number, bool]
-            Annotation value
+            attribute value
         """
 
     @abc.abstractmethod

aws_lambda_powertools/tracing/provider/datadog/dd_tracer.py

@@ -27,18 +27,6 @@ def record_exception(self, exception: BaseException, attributes: Optional[Dict]
             _attributes = kwargs
         self.dd_span.set_tags(tags=attributes)
 
-    def __enter__(self):
-        print("entered")
-        return self
-
-    def __exit__(self, exc_type, exc_val, exc_tb):
-        try:
-            if exc_type:
-                self.dd_span.set_exc_info(exc_type, exc_val, exc_tb)
-            self.dd_span.finish()
-        except Exception as e:
-            logger.exception(f"error closing trace {e}")
-
 
 class DDTraceProvider(BaseProvider):
     def __init__(self, dd_tracer: ddtrace.Tracer):
@@ -72,19 +60,19 @@ async def trace_async(
         span_type: Optional[str] = None,
         **kwargs,
     ) -> AsyncGenerator[DDSpan, None]:
-        dd_span = self.dd_tracer.trace(
+        with self.dd_tracer.trace(
             name=name,
             service=service,
             resource=resource,
             span_type=span_type,
-        )
-        yield DDSpan(dd_span=dd_span)
+        ) as dd_span:
+            yield DDSpan(dd_span=dd_span)
 
     def set_attribute(self, key: str | bytes, value: Any, **kwargs: Any) -> None:
         span = self.dd_tracer.context_provider.active()
+        # ignore if no active span
         if isinstance(span, ddtrace.Span):
             span.set_tag(key=key, value=value)
-        # ignore if no active span
 
     def patch(self, modules: Sequence[str]) -> None:
         module_to_patch = {}

aws_lambda_powertools/tracing/provider/otel/otel_tracer.py

@@ -11,16 +11,14 @@
 logger = logging.getLogger(__name__)
 
 
-# optl terminology first
-# 1. Provider based on OTel terminology
-# 2. X-Ray provider on top of the new BaseProvider
-# 3. Datadog provider on top of the new BaseProvider
-# access xray sdk
 class OtelSpan(BaseSpan):
     def __init__(self, otel_span=otel_trace.Span):
         self.otel_span = otel_span
 
     def set_attribute(self, key: str, value: str | float | bool, **kwargs) -> None:  # type: ignore[override]
+        if not isinstance(value, (str, bool, int, float)):
+            # convert value to str if value is not a supported structur
+            value = str(value)
         self.otel_span.set_attribute(key=key, value=value)
 
     def record_exception(

aws_lambda_powertools/tracing/tracer.py

@@ -175,8 +175,24 @@ def __init__(
             self._disable_xray_trace_batching()
 
     def set_attribute(self, key: str, value: Union[str, numbers.Number, bool]):
+        """Set attribute on current active trace entity with a key-value pair.
+
+        Parameters
+        ----------
+        key : str
+            attribute key
+        value : Union[str, numbers.Number, bool]
+            Value for attribute
+
+        Example
+        -------
+        Set attribute for a pseudo service named payment
+
+            tracer = Tracer(service="payment")
+            tracer.set_attribute("PaymentStatus", "CONFIRMED")
+        """
         if self.disabled:
-            logger.debug("Tracing has been disabled, aborting put_annotation")
+            logger.debug("Tracing has been disabled, aborting set_attribute")
             return
 
         logger.debug(f"setting attribute on key '{key}' with '{value}'")
@@ -206,10 +222,10 @@ def put_annotation(self, key: str, value: Union[str, numbers.Number, bool]):
 
         logger.debug(f"Annotating on key '{key}' with '{value}'")
 
-        if not self._is_custom_provider():
-            self.provider.put_annotation(key=key, value=value)  # type: ignore
-        else:
+        if self._is_custom_provider():
             self.provider.set_attribute(key=key, value=value)
+        else:
+            self.provider.put_annotation(key=key, value=value)  # type: ignore
 
     def put_metadata(self, key: str, value: Any, namespace: Optional[str] = None):
         """Adds metadata to existing segment or subsegment
@@ -237,10 +253,10 @@ def put_metadata(self, key: str, value: Any, namespace: Optional[str] = None):
 
         namespace = namespace or self.service
         logger.debug(f"Adding metadata on key '{key}' with '{value}' at namespace '{namespace}'")
-        if not self._is_custom_provider():
-            self.provider.put_metadata(key=key, value=value, namespace=namespace)  # type: ignore
-        else:
+        if self._is_custom_provider():
             self.provider.set_attribute(key=f"{namespace}.{key}", value=value)
+        else:
+            self.provider.put_metadata(key=key, value=value, namespace=namespace)  # type: ignore
 
     def patch(self, modules: Optional[Sequence[str]] = None):
         """Patch modules for instrumentation.
@@ -348,19 +364,19 @@ def decorate(event, context, **kwargs):
                 finally:
                     global is_cold_start
                     logger.debug("Annotating cold start")
-                    if not self._is_custom_provider():
-                        subsegment.put_annotation(key="ColdStart", value=is_cold_start)
-                    else:
+                    if self._is_custom_provider():
                         subsegment.set_attribute(key="ColdStart", value=is_cold_start)
+                    else:
+                        subsegment.put_annotation(key="ColdStart", value=is_cold_start)
 
                     if is_cold_start:
                         is_cold_start = False
 
                     if self.service:
-                        if not self._is_custom_provider():
-                            subsegment.put_annotation(key="Service", value=self.service)
-                        else:
+                        if self._is_custom_provider():
                             subsegment.set_attribute(key="Service", value=self.service)
+                        else:
+                            subsegment.put_annotation(key="Service", value=self.service)
 
                 return response
 
@@ -739,10 +755,10 @@ def _add_response_as_metadata(
         """
         if data is None or not capture_response or subsegment is None:
             return
-        if not self._is_custom_provider():
-            subsegment.put_metadata(key=f"{method_name} response", value=data, namespace=self.service)  # type: ignore
-        else:
+        if self._is_custom_provider():
             subsegment.set_attribute(key=f"{method_name} response", value=data, namespace=self.service)
+        else:
+            subsegment.put_metadata(key=f"{method_name} response", value=data, namespace=self.service)  # type: ignore
 
     def _add_full_exception_as_metadata(
         self,
@@ -767,14 +783,14 @@ def _add_full_exception_as_metadata(
         if not capture_error:
             return
 
-        if not self._is_custom_provider():
-            subsegment.put_metadata(key=f"{method_name} error", value=error, namespace=self.service)  # type: ignore
-        else:
+        if self._is_custom_provider():
             subsegment.set_attribute(
                 key=f"{self.service}.{method_name} error",
                 value=str(error),
                 namespace=self.service,
             )
+        else:
+            subsegment.put_metadata(key=f"{method_name} error", value=error, namespace=self.service)  # type: ignore
 
     @staticmethod
     def _disable_tracer_provider():
@@ -858,6 +874,7 @@ def _is_xray_provider(self):
         return isinstance(self.provider, XrayProvider)
 
     def _is_custom_provider(self):
+        # check if provider is not default xray_provider. Avoid test conflits
         return not self._is_xray_provider() and isinstance(self.provider, BaseProvider)
 
     def ignore_endpoint(self, hostname: Optional[str] = None, urls: Optional[List[str]] = None):