Merge pull request #99 from cisco-open/fix/aiohttp-url/SDK-668

fix(aiohttp-attributes): add http.url attribute
cisco-open · Jul 4, 2022 · 28bf7ef · 28bf7ef
2 parents 68b44db + b0b2c85
commit 28bf7ef
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 37 deletions.
diff --git a/cisco_telescope/instrumentations/aiohttp/__init__.py b/cisco_telescope/instrumentations/aiohttp/__init__.py
@@ -40,6 +40,7 @@
     StatusCode,
 )
 from opentelemetry import trace
+from opentelemetry.util.http import remove_url_credentials
 from cisco_opentelemetry_specifications import SemanticAttributes
 
 from ..utils import Utils
@@ -109,22 +110,20 @@ async def on_request_start(
 
         http_method = params.method.upper()
         request_span_name = f"HTTP {http_method}"
+        request_url = remove_url_credentials(str(params.url))
+
+        span_attributes = {
+            SpanAttributes.HTTP_METHOD: http_method,
+            SpanAttributes.HTTP_URL: request_url,
+        }
 
         trace_config_ctx.span = trace_config_ctx.tracer.start_span(
-            request_span_name,
-            kind=SpanKind.CLIENT,
+            request_span_name, kind=SpanKind.CLIENT, attributes=span_attributes
         )
 
         if callable(request_hook):
             request_hook(trace_config_ctx.span, params)
 
-        if trace_config_ctx.span.is_recording():
-            attributes = {
-                SpanAttributes.HTTP_METHOD: http_method,
-            }
-            for key, value in attributes.items():
-                trace_config_ctx.span.set_attribute(key, value)
-
         trace_config_ctx.token = context_api.attach(
             trace.set_span_in_context(trace_config_ctx.span)
         )

diff --git a/cisco_telescope/options.py b/cisco_telescope/options.py
@@ -123,7 +123,7 @@ def __init__(
                 exporter_type=Consts.DEFAULT_EXPORTER_TYPE,
                 collector_endpoint=Consts.DEFAULT_COLLECTOR_ENDPOINT,
                 custom_headers={
-                    Consts.TOKEN_HEADER_KEY: verify_token(self.cisco_token)
+                    Consts.TOKEN_HEADER_KEY: _verify_token(self.cisco_token)
                 },
             )
         ]
@@ -137,6 +137,7 @@ def __str__(self):
             f"token: {self.cisco_token},\n\t"
             f"service_name:{self.service_name},\n\t"
             f"max_payload_size: {self.max_payload_size},\n\t"
+            f"disable_instrumentations: {self.disable_instrumentations},\n\t"
             f"exporters: \n\t{', '.join(map(str, self.exporters))})"
         )
 
@@ -172,7 +173,7 @@ def _set_debug(self):
             )
 
 
-def verify_token(token: str) -> str:
+def _verify_token(token: str) -> str:
     auth_prefix = "Bearer "
     if not token:
         return ""

diff --git a/tests/instrumentations/test_aiohttp.py b/tests/instrumentations/test_aiohttp.py
@@ -20,6 +20,7 @@
 from unittest import IsolatedAsyncioTestCase
 
 from cisco_opentelemetry_specifications import SemanticAttributes
+from opentelemetry.semconv.trace import SpanAttributes
 from cisco_telescope.configuration import Configuration
 from cisco_telescope.instrumentations.aiohttp import AiohttpInstrumentorWrapper
 from .base_http_test import BaseHttpTest
@@ -43,20 +44,7 @@ async def test_get_request_sanity(self):
             headers=self.request_headers(),
             chunked=True,
         ) as resp:
-            self.assertEqual(resp.status, 200)
-            spans = self.memory_exporter.get_finished_spans()
-            self.assertEqual(len(spans), 1)
-            span = spans[0]
-            self.assert_captured_headers(
-                span,
-                SemanticAttributes.HTTP_REQUEST_HEADER,
-                self.request_headers(),
-            )
-            self.assert_captured_headers(
-                span,
-                SemanticAttributes.HTTP_RESPONSE_HEADER,
-                self.response_headers(),
-            )
+            self._assert_basic_attributes_and_headers(resp)
 
     async def test_post_request_sanity(self):
         Configuration().payloads_enabled = True
@@ -67,21 +55,10 @@ async def test_post_request_sanity(self):
             chunked=True,
             data=self.request_body(),
         ) as resp:
-            self.assertEqual(resp.status, 200)
+            self._assert_basic_attributes_and_headers(resp)
             spans = self.memory_exporter.get_finished_spans()
-            self.assertEqual(len(spans), 1)
             span = spans[0]
 
-            self.assert_captured_headers(
-                span,
-                SemanticAttributes.HTTP_REQUEST_HEADER,
-                self.request_headers(),
-            )
-            self.assert_captured_headers(
-                span,
-                SemanticAttributes.HTTP_RESPONSE_HEADER,
-                self.response_headers(),
-            )
             self.assertEqual(
                 span.attributes[SemanticAttributes.HTTP_REQUEST_BODY],
                 self.request_body(),
@@ -167,3 +144,21 @@ async def test_response_content_unharmed(self):
                 span.attributes[SemanticAttributes.HTTP_RESPONSE_BODY],
                 resp_body,
             )
+
+    def _assert_basic_attributes_and_headers(self, resp):
+        self.assertEqual(resp.status, 200)
+        spans = self.memory_exporter.get_finished_spans()
+        self.assertEqual(len(spans), 1)
+        span = spans[0]
+        self.assertEqual(span.attributes[SpanAttributes.HTTP_METHOD], resp.method)
+        self.assertEqual(span.attributes[SpanAttributes.HTTP_URL], str(resp.url))
+        self.assert_captured_headers(
+            span,
+            SemanticAttributes.HTTP_REQUEST_HEADER,
+            self.request_headers(),
+        )
+        self.assert_captured_headers(
+            span,
+            SemanticAttributes.HTTP_RESPONSE_HEADER,
+            self.response_headers(),
+        )