diff --git a/CHANGELOG.md b/CHANGELOG.md index d3a746c5f..8302383ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,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-asgi/src/opentelemetry/instrumentation/asgi/__init__.py b/instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py index df7af306d..e077743a1 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))) + 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 efe4dd86c..58d241f0b 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 @@ class _TraceMiddleware: 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-falcon/tests/test_falcon.py b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py index 677ea313b..9e3fc0ec3 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py @@ -114,10 +114,7 @@ class TestFalconInstrumentation(TestFalconBase, WsgiTestBase): 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.description, "NotFound", - ) + self.assertEqual(span.status.status_code, StatusCode.UNSET) self.assertSpanHasAttributes( span, { 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 9e99b6710..03572ea55 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) + 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 3c971e448..54c4705f8 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))) + 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 45741f082..7e1041641 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 273c6f085..8679a43f7 100644 --- a/opentelemetry-instrumentation/tests/test_utils.py +++ b/opentelemetry-instrumentation/tests/test_utils.py @@ -43,3 +43,41 @@ class TestUtils(TestBase): 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)