From ccc57a67cb56ed550125f88214c34a36c148324e Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Mon, 5 Apr 2021 16:42:33 +0530 Subject: [PATCH 1/4] Remove http.status_text from span attributes --- .../opentelemetry/instrumentation/aiohttp_client/__init__.py | 3 --- .../tests/test_aiohttp_client_integration.py | 4 ---- .../tests/test_middleware.py | 3 --- .../tests/test_falcon.py | 2 -- .../tests/test_programmatic.py | 3 --- .../tests/test_programmatic.py | 3 --- .../src/opentelemetry/instrumentation/requests/__init__.py | 1 - .../tests/test_requests_integration.py | 4 ---- .../src/opentelemetry/instrumentation/tornado/__init__.py | 2 -- .../tests/test_instrumentation.py | 4 ---- .../src/opentelemetry/instrumentation/urllib/__init__.py | 1 - .../tests/test_urllib_integration.py | 4 ---- .../src/opentelemetry/instrumentation/urllib3/__init__.py | 1 - .../tests/test_urllib3_integration.py | 2 -- .../src/opentelemetry/instrumentation/wsgi/__init__.py | 3 +-- .../tests/test_wsgi_middleware.py | 5 ----- 16 files changed, 1 insertion(+), 44 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 7d1ad04355..18de3e84ea 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 @@ -198,9 +198,6 @@ async def on_request_end( trace_config_ctx.span.set_attribute( "http.status_code", params.response.status ) - trace_config_ctx.span.set_attribute( - "http.status_text", params.response.reason - ) _end_trace(trace_config_ctx) async def on_request_exception( diff --git a/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py b/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py index 33708dfe4e..8df65597fe 100644 --- a/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py +++ b/instrumentation/opentelemetry-instrumentation-aiohttp-client/tests/test_aiohttp_client_integration.py @@ -134,7 +134,6 @@ def test_status_codes(self): host, port ), "http.status_code": int(status_code), - "http.status_text": status_code.phrase, }, ) ] @@ -191,7 +190,6 @@ def test_span_name_option(self): host, port, path ), "http.status_code": int(HTTPStatus.OK), - "http.status_text": HTTPStatus.OK.phrase, }, ) ] @@ -222,7 +220,6 @@ def strip_query_params(url: yarl.URL) -> str: host, port ), "http.status_code": int(HTTPStatus.OK), - "http.status_text": HTTPStatus.OK.phrase, }, ) ] @@ -361,7 +358,6 @@ def test_instrument(self): span.attributes["http.url"], ) self.assertEqual(200, span.attributes["http.status_code"]) - self.assertEqual("OK", span.attributes["http.status_text"]) def test_instrument_with_existing_trace_config(self): trace_config = aiohttp.TraceConfig() diff --git a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py index 154e68bc15..d50aaca7c9 100644 --- a/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-django/tests/test_middleware.py @@ -116,7 +116,6 @@ def test_templated_route_get(self): ) self.assertEqual(span.attributes["http.scheme"], "http") self.assertEqual(span.attributes["http.status_code"], 200) - self.assertEqual(span.attributes["http.status_text"], "OK") def test_traced_get(self): Client().get("/traced/") @@ -138,7 +137,6 @@ def test_traced_get(self): self.assertEqual(span.attributes["http.route"], "^traced/") self.assertEqual(span.attributes["http.scheme"], "http") self.assertEqual(span.attributes["http.status_code"], 200) - self.assertEqual(span.attributes["http.status_text"], "OK") def test_not_recording(self): mock_tracer = Mock() @@ -173,7 +171,6 @@ def test_traced_post(self): self.assertEqual(span.attributes["http.route"], "^traced/") self.assertEqual(span.attributes["http.scheme"], "http") self.assertEqual(span.attributes["http.status_code"], 200) - self.assertEqual(span.attributes["http.status_text"], "OK") def test_error(self): with self.assertRaises(ValueError): diff --git a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py index 1b68c3d075..74882fd681 100644 --- a/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py +++ b/instrumentation/opentelemetry-instrumentation-falcon/tests/test_falcon.py @@ -104,7 +104,6 @@ def _test_method(self, method): "net.peer.port": "65133", "http.flavor": "1.1", "falcon.resource": "HelloWorldResource", - "http.status_text": "Created", "http.status_code": 201, }, ) @@ -129,7 +128,6 @@ def test_404(self): "net.peer.ip": "127.0.0.1", "net.peer.port": "65133", "http.flavor": "1.1", - "http.status_text": "Not Found", "http.status_code": 404, }, ) diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index 2b78a7b9f5..3c62dd751d 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -35,7 +35,6 @@ def expected_attributes(override_attributes): "http.host": "localhost", "http.target": "/", "http.flavor": "1.1", - "http.status_text": "OK", "http.status_code": 200, } for key, val in override_attributes.items(): @@ -138,7 +137,6 @@ def test_404(self): { "http.method": "POST", "http.target": "/bye", - "http.status_text": "NOT FOUND", "http.status_code": 404, } ) @@ -157,7 +155,6 @@ def test_internal_error(self): { "http.target": "/hello/500", "http.route": "/hello/", - "http.status_text": "INTERNAL SERVER ERROR", "http.status_code": 500, } ) diff --git a/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py index a4f2aeba1d..ea3fd266a1 100644 --- a/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-pyramid/tests/test_programmatic.py @@ -35,7 +35,6 @@ def expected_attributes(override_attributes): "http.host": "localhost", "http.target": "/", "http.flavor": "1.1", - "http.status_text": "OK", "http.status_code": 200, } for key, val in override_attributes.items(): @@ -118,7 +117,6 @@ def test_404(self): { "http.method": "POST", "http.target": "/bye", - "http.status_text": "Not Found", "http.status_code": 404, } ) @@ -137,7 +135,6 @@ def test_internal_error(self): { "http.target": "/hello/500", "http.route": "/hello/{helloid}", - "http.status_text": "Internal Server Error", "http.status_code": 500, } ) diff --git a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py index 0735eba622..e70a6cf9d7 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -150,7 +150,6 @@ def _instrumented_requests_call( if isinstance(result, Response): if span.is_recording(): span.set_attribute("http.status_code", result.status_code) - span.set_attribute("http.status_text", result.reason) span.set_status( Status(http_status_to_status_code(result.status_code)) ) diff --git a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py index ecc56b7c54..16f95395a6 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py +++ b/instrumentation/opentelemetry-instrumentation-requests/tests/test_requests_integration.py @@ -82,7 +82,6 @@ def test_basic(self): "http.method": "GET", "http.url": self.URL, "http.status_code": 200, - "http.status_text": "OK", }, ) @@ -127,7 +126,6 @@ def test_not_foundbasic(self): span = self.assert_span() self.assertEqual(span.attributes.get("http.status_code"), 404) - self.assertEqual(span.attributes.get("http.status_text"), "Not Found") self.assertIs( span.status.status_code, trace.StatusCode.ERROR, @@ -237,7 +235,6 @@ def span_callback(span, result: requests.Response): "http.method": "GET", "http.url": self.URL, "http.status_code": 200, - "http.status_text": "OK", "http.response.body": "Hello!", }, ) @@ -306,7 +303,6 @@ def test_requests_exception_with_response(self, *_, **__): "http.method": "GET", "http.url": self.URL, "http.status_code": 500, - "http.status_text": "Internal Server Error", }, ) self.assertEqual(span.status.status_code, StatusCode.ERROR) 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 05e850e42d..5b7654b593 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py @@ -238,8 +238,6 @@ def _finish_span(tracer, handler, error=None): return if ctx.span.is_recording(): - if reason: - ctx.span.set_attribute("http.status_text", reason) ctx.span.set_attribute("http.status_code", status_code) ctx.span.set_status( Status( diff --git a/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py b/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py index d760307f09..d7e167b722 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py @@ -132,7 +132,6 @@ def _test_http_method_call(self, method): "http.host": "127.0.0.1:" + str(self.get_http_port()), "http.target": "/", "net.peer.ip": "127.0.0.1", - "http.status_text": "Created", "http.status_code": 201, }, ) @@ -205,7 +204,6 @@ def _test_async_handler(self, url, handler_name): "http.host": "127.0.0.1:" + str(self.get_http_port()), "http.target": url, "net.peer.ip": "127.0.0.1", - "http.status_text": "Created", "http.status_code": 201, }, ) @@ -274,7 +272,6 @@ def test_404(self): "http.host": "127.0.0.1:" + str(self.get_http_port()), "http.target": "/missing-url", "net.peer.ip": "127.0.0.1", - "http.status_text": "Not Found", "http.status_code": 404, }, ) @@ -318,7 +315,6 @@ def test_dynamic_handler(self): "http.host": "127.0.0.1:" + str(self.get_http_port()), "http.target": "/dyna", "net.peer.ip": "127.0.0.1", - "http.status_text": "Accepted", "http.status_code": 202, }, ) diff --git a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py index 1ef29012af..958f32685d 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py @@ -171,7 +171,6 @@ def _instrumented_open_call( if span.is_recording(): span.set_attribute("http.status_code", code_) - span.set_attribute("http.status_text", result.reason) span.set_status(Status(http_status_to_status_code(code_))) ver_ = str(getattr(result, "version", "")) diff --git a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py index e5a1092689..d88c4f246d 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py @@ -107,7 +107,6 @@ def test_basic(self): "http.method": "GET", "http.url": self.URL, "http.status_code": 200, - "http.status_text": "OK", }, ) @@ -158,7 +157,6 @@ def test_not_foundbasic(self): span = self.assert_span() self.assertEqual(span.attributes.get("http.status_code"), 404) - self.assertEqual(span.attributes.get("http.status_text"), "Not Found") self.assertIs( span.status.status_code, trace.StatusCode.ERROR, @@ -271,7 +269,6 @@ def span_callback(span, result: HTTPResponse): "http.method": "GET", "http.url": self.URL, "http.status_code": 200, - "http.status_text": "OK", "http.response.body": "Hello!", }, ) @@ -301,7 +298,6 @@ def test_requests_exception_with_response(self, *_, **__): "http.method": "GET", "http.url": "http://httpbin.org/status/500", "http.status_code": 500, - "http.status_text": "Internal Server Error", }, ) self.assertEqual(span.status.status_code, StatusCode.ERROR) diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py b/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py index 8c24854207..33e6ffa679 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py @@ -203,7 +203,6 @@ def _apply_response(span: Span, response: urllib3.response.HTTPResponse): return span.set_attribute("http.status_code", response.status) - span.set_attribute("http.status_text", response.reason) span.set_status(Status(http_status_to_status_code(response.status))) diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py index aee3d4bc0e..ee878b97db 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py @@ -72,7 +72,6 @@ def assert_success_span( "http.method": "GET", "http.url": url, "http.status_code": 200, - "http.status_text": "OK", } self.assertEqual(attributes, span.attributes) @@ -127,7 +126,6 @@ def test_basic_not_found(self): span = self.assert_span() self.assertEqual(404, span.attributes.get("http.status_code")) - self.assertEqual("Not Found", span.attributes.get("http.status_text")) self.assertIs(trace.status.StatusCode.ERROR, span.status.status_code) def test_basic_http_non_default_port(self): 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 c3be93c5bd..c57aca11b5 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -157,8 +157,7 @@ def add_response_attributes( passed to a PEP3333-conforming start_response callable.""" if not span.is_recording(): return - status_code, status_text = start_response_status.split(" ", 1) - span.set_attribute("http.status_text", status_text) + status_code, _ = start_response_status.split(" ", 1) try: status_code = int(status_code) diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py index 3bfbe8c36c..6605e22a05 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py @@ -112,7 +112,6 @@ def validate_response( "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: @@ -339,7 +338,6 @@ def test_response_attributes(self): otel_wsgi.add_response_attributes(self.span, "404 Not Found", {}) expected = ( mock.call("http.status_code", 404), - mock.call("http.status_text", "Not Found"), ) self.assertEqual(self.span.set_attribute.call_count, len(expected)) self.span.set_attribute.assert_has_calls(expected, any_order=True) @@ -347,9 +345,6 @@ def test_response_attributes(self): def test_response_attributes_invalid_status_code(self): otel_wsgi.add_response_attributes(self.span, "Invalid Status Code", {}) self.assertEqual(self.span.set_attribute.call_count, 1) - self.span.set_attribute.assert_called_with( - "http.status_text", "Status Code" - ) if __name__ == "__main__": From d7f67baefa9534a2129cfe37662bb2474314d569 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Mon, 5 Apr 2021 18:48:50 +0530 Subject: [PATCH 2/4] Remove test for invalid status text --- .../tests/test_wsgi_middleware.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py index 6605e22a05..f413cc2bf4 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py @@ -342,10 +342,6 @@ def test_response_attributes(self): self.assertEqual(self.span.set_attribute.call_count, len(expected)) self.span.set_attribute.assert_has_calls(expected, any_order=True) - def test_response_attributes_invalid_status_code(self): - otel_wsgi.add_response_attributes(self.span, "Invalid Status Code", {}) - self.assertEqual(self.span.set_attribute.call_count, 1) - if __name__ == "__main__": unittest.main() From 08719e1b4c707a53967c6700eeeffa231533d590 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Mon, 5 Apr 2021 19:12:23 +0530 Subject: [PATCH 3/4] Fix lint --- .../tests/test_wsgi_middleware.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py index f413cc2bf4..1f52a7d7f3 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py @@ -336,9 +336,7 @@ def test_http_user_agent_attribute(self): def test_response_attributes(self): otel_wsgi.add_response_attributes(self.span, "404 Not Found", {}) - expected = ( - mock.call("http.status_code", 404), - ) + expected = (mock.call("http.status_code", 404),) self.assertEqual(self.span.set_attribute.call_count, len(expected)) self.span.set_attribute.assert_has_calls(expected, any_order=True) From 383a7ae0879f3b858a87bfbf3e3806d7b299f2d0 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Mon, 5 Apr 2021 19:17:21 +0530 Subject: [PATCH 4/4] Add CHANGELOG entry --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0fc28a168a..be1071e069 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-instrumentation-urllib3` Add urllib3 instrumentation ([#299](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/299)) +### Removed +- Remove `http.status_text` from span attributes + ([#406](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/406)) + ## [0.19b0](https://github.com/open-telemetry/opentelemetry-python-contrib/releases/tag/v0.19b0) - 2021-03-26 ### Changed