botocore: Make common span attributes compliant with semconv in spec

CHANGELOG.md

@@ -6,10 +6,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.5.0-0.24b0...HEAD)
-- `opentelemetry-sdk-extension-aws` Release AWS Python SDK Extension as 1.0.0
-  ([#667](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/667))
 
 ### Changed
+- `opentelemetry-instrumentation-botocore` Make common span attributes compliant with semantic conventions
+  ([#674](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/674))
+- `opentelemetry-sdk-extension-aws` Release AWS Python SDK Extension as 1.0.0
+  ([#667](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/667))
 - `opentelemetry-instrumentation-botocore` Unpatch botocore Endpoint.prepare_request on uninstrument
   ([#664](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/664))
 - `opentelemetry-instrumentation-botocore` Fix span injection for lambda invoke

instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py

@@ -48,14 +48,17 @@
 
 import json
 import logging
-from typing import Collection
+from typing import Collection, Optional
 
 from botocore.client import BaseClient
 from botocore.endpoint import Endpoint
 from botocore.exceptions import ClientError
 from wrapt import wrap_function_wrapper
 
 from opentelemetry import context as context_api
+from opentelemetry.instrumentation.botocore.extensions.types import (
+    _AwsSdkCallContext,
+)
 from opentelemetry.instrumentation.botocore.package import _instruments
 from opentelemetry.instrumentation.botocore.version import __version__
 from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
@@ -65,7 +68,8 @@
 )
 from opentelemetry.propagate import inject
 from opentelemetry.semconv.trace import SpanAttributes
-from opentelemetry.trace import SpanKind, get_tracer
+from opentelemetry.trace import get_tracer
+from opentelemetry.trace.span import Span
 
 logger = logging.getLogger(__name__)
 
@@ -118,12 +122,12 @@ def _uninstrument(self, **kwargs):
         unwrap(Endpoint, "prepare_request")
 
     @staticmethod
-    def _is_lambda_invoke(service_name, operation_name, api_params):
+    def _is_lambda_invoke(call_context: _AwsSdkCallContext):
         return (
-            service_name == "lambda"
-            and operation_name == "Invoke"
-            and isinstance(api_params, dict)
-            and "Payload" in api_params
+            call_context.service == "lambda"
+            and call_context.operation == "Invoke"
+            and isinstance(call_context.params, dict)
+            and "Payload" in call_context.params
         )
 
     @staticmethod
@@ -143,31 +147,35 @@ def _patched_api_call(self, original_func, instance, args, kwargs):
         if context_api.get_value(_SUPPRESS_INSTRUMENTATION_KEY):
             return original_func(*args, **kwargs)
 
-        # pylint: disable=protected-access
-        service_name = instance._service_model.service_name
-        operation_name, api_params = args
+        call_context = _determine_call_context(instance, args)
+        if call_context is None:
+            return original_func(*args, **kwargs)
 
-        error = None
-        result = None
+        attributes = {
+            SpanAttributes.RPC_SYSTEM: "aws-api",
+            SpanAttributes.RPC_SERVICE: call_context.service_id,
+            SpanAttributes.RPC_METHOD: call_context.operation,
+            # TODO: update when semantic conventions exist
+            "aws.region": call_context.region,
+        }
 
         with self._tracer.start_as_current_span(
-            "{}".format(service_name), kind=SpanKind.CLIENT,
+            call_context.span_name,
+            kind=call_context.span_kind,
+            attributes=attributes,
         ) as span:
             # inject trace context into payload headers for lambda Invoke
-            if BotocoreInstrumentor._is_lambda_invoke(
-                service_name, operation_name, api_params
-            ):
-                BotocoreInstrumentor._patch_lambda_invoke(api_params)
+            if BotocoreInstrumentor._is_lambda_invoke(call_context):
+                BotocoreInstrumentor._patch_lambda_invoke(call_context.params)
 
             if span.is_recording():
-                span.set_attribute("aws.operation", operation_name)
-                span.set_attribute("aws.region", instance.meta.region_name)
-                span.set_attribute("aws.service", service_name)
-                if "QueueUrl" in api_params:
-                    span.set_attribute("aws.queue_url", api_params["QueueUrl"])
-                if "TableName" in api_params:
+                if "QueueUrl" in call_context.params:
+                    span.set_attribute(
+                        "aws.queue_url", call_context.params["QueueUrl"]
+                    )
+                if "TableName" in call_context.params:
                     span.set_attribute(
-                        "aws.table_name", api_params["TableName"]
+                        "aws.table_name", call_context.params["TableName"]
                     )
 
             token = context_api.attach(
@@ -176,46 +184,63 @@ def _patched_api_call(self, original_func, instance, args, kwargs):
 
             try:
                 result = original_func(*args, **kwargs)
-            except ClientError as ex:
-                error = ex
+            except ClientError as error:
+                error_result = getattr(error, "response", None)
+                _apply_response_attributes(span, error_result)
+                raise
+            else:
+                _apply_response_attributes(span, result)
             finally:
                 context_api.detach(token)
+            return result
 
-            if error:
-                result = error.response
 
-            if span.is_recording():
-                if "ResponseMetadata" in result:
-                    metadata = result["ResponseMetadata"]
-                    req_id = None
-                    if "RequestId" in metadata:
-                        req_id = metadata["RequestId"]
-                    elif "HTTPHeaders" in metadata:
-                        headers = metadata["HTTPHeaders"]
-                        if "x-amzn-RequestId" in headers:
-                            req_id = headers["x-amzn-RequestId"]
-                        elif "x-amz-request-id" in headers:
-                            req_id = headers["x-amz-request-id"]
-                        elif "x-amz-id-2" in headers:
-                            req_id = headers["x-amz-id-2"]
-
-                    if req_id:
-                        span.set_attribute(
-                            "aws.request_id", req_id,
-                        )
-
-                    if "RetryAttempts" in metadata:
-                        span.set_attribute(
-                            "retry_attempts", metadata["RetryAttempts"],
-                        )
-
-                    if "HTTPStatusCode" in metadata:
-                        span.set_attribute(
-                            SpanAttributes.HTTP_STATUS_CODE,
-                            metadata["HTTPStatusCode"],
-                        )
-
-            if error:
-                raise error
+def _apply_response_attributes(span: Span, result):
+    if result is None or not span.is_recording():
+        return
 
-            return result
+    metadata = result.get("ResponseMetadata")
+    if metadata is None:
+        return
+
+    request_id = metadata.get("RequestId")
+    if request_id is None:
+        headers = metadata.get("HTTPHeaders")
+        if headers is not None:
+            request_id = (
+                headers.get("x-amzn-RequestId")
+                or headers.get("x-amz-request-id")
+                or headers.get("x-amz-id-2")
+            )
+    if request_id:
+        # TODO: update when semantic conventions exist
+        span.set_attribute("aws.request_id", request_id)
+
+    retry_attempts = metadata.get("RetryAttempts")
+    if retry_attempts is not None:
+        # TODO: update when semantic conventinos exists
+        span.set_attribute("retry_attempts", retry_attempts)
+
+    status_code = metadata.get("HTTPStatusCode")
+    if status_code is not None:
+        span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, status_code)
+
+
+def _determine_call_context(
+    client: BaseClient, args
+) -> Optional[_AwsSdkCallContext]:
+    try:
+        operation = args[0]
+        params = args[1]
+        call_context = _AwsSdkCallContext(client, operation, params)
+
+        logger.debug(
+            "AWS SDK invocation: %s %s",
+            call_context.service,
+            call_context.operation,
+        )
+
+        return call_context
+    except Exception as ex:  # pylint:disable=broad-except
+        logger.warning("Error when initializing call context", exc_info=ex)
+        return None

instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/extensions/types.py

@@ -0,0 +1,45 @@
+from typing import Any, Dict
+
+from opentelemetry.trace import SpanKind
+
+_BotoClientT = "botocore.client.BaseClient"
+
+_OperationParamsT = Dict[str, Any]
+
+
+class _AwsSdkCallContext:
+    """An context object providing information about the invoked AWS service
+    call.
+
+    Args:
+        service: the AWS service (e.g. s3, lambda, ...) which is called
+        service_id: the name of the service in propper casing
+        operation: the called operation (e.g. ListBuckets, Invoke, ...) of the
+            AWS service.
+        params: a dict of input parameters passed to the service operation.
+        region: the AWS region in which the service call is made
+        endpoint_url: the endpoint which the service operation is calling
+        api_version: the API version of the called AWS service.
+        span_name: the name used to create the span.
+        span_kind: the kind used to create the span.
+    """
+
+    def __init__(
+        self, client: _BotoClientT, operation: str, params: _OperationParamsT
+    ):
+        boto_meta = client.meta
+        service_model = boto_meta.service_model
+
+        self.service = service_model.service_name.lower()
+        self.operation = operation
+        self.params = params
+
+        self.region = boto_meta.region_name  # type: str
+        self.endpoint_url = boto_meta.endpoint_url  # type: str
+
+        self.api_version = service_model.api_version  # type: str
+        # name of the service in proper casing
+        self.service_id = str(service_model.service_id)
+
+        self.span_name = "{}.{}".format(self.service_id, self.operation)
+        self.span_kind = SpanKind.CLIENT