CLIENTOBS-1349

Update zipkin attributes to adhere to Stable components of the opentelemetry semantic conventions

Set otel span status for http server spans

.github/workflows/build.yaml

@@ -12,7 +12,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        python-version: ['3.8', '3.9', '3.10', '3.11', '3.12']
+        python-version: ['3.8', '3.10', '3.11', '3.12']
 
     steps:
     - uses: actions/checkout@v2

pyramid_zipkin/request_helper.py

@@ -165,16 +165,39 @@ def get_binary_annotations(
     :param response: the Pyramid response object
     :returns: binary annotation dict of {str: str}
     """
-    route = request.matched_route.pattern if request.matched_route else ''
 
     annotations = {
+        'http.request.method': request.method,
+        'network.protocol.version': request.http_version,
+        'url.path': request.path,
+        'server.address': request.server_name,
+        'server.port': str(request.server_port),
+        'url.scheme': request.scheme,
         'http.uri': request.path,
         'http.uri.qs': request.path_qs,
-        'http.route': route,
     }
+
+    if request.user_agent:
+        annotations['user_agent.original'] = request.user_agent
+
+    if request.query_string:
+        annotations["url.query"] = request.query_string
+
+    if request.matched_route:
+        annotations['http.route'] = request.matched_route.pattern
+
+    if request.client_addr:
+        annotations['client.address'] = request.client_addr
+
     if response:
-        annotations['response_status_code'] = str(response.status_code)
-        annotations['http.response.status_code'] = str(response.status_code)
+        status_code = response.status_code
+        if isinstance(status_code, int):
+            if status_code >= 500:
+                annotations['otel.status_code'] = 'Error'
+            elif 200 <= status_code < 300:
+                annotations['otel.status_code'] = 'Ok'
+        annotations['http.response.status_code'] = str(status_code)
+        annotations['response_status_code'] = str(status_code)
 
     settings = request.registry.settings
     if 'zipkin.set_extra_binary_annotations' in settings:

pyramid_zipkin/tween.py

@@ -1,4 +1,5 @@
 import functools
+import traceback
 import warnings
 from collections import namedtuple
 from typing import Any
@@ -196,11 +197,15 @@ def tween(request: Request) -> Response:
             try:
                 response = handler(request)
             except Exception as e:
+                exception_type = type(e).__name__
+                exception_stacktrace = traceback.format_exc()
                 zipkin_context.update_binary_annotations({
-                    'error.type': type(e).__name__,
+                    'error.type': exception_type,
                     'response_status_code': '500',
                     'http.response.status_code': '500',
+                    'exception.stacktrace': exception_stacktrace,
                 })
+                zipkin_context.add_annotation(exception_type)
                 raise e
             finally:
                 if zipkin_settings.use_pattern_as_span_name \

tests/acceptance/conftest.py

@@ -24,8 +24,15 @@ def get_span():
             'http.uri': '/sample',
             'http.uri.qs': '/sample',
             'http.route': '/sample',
+            'url.path': '/sample',
+            'url.scheme': 'http',
+            'network.protocol.version': 'HTTP/1.0',
+            'server.address': 'localhost',
+            'server.port': '80',
             'response_status_code': '200',
             'http.response.status_code': '200',
+            'http.request.method': 'GET',
+            'otel.status_code': 'Ok',
         },
         'name': 'GET /sample',
         'traceId': '17133d482ba4f605',

tests/acceptance/server_span_test.py

@@ -174,6 +174,14 @@ def set_extra_binary_annotations(dummy_request, response):
         'http.route': '/pet/{petId}',
         'response_status_code': '200',
         'http.response.status_code': '200',
+        'http.request.method': 'GET',
+        'otel.status_code': 'Ok',
+        'network.protocol.version': 'HTTP/1.0',
+        'server.address': 'localhost',
+        'server.port': '80',
+        'url.path': '/pet/123',
+        'url.scheme': 'http',
+        'url.query': 'test=1',
         'other': '42',
     }
 
@@ -195,9 +203,45 @@ def test_binary_annotations_404(default_trace_id_generator):
     assert span['tags'] == {
         'http.uri': '/abcd',
         'http.uri.qs': '/abcd?test=1',
-        'http.route': '',
         'response_status_code': '404',
+        'http.request.method': 'GET',
         'http.response.status_code': '404',
+        'network.protocol.version': 'HTTP/1.0',
+        'server.address': 'localhost',
+        'server.port': '80',
+        'url.path': '/abcd',
+        'url.query': 'test=1',
+        'url.scheme': 'http',
+    }
+
+
+def test_binary_annotations_500(default_trace_id_generator):
+    settings = {
+        'zipkin.tracing_percent': 100,
+        'zipkin.trace_id_generator': default_trace_id_generator,
+    }
+    app_main, transport, _ = generate_app_main(settings)
+
+    WebTestApp(app_main).get('/server_error', status=500)
+
+    assert len(transport.output) == 1
+    spans = json.loads(transport.output[0])
+    assert len(spans) == 1
+
+    span = spans[0]
+    assert span['tags'] == {
+        'http.uri': '/server_error',
+        'http.uri.qs': '/server_error',
+        'http.route': '/server_error',
+        'response_status_code': '500',
+        'http.request.method': 'GET',
+        'http.response.status_code': '500',
+        'otel.status_code': 'Error',
+        'network.protocol.version': 'HTTP/1.0',
+        'server.address': 'localhost',
+        'server.port': '80',
+        'url.path': '/server_error',
+        'url.scheme': 'http',
     }
 
 

tox.ini

@@ -1,5 +1,5 @@
 [tox]
-envlist = pre-commit, py38, py39, py310, py311, py312, flake8, pre-commit
+envlist = pre-commit, py38, py310, py311, py312, flake8, pre-commit
 
 [testenv]
 deps = -rrequirements-dev.txt