From bb004d0527a1fea431cbaf333f4ba46f7b250fcc Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 25 Oct 2021 13:12:19 -0700 Subject: [PATCH 1/5] server --- .../aiohttp_client/__init__.py | 2 +- .../instrumentation/asgi/__init__.py | 2 +- .../instrumentation/falcon/__init__.py | 2 +- .../instrumentation/tornado/__init__.py | 2 +- .../instrumentation/wsgi/__init__.py | 2 +- .../opentelemetry/instrumentation/utils.py | 4 ++- .../tests/test_utils.py | 34 +++++++++++++++++++ 7 files changed, 42 insertions(+), 6 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py b/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py index a89b3a2220..55700288d4 100644 --- a/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py @@ -238,7 +238,7 @@ async def on_request_end( if trace_config_ctx.span.is_recording(): trace_config_ctx.span.set_status( - Status(http_status_to_status_code(int(params.response.status))) + Status(http_status_to_status_code(int(params.response.status), server_span=True)) ) trace_config_ctx.span.set_attribute( SpanAttributes.HTTP_STATUS_CODE, params.response.status diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index df7af306d7..e28ee1bea2 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -218,7 +218,7 @@ def set_status_code(span, status_code): ) else: span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, status_code) - span.set_status(Status(http_status_to_status_code(status_code))) + span.set_status(Status(http_status_to_status_code(status_code, server_span=True))) def get_default_span_details(scope: dict) -> Tuple[str, dict]: diff --git a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py index efe4dd86c6..58ea05b469 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -299,7 +299,7 @@ def process_response( span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, status_code) span.set_status( Status( - status_code=http_status_to_status_code(status_code), + status_code=http_status_to_status_code(status_code, server_span=True), description=reason, ) ) diff --git a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py index 9e99b67104..7efe1a6f11 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py @@ -345,7 +345,7 @@ def _finish_span(tracer, handler, error=None): if ctx.span.is_recording(): ctx.span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, status_code) - otel_status_code = http_status_to_status_code(status_code) + otel_status_code = http_status_to_status_code(status_code, server_span=True) otel_status_description = None if otel_status_code is StatusCode.ERROR: otel_status_description = reason diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py index 3c971e4482..61b64e4d02 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -226,7 +226,7 @@ def add_response_attributes( ) else: span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, status_code) - span.set_status(Status(http_status_to_status_code(status_code))) + span.set_status(Status(http_status_to_status_code(status_code, server_span=True))) def get_default_span_name(environ): diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py index 45741f0821..7e10416410 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py @@ -36,7 +36,7 @@ def extract_attributes_from_object( def http_status_to_status_code( - status: int, allow_redirect: bool = True + status: int, allow_redirect: bool = True, server_span: bool = False, ) -> StatusCode: """Converts an HTTP status code to an OpenTelemetry canonical status code @@ -50,6 +50,8 @@ def http_status_to_status_code( return StatusCode.UNSET if status <= 399 and allow_redirect: return StatusCode.UNSET + if status <= 499 and server_span: + return StatusCode.UNSET return StatusCode.ERROR diff --git a/opentelemetry-instrumentation/tests/test_utils.py b/opentelemetry-instrumentation/tests/test_utils.py index 273c6f085c..1fbe155b9a 100644 --- a/opentelemetry-instrumentation/tests/test_utils.py +++ b/opentelemetry-instrumentation/tests/test_utils.py @@ -43,3 +43,37 @@ def test_http_status_to_status_code(self): with self.subTest(status_code=status_code): actual = http_status_to_status_code(int(status_code)) self.assertEqual(actual, expected, status_code) + + def test_http_status_to_status_code_redirect(self): + for status_code, expected in ( + (HTTPStatus.MULTIPLE_CHOICES, StatusCode.ERROR), + (HTTPStatus.MOVED_PERMANENTLY, StatusCode.ERROR), + (HTTPStatus.TEMPORARY_REDIRECT, StatusCode.ERROR), + (HTTPStatus.PERMANENT_REDIRECT, StatusCode.ERROR), + ): + with self.subTest(status_code=status_code): + actual = http_status_to_status_code(int(status_code), allow_redirect=False) + self.assertEqual(actual, expected, status_code) + + def test_http_status_to_status_code_server(self): + for status_code, expected in ( + (HTTPStatus.OK, StatusCode.UNSET), + (HTTPStatus.ACCEPTED, StatusCode.UNSET), + (HTTPStatus.IM_USED, StatusCode.UNSET), + (HTTPStatus.MULTIPLE_CHOICES, StatusCode.UNSET), + (HTTPStatus.BAD_REQUEST, StatusCode.UNSET), + (HTTPStatus.UNAUTHORIZED, StatusCode.UNSET), + (HTTPStatus.FORBIDDEN, StatusCode.UNSET), + (HTTPStatus.NOT_FOUND, StatusCode.UNSET), + (HTTPStatus.UNPROCESSABLE_ENTITY, StatusCode.UNSET,), + (HTTPStatus.TOO_MANY_REQUESTS, StatusCode.UNSET,), + (HTTPStatus.NOT_IMPLEMENTED, StatusCode.ERROR), + (HTTPStatus.SERVICE_UNAVAILABLE, StatusCode.ERROR), + (HTTPStatus.GATEWAY_TIMEOUT, StatusCode.ERROR,), + (HTTPStatus.HTTP_VERSION_NOT_SUPPORTED, StatusCode.ERROR,), + (600, StatusCode.ERROR), + (99, StatusCode.ERROR), + ): + with self.subTest(status_code=status_code): + actual = http_status_to_status_code(int(status_code), server_span=True) + self.assertEqual(actual, expected, status_code) From edbfb655cbe502131ab84e6de2e882e56481e95c Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 25 Oct 2021 13:24:34 -0700 Subject: [PATCH 2/5] lint --- CHANGELOG.md | 2 ++ .../instrumentation/aiohttp_client/__init__.py | 6 +++++- .../src/opentelemetry/instrumentation/asgi/__init__.py | 4 +++- .../src/opentelemetry/instrumentation/falcon/__init__.py | 4 +++- .../src/opentelemetry/instrumentation/tornado/__init__.py | 4 +++- .../src/opentelemetry/instrumentation/wsgi/__init__.py | 4 +++- opentelemetry-instrumentation/tests/test_utils.py | 8 ++++++-- 7 files changed, 25 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 941fa396f9..a855213972 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#774](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/774)) - `opentelemetry-instrumentation-django` Fixed carrier usage on ASGI requests. ([#767](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/767)) +- Don't set Span Status on 4xx http status code for SpanKind.SERVER spans + ([#776](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/776)) ## [1.6.2-0.25b2](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.6.2-0.25b2) - 2021-10-19 diff --git a/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py b/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py index 55700288d4..604c41d96e 100644 --- a/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py @@ -238,7 +238,11 @@ async def on_request_end( if trace_config_ctx.span.is_recording(): trace_config_ctx.span.set_status( - Status(http_status_to_status_code(int(params.response.status), server_span=True)) + Status( + http_status_to_status_code( + int(params.response.status), server_span=True + ) + ) ) trace_config_ctx.span.set_attribute( SpanAttributes.HTTP_STATUS_CODE, params.response.status diff --git a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index e28ee1bea2..e077743a18 100644 --- a/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py @@ -218,7 +218,9 @@ def set_status_code(span, status_code): ) else: span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, status_code) - span.set_status(Status(http_status_to_status_code(status_code, server_span=True))) + span.set_status( + Status(http_status_to_status_code(status_code, server_span=True)) + ) def get_default_span_details(scope: dict) -> Tuple[str, dict]: diff --git a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py index 58ea05b469..58d241f0b0 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/src/opentelemetry/instrumentation/falcon/__init__.py @@ -299,7 +299,9 @@ def process_response( span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, status_code) span.set_status( Status( - status_code=http_status_to_status_code(status_code, server_span=True), + status_code=http_status_to_status_code( + status_code, server_span=True + ), description=reason, ) ) diff --git a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py index 7efe1a6f11..03572ea553 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py @@ -345,7 +345,9 @@ def _finish_span(tracer, handler, error=None): if ctx.span.is_recording(): ctx.span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, status_code) - otel_status_code = http_status_to_status_code(status_code, server_span=True) + otel_status_code = http_status_to_status_code( + status_code, server_span=True + ) otel_status_description = None if otel_status_code is StatusCode.ERROR: otel_status_description = reason diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py index 61b64e4d02..54c4705f8e 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -226,7 +226,9 @@ def add_response_attributes( ) else: span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, status_code) - span.set_status(Status(http_status_to_status_code(status_code, server_span=True))) + span.set_status( + Status(http_status_to_status_code(status_code, server_span=True)) + ) def get_default_span_name(environ): diff --git a/opentelemetry-instrumentation/tests/test_utils.py b/opentelemetry-instrumentation/tests/test_utils.py index 1fbe155b9a..8679a43f78 100644 --- a/opentelemetry-instrumentation/tests/test_utils.py +++ b/opentelemetry-instrumentation/tests/test_utils.py @@ -52,7 +52,9 @@ def test_http_status_to_status_code_redirect(self): (HTTPStatus.PERMANENT_REDIRECT, StatusCode.ERROR), ): with self.subTest(status_code=status_code): - actual = http_status_to_status_code(int(status_code), allow_redirect=False) + actual = http_status_to_status_code( + int(status_code), allow_redirect=False + ) self.assertEqual(actual, expected, status_code) def test_http_status_to_status_code_server(self): @@ -75,5 +77,7 @@ def test_http_status_to_status_code_server(self): (99, StatusCode.ERROR), ): with self.subTest(status_code=status_code): - actual = http_status_to_status_code(int(status_code), server_span=True) + actual = http_status_to_status_code( + int(status_code), server_span=True + ) self.assertEqual(actual, expected, status_code) From 3df8de4026f3ed78061ac868f29e91f35805b488 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 25 Oct 2021 13:37:42 -0700 Subject: [PATCH 3/5] Update test_falcon.py --- .../opentelemetry-instrumentation-falcon/tests/test_falcon.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py index 677ea313b0..8fbae7aecc 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py @@ -114,7 +114,7 @@ def test_404(self): self.assertEqual(len(spans), 1) span = spans[0] self.assertEqual(span.name, "HTTP GET") - self.assertEqual(span.status.status_code, StatusCode.ERROR) + self.assertEqual(span.status.status_code, StatusCode.UNSET) self.assertEqual( span.status.description, "NotFound", ) From 90450105e1031a4f8c055724dcf3934f5ddc4e00 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Mon, 25 Oct 2021 14:28:00 -0700 Subject: [PATCH 4/5] Update test_falcon.py --- .../opentelemetry-instrumentation-falcon/tests/test_falcon.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py index 8fbae7aecc..9e3fc0ec31 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py @@ -115,9 +115,6 @@ def test_404(self): span = spans[0] self.assertEqual(span.name, "HTTP GET") self.assertEqual(span.status.status_code, StatusCode.UNSET) - self.assertEqual( - span.status.description, "NotFound", - ) self.assertSpanHasAttributes( span, { From 5d4c78aa380618d9b1b560177697b49d90314b75 Mon Sep 17 00:00:00 2001 From: Leighton Chen Date: Tue, 26 Oct 2021 09:55:44 -0700 Subject: [PATCH 5/5] Update __init__.py --- .../instrumentation/aiohttp_client/__init__.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py b/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py index 604c41d96e..a89b3a2220 100644 --- a/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-aiohttp-client/src/opentelemetry/instrumentation/aiohttp_client/__init__.py @@ -238,11 +238,7 @@ async def on_request_end( if trace_config_ctx.span.is_recording(): trace_config_ctx.span.set_status( - Status( - http_status_to_status_code( - int(params.response.status), server_span=True - ) - ) + Status(http_status_to_status_code(int(params.response.status))) ) trace_config_ctx.span.set_attribute( SpanAttributes.HTTP_STATUS_CODE, params.response.status