From 1d4a264e3a12fbf1e592c1601c9ac0d76346dca4 Mon Sep 17 00:00:00 2001 From: Emil Madsen Date: Fri, 21 Feb 2020 17:51:39 +0100 Subject: [PATCH 1/9] WSGI: Changed default span-name, added callback to change span-name. Signed-off-by: Emil Madsen --- .../src/opentelemetry/ext/wsgi/__init__.py | 16 ++++++++-------- .../tests/test_wsgi_middleware.py | 15 +++++++++++++-- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py b/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py index e6751f34ced..bbe56ab8e7f 100644 --- a/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py +++ b/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py @@ -142,12 +142,8 @@ def add_response_attributes( def get_default_span_name(environ): - """Calculates a (generic) span name for an incoming HTTP request based on the PEP3333 conforming WSGI environ.""" - - # TODO: Update once - # https://github.com/open-telemetry/opentelemetry-specification/issues/270 - # is resolved - return environ.get("PATH_INFO", "/") + """Default implementation for name_callback, returns HTTP {METHOD_NAME}.""" + return "HTTP " + environ.get("REQUEST_METHOD") class OpenTelemetryMiddleware: @@ -158,11 +154,15 @@ class OpenTelemetryMiddleware: Args: wsgi: The WSGI application callable to forward requests to. + name_callback: Callback which calculates a generic span name for an + incoming HTTP request based on the PEP3333 WSGI environ. + Optional: Defaults to get_default_span_name. """ - def __init__(self, wsgi): + def __init__(self, wsgi, name_callback=None): self.wsgi = wsgi self.tracer = trace.tracer_source().get_tracer(__name__, __version__) + self.name_callback = name_callback or get_default_span_name @staticmethod def _create_start_response(span, start_response): @@ -182,7 +182,7 @@ def __call__(self, environ, start_response): """ parent_span = propagators.extract(get_header_from_environ, environ) - span_name = get_default_span_name(environ) + span_name = self.name_callback(environ) span = self.tracer.start_span( span_name, diff --git a/ext/opentelemetry-ext-wsgi/tests/test_wsgi_middleware.py b/ext/opentelemetry-ext-wsgi/tests/test_wsgi_middleware.py index 1912dd0079f..1a0a5e7ddfe 100644 --- a/ext/opentelemetry-ext-wsgi/tests/test_wsgi_middleware.py +++ b/ext/opentelemetry-ext-wsgi/tests/test_wsgi_middleware.py @@ -74,7 +74,7 @@ def error_wsgi(environ, start_response): class TestWsgiApplication(WsgiTestBase): - def validate_response(self, response, error=None): + def validate_response(self, response, error=None, span_name="HTTP GET"): while True: try: value = next(response) @@ -95,7 +95,7 @@ def validate_response(self, response, error=None): span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) - self.assertEqual(span_list[0].name, "/") + self.assertEqual(span_list[0].name, span_name) self.assertEqual(span_list[0].kind, trace_api.SpanKind.SERVER) self.assertEqual( span_list[0].attributes, @@ -147,6 +147,17 @@ def test_wsgi_exc_info(self): response = app(self.environ, self.start_response) self.validate_response(response, error=ValueError) + def test_override_span_name(self): + """Test that span_names can be overwritten by our callback function.""" + span_name = "Dymaxion" + def get_predefined_span_name(scope): + return span_name + app = otel_wsgi.OpenTelemetryMiddleware( + simple_wsgi, name_callback=get_predefined_span_name + ) + response = app(self.environ, self.start_response) + self.validate_response(response, span_name=span_name) + class TestWsgiAttributes(unittest.TestCase): def setUp(self): From 804d4a442cd2445cfdbc9aff87816d0aa63513e0 Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Wed, 17 Jun 2020 21:13:46 -0400 Subject: [PATCH 2/9] use default function --- .../src/opentelemetry/ext/wsgi/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py b/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py index 7b7af5fce32..53ee2ecb949 100644 --- a/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py +++ b/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py @@ -166,10 +166,10 @@ class OpenTelemetryMiddleware: Optional: Defaults to get_default_span_name. """ - def __init__(self, wsgi, name_callback=None): + def __init__(self, wsgi, name_callback=get_default_span_name): self.wsgi = wsgi self.tracer = trace.get_tracer(__name__, __version__) - self.name_callback = name_callback or get_default_span_name + self.name_callback = name_callback @staticmethod def _create_start_response(span, start_response): From 18bde06a182df2880c15857d78f6f05d06cd2f6b Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Wed, 17 Jun 2020 21:32:09 -0400 Subject: [PATCH 3/9] update flask and pyramid --- ext/opentelemetry-ext-flask/tests/test_programmatic.py | 2 +- ext/opentelemetry-ext-pyramid/tests/test_programmatic.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/opentelemetry-ext-flask/tests/test_programmatic.py b/ext/opentelemetry-ext-flask/tests/test_programmatic.py index 95f82373e3d..30fa9c4eb24 100644 --- a/ext/opentelemetry-ext-flask/tests/test_programmatic.py +++ b/ext/opentelemetry-ext-flask/tests/test_programmatic.py @@ -118,7 +118,7 @@ def test_404(self): resp.close() span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) - self.assertEqual(span_list[0].name, "/bye") + self.assertEqual(span_list[0].name, "HTTP POST") self.assertEqual(span_list[0].kind, trace.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) diff --git a/ext/opentelemetry-ext-pyramid/tests/test_programmatic.py b/ext/opentelemetry-ext-pyramid/tests/test_programmatic.py index b1ecbb38bb7..e8937e8f5e6 100644 --- a/ext/opentelemetry-ext-pyramid/tests/test_programmatic.py +++ b/ext/opentelemetry-ext-pyramid/tests/test_programmatic.py @@ -101,7 +101,7 @@ def test_404(self): resp.close() span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) - self.assertEqual(span_list[0].name, "/bye") + self.assertEqual(span_list[0].name, "HTTP POST") self.assertEqual(span_list[0].kind, trace.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) From f15652d3574cb9593ab0b1158fb7ab7452a302ff Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Thu, 18 Jun 2020 09:22:44 -0400 Subject: [PATCH 4/9] lint fix --- ext/opentelemetry-ext-wsgi/tests/test_wsgi_middleware.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ext/opentelemetry-ext-wsgi/tests/test_wsgi_middleware.py b/ext/opentelemetry-ext-wsgi/tests/test_wsgi_middleware.py index 98b3fdb8042..6ce0ef550bd 100644 --- a/ext/opentelemetry-ext-wsgi/tests/test_wsgi_middleware.py +++ b/ext/opentelemetry-ext-wsgi/tests/test_wsgi_middleware.py @@ -150,8 +150,11 @@ def test_wsgi_exc_info(self): def test_override_span_name(self): """Test that span_names can be overwritten by our callback function.""" span_name = "Dymaxion" + def get_predefined_span_name(scope): + # pylint: disable=unused-argument return span_name + app = otel_wsgi.OpenTelemetryMiddleware( simple_wsgi, name_callback=get_predefined_span_name ) From 84b2057ed397fe15c99e9bd0bb0da5844c5fefe2 Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Mon, 22 Jun 2020 09:17:03 -0400 Subject: [PATCH 5/9] handle empty request method --- .../src/opentelemetry/ext/wsgi/__init__.py | 16 +++++--- .../tests/test_wsgi_middleware.py | 40 +++++++++++-------- 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py b/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py index 53ee2ecb949..0a3e30a49f3 100644 --- a/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py +++ b/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py @@ -93,10 +93,10 @@ def collect_request_attributes(environ): result = { "component": "http", - "http.method": environ["REQUEST_METHOD"], - "http.server_name": environ["SERVER_NAME"], - "http.scheme": environ["wsgi.url_scheme"], - "host.port": int(environ["SERVER_PORT"]), + "http.method": environ.get("REQUEST_METHOD"), + "http.server_name": environ.get("SERVER_NAME"), + "http.scheme": environ.get("wsgi.url_scheme"), + "host.port": int(environ.get("SERVER_PORT")), } setifnotnone(result, "http.host", environ.get("HTTP_HOST")) @@ -150,7 +150,12 @@ def add_response_attributes( def get_default_span_name(environ): """Default implementation for name_callback, returns HTTP {METHOD_NAME}.""" - return "HTTP " + environ.get("REQUEST_METHOD") + request_method = environ.get("REQUEST_METHOD") + return "HTTP" + ( + " " + request_method + if request_method is not None and request_method != "" + else "" + ) class OpenTelemetryMiddleware: @@ -191,7 +196,6 @@ def __call__(self, environ, start_response): token = context.attach( propagators.extract(get_header_from_environ, environ) ) - span_name = get_default_span_name(environ) span_name = self.name_callback(environ) span = self.tracer.start_span( diff --git a/ext/opentelemetry-ext-wsgi/tests/test_wsgi_middleware.py b/ext/opentelemetry-ext-wsgi/tests/test_wsgi_middleware.py index 6ce0ef550bd..0d018b68414 100644 --- a/ext/opentelemetry-ext-wsgi/tests/test_wsgi_middleware.py +++ b/ext/opentelemetry-ext-wsgi/tests/test_wsgi_middleware.py @@ -74,7 +74,9 @@ def error_wsgi(environ, start_response): class TestWsgiApplication(WsgiTestBase): - def validate_response(self, response, error=None, span_name="HTTP GET"): + def validate_response( + self, response, error=None, span_name="HTTP GET", http_method="GET" + ): while True: try: value = next(response) @@ -97,21 +99,20 @@ def validate_response(self, response, error=None, span_name="HTTP GET"): self.assertEqual(len(span_list), 1) self.assertEqual(span_list[0].name, span_name) self.assertEqual(span_list[0].kind, trace_api.SpanKind.SERVER) - self.assertEqual( - span_list[0].attributes, - { - "component": "http", - "http.method": "GET", - "http.server_name": "127.0.0.1", - "http.scheme": "http", - "host.port": 80, - "http.host": "127.0.0.1", - "http.flavor": "1.0", - "http.url": "http://127.0.0.1/", - "http.status_text": "OK", - "http.status_code": 200, - }, - ) + expected_attributes = { + "component": "http", + "http.server_name": "127.0.0.1", + "http.scheme": "http", + "host.port": 80, + "http.host": "127.0.0.1", + "http.flavor": "1.0", + "http.url": "http://127.0.0.1/", + "http.status_text": "OK", + "http.status_code": 200, + } + if http_method is not None: + expected_attributes["http.method"] = http_method + self.assertEqual(span_list[0].attributes, expected_attributes) def test_basic_wsgi_call(self): app = otel_wsgi.OpenTelemetryMiddleware(simple_wsgi) @@ -161,6 +162,13 @@ def get_predefined_span_name(scope): response = app(self.environ, self.start_response) self.validate_response(response, span_name=span_name) + def test_default_span_name_missing_request_method(self): + """Test that default span_names with missing request method.""" + self.environ.pop("REQUEST_METHOD") + app = otel_wsgi.OpenTelemetryMiddleware(simple_wsgi) + response = app(self.environ, self.start_response) + self.validate_response(response, span_name="HTTP", http_method=None) + class TestWsgiAttributes(unittest.TestCase): def setUp(self): From a5016eecc7086b0fa28cdbbae3d7d5c97b401e09 Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Mon, 22 Jun 2020 12:33:41 -0400 Subject: [PATCH 6/9] Update ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py Co-authored-by: Diego Hurtado --- .../src/opentelemetry/ext/wsgi/__init__.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py b/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py index 0a3e30a49f3..7d30614ddc2 100644 --- a/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py +++ b/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py @@ -150,11 +150,7 @@ def add_response_attributes( def get_default_span_name(environ): """Default implementation for name_callback, returns HTTP {METHOD_NAME}.""" - request_method = environ.get("REQUEST_METHOD") - return "HTTP" + ( - " " + request_method - if request_method is not None and request_method != "" - else "" + return "HTTP {}".format(environ.get("REQUEST_METHOD", "")).strip() ) From c1ce80a916f05925456339391b306daf62acc768 Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Mon, 22 Jun 2020 13:00:03 -0400 Subject: [PATCH 7/9] handle int server port --- .../src/opentelemetry/ext/wsgi/__init__.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py b/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py index 7d30614ddc2..78cd24b7c1d 100644 --- a/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py +++ b/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py @@ -96,9 +96,14 @@ def collect_request_attributes(environ): "http.method": environ.get("REQUEST_METHOD"), "http.server_name": environ.get("SERVER_NAME"), "http.scheme": environ.get("wsgi.url_scheme"), - "host.port": int(environ.get("SERVER_PORT")), } + host_port = environ.get("SERVER_PORT") + if host_port is not None: + if isinstance(host_port, str): + host_port = int(host_port) + result.update({"host.port": host_port}) + setifnotnone(result, "http.host", environ.get("HTTP_HOST")) target = environ.get("RAW_URI") if target is None: # Note: `"" or None is None` @@ -151,7 +156,6 @@ def add_response_attributes( def get_default_span_name(environ): """Default implementation for name_callback, returns HTTP {METHOD_NAME}.""" return "HTTP {}".format(environ.get("REQUEST_METHOD", "")).strip() - ) class OpenTelemetryMiddleware: From 8e4584c244c3932a824942272613d65f1ed05eae Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Mon, 22 Jun 2020 13:23:51 -0400 Subject: [PATCH 8/9] fix lint --- .../src/opentelemetry/ext/wsgi/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py b/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py index 78cd24b7c1d..0531f6a38e1 100644 --- a/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py +++ b/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py @@ -103,7 +103,7 @@ def collect_request_attributes(environ): if isinstance(host_port, str): host_port = int(host_port) result.update({"host.port": host_port}) - + setifnotnone(result, "http.host", environ.get("HTTP_HOST")) target = environ.get("RAW_URI") if target is None: # Note: `"" or None is None` From c60f32ad9b15044d6ae1d2970f01c73b3351e1ce Mon Sep 17 00:00:00 2001 From: "Tahir H. Butt" Date: Mon, 22 Jun 2020 13:41:17 -0400 Subject: [PATCH 9/9] assume port is string --- .../src/opentelemetry/ext/wsgi/__init__.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py b/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py index 0531f6a38e1..9968a0c1c83 100644 --- a/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py +++ b/ext/opentelemetry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py @@ -100,9 +100,7 @@ def collect_request_attributes(environ): host_port = environ.get("SERVER_PORT") if host_port is not None: - if isinstance(host_port, str): - host_port = int(host_port) - result.update({"host.port": host_port}) + result.update({"host.port": int(host_port)}) setifnotnone(result, "http.host", environ.get("HTTP_HOST")) target = environ.get("RAW_URI")