Simplify attributes for botocore instrumentation

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

@@ -51,8 +51,8 @@
 from wrapt import wrap_function_wrapper
 
 from opentelemetry.instrumentation.boto.version import __version__
-from opentelemetry.instrumentation.botocore import add_span_arg_tags, unwrap
 from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
+from opentelemetry.instrumentation.utils import unwrap
 from opentelemetry.sdk.trace import Resource
 from opentelemetry.trace import SpanKind, get_tracer
 
@@ -204,3 +204,46 @@ def _patched_auth_request(self, original_func, instance, args, kwargs):
             args,
             kwargs,
         )
+
+
+def add_span_arg_tags(span, endpoint_name, args, args_names, args_traced):
+    def truncate_arg_value(value, max_len=1024):
+        """Truncate values which are bytes and greater than `max_len`.
+        Useful for parameters like "Body" in `put_object` operations.
+        """
+        if isinstance(value, bytes) and len(value) > max_len:
+            return b"..."
+
+        return value
+
+    def flatten_dict(dict_, sep=".", prefix=""):
+        """
+        Returns a normalized dict of depth 1 with keys in order of embedding
+        """
+        # adapted from https://stackoverflow.com/a/19647596
+        return (
+            {
+                prefix + sep + k if prefix else k: v
+                for kk, vv in dict_.items()
+                for k, v in flatten_dict(vv, sep, kk).items()
+            }
+            if isinstance(dict_, dict)
+            else {prefix: dict_}
+        )
+
+    if not span.is_recording():
+        return
+
+    if endpoint_name not in {"kms", "sts"}:
+        tags = dict(
+            (name, value)
+            for (name, value) in zip(args_names, args)
+            if name in args_traced
+        )
+        tags = flatten_dict(tags)
+        for key, value in {
+            k: truncate_arg_value(v)
+            for k, v in tags.items()
+            if k not in {"s3": ["params.Body"]}.get(endpoint_name, [])
+        }.items():
+            span.set_attribute(key, value)

instrumentation/opentelemetry-instrumentation-botocore/CHANGELOG.md

@@ -2,6 +2,9 @@
 
 ## Unreleased
 
+- Botocore SpanKind as CLIENT and match Trace Attributes to Specification
+  ([#150])(https://github.com/open-telemetry/opentelemetry-python-contrib/pull/150)
+
 ## Version 0.13b0
 
 Released 2020-09-17

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

@@ -58,22 +58,21 @@
 
 from opentelemetry.instrumentation.botocore.version import __version__
 from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
+from opentelemetry.instrumentation.utils import unwrap
 from opentelemetry.sdk.trace import Resource
 from opentelemetry.trace import SpanKind, get_tracer
 
 logger = logging.getLogger(__name__)
 
 
 class BotocoreInstrumentor(BaseInstrumentor):
-    """A instrumentor for Botocore
+    """A instrumentor for Botocore.
 
     See `BaseInstrumentor`
     """
 
     def _instrument(self, **kwargs):
 
-        # FIXME should the tracer provider be accessed via Configuration
-        # instead?
         # pylint: disable=attribute-defined-outside-init
         self._tracer = get_tracer(
             __name__, __version__, kwargs.get("tracer_provider")
@@ -90,45 +89,18 @@ def _uninstrument(self, **kwargs):
 
     def _patched_api_call(self, original_func, instance, args, kwargs):
 
-        endpoint_name = deep_getattr(instance, "_endpoint._endpoint_prefix")
+        # pylint: disable=protected-access
+        service_name = instance._service_model.service_name
+        operation_name, _ = args
 
         with self._tracer.start_as_current_span(
-            "{}.command".format(endpoint_name), kind=SpanKind.CONSUMER,
+            "{}".format(service_name), kind=SpanKind.CLIENT,
         ) as span:
 
-            operation = None
-            if args and span.is_recording():
-                operation = args[0]
-                span.resource = Resource(
-                    attributes={
-                        "endpoint": endpoint_name,
-                        "operation": operation.lower(),
-                    }
-                )
-
-            else:
-                span.resource = Resource(
-                    attributes={"endpoint": endpoint_name}
-                )
-
-            add_span_arg_tags(
-                span,
-                endpoint_name,
-                args,
-                ("action", "params", "path", "verb"),
-                {"params", "path", "verb"},
-            )
-
             if span.is_recording():
-                region_name = deep_getattr(instance, "meta.region_name")
-
-                meta = {
-                    "aws.agent": "botocore",
-                    "aws.operation": operation,
-                    "aws.region": region_name,
-                }
-                for key, value in meta.items():
-                    span.set_attribute(key, value)
+                span.set_attribute("aws.operation", operation_name)
+                span.set_attribute("aws.region", instance.meta.region_name)
+                span.set_attribute("aws.service", service_name)
 
             result = original_func(*args, **kwargs)
 
@@ -137,86 +109,5 @@ def _patched_api_call(self, original_func, instance, args, kwargs):
                     "http.status_code",
                     result["ResponseMetadata"]["HTTPStatusCode"],
                 )
-                span.set_attribute(
-                    "retry_attempts",
-                    result["ResponseMetadata"]["RetryAttempts"],
-                )
 
             return result
-
-
-def unwrap(obj, attr):
-    function = getattr(obj, attr, None)
-    if (
-        function
-        and isinstance(function, ObjectProxy)
-        and hasattr(function, "__wrapped__")
-    ):
-        setattr(obj, attr, function.__wrapped__)
-
-
-def add_span_arg_tags(span, endpoint_name, args, args_names, args_traced):
-    def truncate_arg_value(value, max_len=1024):
-        """Truncate values which are bytes and greater than `max_len`.
-        Useful for parameters like "Body" in `put_object` operations.
-        """
-        if isinstance(value, bytes) and len(value) > max_len:
-            return b"..."
-
-        return value
-
-    def flatten_dict(dict_, sep=".", prefix=""):
-        """
-        Returns a normalized dict of depth 1 with keys in order of embedding
-        """
-        # adapted from https://stackoverflow.com/a/19647596
-        return (
-            {
-                prefix + sep + k if prefix else k: v
-                for kk, vv in dict_.items()
-                for k, v in flatten_dict(vv, sep, kk).items()
-            }
-            if isinstance(dict_, dict)
-            else {prefix: dict_}
-        )
-
-    if not span.is_recording():
-        return
-
-    if endpoint_name not in {"kms", "sts"}:
-        tags = dict(
-            (name, value)
-            for (name, value) in zip(args_names, args)
-            if name in args_traced
-        )
-        tags = flatten_dict(tags)
-        for key, value in {
-            k: truncate_arg_value(v)
-            for k, v in tags.items()
-            if k not in {"s3": ["params.Body"]}.get(endpoint_name, [])
-        }.items():
-            span.set_attribute(key, value)
-
-
-def deep_getattr(obj, attr_string, default=None):
-    """
-    Returns the attribute of ``obj`` at the dotted path given by
-    ``attr_string``, if no such attribute is reachable, returns ``default``.
-
-    >>> deep_getattr(cass, "cluster")
-    <cassandra.cluster.Cluster object at 0xa20c350
-
-    >>> deep_getattr(cass, "cluster.metadata.partitioner")
-    u"org.apache.cassandra.dht.Murmur3Partitioner"
-
-    >>> deep_getattr(cass, "i.dont.exist", default="default")
-    "default"
-    """
-    attrs = attr_string.split(".")
-    for attr in attrs:
-        try:
-            obj = getattr(obj, attr)
-        except AttributeError:
-            return default
-
-    return obj