Merge pull request #129 from Yelp/u/eojeah/clientobs-1083-status-code…

…-attribute

U/eojeah/otel-semantic-attributes-http

.github/workflows/build.yaml

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

.gitignore

@@ -10,3 +10,4 @@ dist/
 venv
 tags
 *.swp
+.idea/

.pre-commit-config.yaml

@@ -1,19 +1,18 @@
 repos:
 -   repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v0.9.1
+    rev: v4.5.0
     hooks:
     -   id: trailing-whitespace
     -   id: end-of-file-fixer
-    -   id: autopep8-wrapper
     -   id: check-json
         files: \.(bowerrc|jshintrc|json)$
     -   id: check-yaml
     -   id: debug-statements
     -   id: name-tests-test
         exclude: tests/acceptance/test_helper.py
     -   id: requirements-txt-fixer
--   repo: https://gitlab.com/pycqa/flake8
-    rev: 5.0.4
+-   repo: https://github.com/pycqa/flake8
+    rev: 7.0.0
     hooks:
     -   id: flake8
         args:
@@ -23,9 +22,9 @@ repos:
     rev: v0.3.5
     hooks:
     -   id: reorder-python-imports
-        language_version: python3.7
+        language_version: python3.8
 -   repo: https://github.com/asottile/pyupgrade
     rev: v2.38.2
     hooks:
     -   id: pyupgrade
-        args: ['--py37-plus']
+        args: ['--py38-plus']

mypy.ini

@@ -1,5 +1,5 @@
 [mypy]
-python_version = 3.7
+python_version = 3.8
 pretty = true
 show_error_codes = true
 show_error_context = true

pyramid_zipkin/request_helper.py

@@ -10,6 +10,8 @@
 from pyramid.request import Request
 from pyramid.response import Response
 
+from pyramid_zipkin.version import __version__
+
 
 DEFAULT_REQUEST_TRACING_PERCENT = 0.5
 
@@ -165,15 +167,51 @@ 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,
+        'otel.library.name': __name__.split('.')[0],
+        'otel.library.version': __version__,
     }
+
+    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)
+        status_code = response.status_code
+        if isinstance(status_code, int):
+            annotations['http.response.status_code'] = str(status_code)
+            annotations['response_status_code'] = str(status_code)
+            if 100 <= status_code < 200:
+                annotations['otel.status_code'] = 'Unset'
+            elif 200 <= status_code < 300:
+                annotations['otel.status_code'] = 'Ok'
+            elif 300 <= status_code < 500:
+                annotations['otel.status_code'] = 'Unset'
+            else:
+                annotations['otel.status_code'] = 'Error'
+
+        else:
+            annotations['otel.status_code'] = 'Error'
+            annotations['otel.status_description'] = (
+                f'Non-integer HTTP status code: {repr(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,10 +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__,
-                    'response_status_code': '500'
+                    '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 \

pyramid_zipkin/version.py

@@ -0,0 +1 @@
+__version__ = '2.2.0'

requirements-dev.txt

@@ -2,4 +2,5 @@ coverage
 mypy
 ordereddict
 pytest
+tox
 webtest

setup.py

@@ -2,11 +2,11 @@
 from setuptools import find_packages
 from setuptools import setup
 
-__version__ = '2.2.0'
+version = exec(open('pyramid_zipkin/version.py').read())
 
 setup(
     name='pyramid_zipkin',
-    version=__version__,
+    version=version,
     provides=["pyramid_zipkin"],
     author='Yelp, Inc.',
     author_email='[email protected]',
@@ -28,8 +28,7 @@
         "Topic :: Software Development :: Libraries :: Python Modules",
         "License :: OSI Approved :: Apache Software License",
         "Operating System :: OS Independent",
-        "Programming Language :: Python :: 3.7",
         "Programming Language :: Python :: 3.8",
     ],
-    python_requires=">=3.7",
+    python_requires=">=3.8",
 )

tests/acceptance/app/__init__.py

@@ -96,6 +96,20 @@ def client_error(dummy_request):
     return response
 
 
+@view_config(route_name='redirect', renderer='json')
+def redirect(dummy_request):
+    response = Response('Redirectional')
+    response.status_int = 302
+    return response
+
+
+@view_config(route_name='information_route', renderer='json')
+def information_route(dummy_request):
+    response = Response('Informational')
+    response.status_int = 199
+    return response
+
+
 def main(global_config, **settings):
     """ Very basic pyramid app """
     settings['service_name'] = 'acceptance_service'
@@ -114,6 +128,8 @@ def main(global_config, **settings):
 
     config.add_route('server_error', '/server_error')
     config.add_route('client_error', '/client_error')
+    config.add_route('information_route', '/information_route')
+    config.add_route('redirect', '/redirect')
 
     config.scan()
 

tests/acceptance/conftest.py

@@ -2,6 +2,8 @@
 
 import pytest
 
+from pyramid_zipkin.version import __version__
+
 
 @pytest.fixture
 def default_trace_id_generator(dummy_request):
@@ -24,7 +26,17 @@ 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',
+            'otel.library.version': __version__,
+            'otel.library.name': 'pyramid_zipkin',
         },
         'name': 'GET /sample',
         'traceId': '17133d482ba4f605',