From 35cc6f2854c419f912ef3d2fa2660573e8e277de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Em=C3=ADdio=20Neto?= <9735060+emdneto@users.noreply.github.com> Date: Tue, 30 Jul 2024 13:43:13 -0300 Subject: [PATCH] add missing tests to urllib3 after semconv migration (#2748) --- .../tests/test_urllib3_integration.py | 101 +++++---- .../tests/test_urllib3_metrics.py | 195 ++++++++++++++---- 2 files changed, 205 insertions(+), 91 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py index 0e6037ed11..69bed0eaee 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py @@ -36,13 +36,6 @@ suppress_instrumentation, ) from opentelemetry.propagate import get_global_textmap, set_global_textmap -from opentelemetry.semconv.attributes.http_attributes import ( - HTTP_REQUEST_METHOD, - HTTP_REQUEST_METHOD_ORIGINAL, - HTTP_RESPONSE_STATUS_CODE, -) -from opentelemetry.semconv.attributes.url_attributes import URL_FULL -from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.test.mock_textmap import MockTextMapPropagator from opentelemetry.test.test_base import TestBase from opentelemetry.trace import Span @@ -123,24 +116,29 @@ def assert_success_span( self.assertEqual( span.status.status_code, trace.status.StatusCode.UNSET ) - attr_old = { - SpanAttributes.HTTP_METHOD: "GET", - SpanAttributes.HTTP_URL: url, - SpanAttributes.HTTP_STATUS_CODE: 200, + expected_attr_old = { + "http.method": "GET", + "http.url": url, + "http.status_code": 200, } - attr_new = { - HTTP_REQUEST_METHOD: "GET", - URL_FULL: url, - HTTP_RESPONSE_STATUS_CODE: 200, + expected_attr_new = { + "http.request.method": "GET", + "url.full": url, + "http.response.status_code": 200, } attributes = { - _HTTPStabilityMode.DEFAULT: attr_old, - _HTTPStabilityMode.HTTP: attr_new, - _HTTPStabilityMode.HTTP_DUP: {**attr_new, **attr_old}, + _HTTPStabilityMode.DEFAULT: expected_attr_old, + _HTTPStabilityMode.HTTP: expected_attr_new, + _HTTPStabilityMode.HTTP_DUP: { + **expected_attr_new, + **expected_attr_old, + }, } - self.assertEqual(span.attributes, attributes.get(sem_conv_opt_in_mode)) + self.assertDictEqual( + dict(span.attributes), attributes.get(sem_conv_opt_in_mode) + ) def assert_exception_span( self, @@ -149,24 +147,29 @@ def assert_exception_span( ): span = self.assert_span() - attr_old = { - SpanAttributes.HTTP_METHOD: "GET", - SpanAttributes.HTTP_URL: url, + expected_attr_old = { + "http.method": "GET", + "http.url": url, } - attr_new = { - HTTP_REQUEST_METHOD: "GET", - URL_FULL: url, + expected_attr_new = { + "http.request.method": "GET", + "url.full": url, # TODO: Add `error.type` attribute when supported } attributes = { - _HTTPStabilityMode.DEFAULT: attr_old, - _HTTPStabilityMode.HTTP: attr_new, - _HTTPStabilityMode.HTTP_DUP: {**attr_new, **attr_old}, + _HTTPStabilityMode.DEFAULT: expected_attr_old, + _HTTPStabilityMode.HTTP: expected_attr_new, + _HTTPStabilityMode.HTTP_DUP: { + **expected_attr_new, + **expected_attr_old, + }, } - self.assertEqual(span.attributes, attributes.get(sem_conv_opt_in_mode)) + self.assertDictEqual( + dict(span.attributes), attributes.get(sem_conv_opt_in_mode) + ) self.assertEqual( trace.status.StatusCode.ERROR, span.status.status_code ) @@ -265,9 +268,7 @@ def test_basic_not_found(self): self.assertEqual(404, response.status) span = self.assert_span() - self.assertEqual( - 404, span.attributes.get(SpanAttributes.HTTP_STATUS_CODE) - ) + self.assertEqual(404, span.attributes.get("http.status_code")) self.assertIs(trace.status.StatusCode.ERROR, span.status.status_code) def test_basic_not_found_new_semconv(self): @@ -278,7 +279,7 @@ def test_basic_not_found_new_semconv(self): self.assertEqual(404, response.status) span = self.assert_span() - self.assertEqual(404, span.attributes.get(HTTP_RESPONSE_STATUS_CODE)) + self.assertEqual(404, span.attributes.get("http.response.status_code")) self.assertIs(trace.status.StatusCode.ERROR, span.status.status_code) def test_basic_not_found_both_semconv(self): @@ -289,10 +290,8 @@ def test_basic_not_found_both_semconv(self): self.assertEqual(404, response.status) span = self.assert_span() - self.assertEqual(404, span.attributes.get(HTTP_RESPONSE_STATUS_CODE)) - self.assertEqual( - 404, span.attributes.get(SpanAttributes.HTTP_STATUS_CODE) - ) + self.assertEqual(404, span.attributes.get("http.response.status_code")) + self.assertEqual(404, span.attributes.get("http.status_code")) self.assertIs(trace.status.StatusCode.ERROR, span.status.status_code) @mock.patch("httpretty.http.HttpBaseClass.METHODS", ("NONSTANDARD",)) @@ -303,12 +302,8 @@ def test_nonstandard_http_method(self): self.perform_request(self.HTTP_URL, method="NONSTANDARD") span = self.assert_span() self.assertEqual("HTTP", span.name) - self.assertEqual( - span.attributes.get(SpanAttributes.HTTP_METHOD), "_OTHER" - ) - self.assertEqual( - span.attributes.get(SpanAttributes.HTTP_STATUS_CODE), 405 - ) + self.assertEqual(span.attributes.get("http.method"), "_OTHER") + self.assertEqual(span.attributes.get("http.status_code"), 405) @mock.patch("httpretty.http.HttpBaseClass.METHODS", ("NONSTANDARD",)) def test_nonstandard_http_method_new_semconv(self): @@ -318,11 +313,11 @@ def test_nonstandard_http_method_new_semconv(self): self.perform_request(self.HTTP_URL, method="NONSTANDARD") span = self.assert_span() self.assertEqual("HTTP", span.name) - self.assertEqual(span.attributes.get(HTTP_REQUEST_METHOD), "_OTHER") + self.assertEqual(span.attributes.get("http.request.method"), "_OTHER") self.assertEqual( - span.attributes.get(HTTP_REQUEST_METHOD_ORIGINAL), "NONSTANDARD" + span.attributes.get("http.request.method_original"), "NONSTANDARD" ) - self.assertEqual(span.attributes.get(HTTP_RESPONSE_STATUS_CODE), 405) + self.assertEqual(span.attributes.get("http.response.status_code"), 405) @mock.patch("httpretty.http.HttpBaseClass.METHODS", ("NONSTANDARD",)) def test_nonstandard_http_method_both_semconv(self): @@ -332,17 +327,13 @@ def test_nonstandard_http_method_both_semconv(self): self.perform_request(self.HTTP_URL, method="NONSTANDARD") span = self.assert_span() self.assertEqual("HTTP", span.name) + self.assertEqual(span.attributes.get("http.method"), "_OTHER") + self.assertEqual(span.attributes.get("http.status_code"), 405) + self.assertEqual(span.attributes.get("http.request.method"), "_OTHER") self.assertEqual( - span.attributes.get(SpanAttributes.HTTP_METHOD), "_OTHER" - ) - self.assertEqual( - span.attributes.get(SpanAttributes.HTTP_STATUS_CODE), 405 - ) - self.assertEqual(span.attributes.get(HTTP_REQUEST_METHOD), "_OTHER") - self.assertEqual( - span.attributes.get(HTTP_REQUEST_METHOD_ORIGINAL), "NONSTANDARD" + span.attributes.get("http.request.method_original"), "NONSTANDARD" ) - self.assertEqual(span.attributes.get(HTTP_RESPONSE_STATUS_CODE), 405) + self.assertEqual(span.attributes.get("http.response.status_code"), 405) def test_basic_http_non_default_port(self): url = "http://mock:666/status/200" diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_metrics.py b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_metrics.py index c6e9011351..959f793398 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_metrics.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_metrics.py @@ -26,18 +26,6 @@ _OpenTelemetrySemanticConventionStability, ) from opentelemetry.instrumentation.urllib3 import URLLib3Instrumentor -from opentelemetry.semconv.attributes.http_attributes import ( - HTTP_REQUEST_METHOD, - HTTP_RESPONSE_STATUS_CODE, -) -from opentelemetry.semconv.attributes.network_attributes import ( - NETWORK_PROTOCOL_VERSION, -) -from opentelemetry.semconv.attributes.server_attributes import ( - SERVER_ADDRESS, - SERVER_PORT, -) -from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.test.httptest import HttpTestBase from opentelemetry.test.test_base import TestBase @@ -85,6 +73,7 @@ def test_basic_metrics(self): response = self.pool.request("GET", self.HTTP_URL) duration_ms = max(round((default_timer() - start_time) * 1000), 0) metrics = self.get_sorted_metrics() + self.assertEqual(len(metrics), 3) ( client_duration, @@ -93,13 +82,13 @@ def test_basic_metrics(self): ) = metrics attrs_old = { - SpanAttributes.HTTP_STATUS_CODE: 200, - SpanAttributes.HTTP_HOST: "mock", - SpanAttributes.NET_PEER_PORT: 80, - SpanAttributes.NET_PEER_NAME: "mock", - SpanAttributes.HTTP_METHOD: "GET", - SpanAttributes.HTTP_FLAVOR: "1.1", - SpanAttributes.HTTP_SCHEME: "http", + "http.status_code": 200, + "http.host": "mock", + "net.peer.port": 80, + "net.peer.name": "mock", + "http.method": "GET", + "http.flavor": "1.1", + "http.scheme": "http", } self.assertEqual(client_duration.name, "http.client.duration") @@ -154,6 +143,7 @@ def test_basic_metrics_new_semconv(self): duration_s = max(default_timer() - start_time, 0) metrics = self.get_sorted_metrics() + self.assertEqual(len(metrics), 3) ( client_request_size, client_duration, @@ -161,11 +151,11 @@ def test_basic_metrics_new_semconv(self): ) = metrics attrs_new = { - NETWORK_PROTOCOL_VERSION: "1.1", - SERVER_ADDRESS: "mock", - SERVER_PORT: 80, - HTTP_REQUEST_METHOD: "GET", - HTTP_RESPONSE_STATUS_CODE: 200, + "network.protocol.version": "1.1", + "server.address": "mock", + "server.port": 80, + "http.request.method": "GET", + "http.response.status_code": 200, # TODO: add URL_SCHEME to tests when supported in the implementation } @@ -217,6 +207,139 @@ def test_basic_metrics_new_semconv(self): ], ) + def test_basic_metrics_both_semconv(self): + start_time = default_timer() + response = self.pool.request("GET", self.HTTP_URL) + duration_s = max(default_timer() - start_time, 0) + duration = max(round(duration_s * 1000), 0) + expected_size = len(response.data) + + metrics = self.get_sorted_metrics() + self.assertEqual(len(metrics), 6) + + ( + client_duration, + client_request_body_size, + client_request_duration, + client_request_size, + client_response_body_size, + client_response_size, + ) = metrics[:6] + + attrs_new = { + "network.protocol.version": "1.1", + "server.address": "mock", + "server.port": 80, + "http.request.method": "GET", + "http.response.status_code": 200, + # TODO: add URL_SCHEME to tests when supported in the implementation + } + + attrs_old = { + "http.status_code": 200, + "http.host": "mock", + "net.peer.port": 80, + "net.peer.name": "mock", + "http.method": "GET", + "http.flavor": "1.1", + "http.scheme": "http", + } + + # assert new semconv metrics + self.assertEqual( + client_request_duration.name, "http.client.request.duration" + ) + self.assert_metric_expected( + client_request_duration, + [ + self.create_histogram_data_point( + count=1, + sum_data_point=duration_s, + max_data_point=duration_s, + min_data_point=duration_s, + attributes=attrs_new, + ) + ], + est_value_delta=40 / 1000, + ) + + self.assertEqual( + client_request_body_size.name, "http.client.request.body.size" + ) + self.assert_metric_expected( + client_request_body_size, + [ + self.create_histogram_data_point( + count=1, + sum_data_point=0, + max_data_point=0, + min_data_point=0, + attributes=attrs_new, + ) + ], + ) + + self.assertEqual( + client_response_body_size.name, "http.client.response.body.size" + ) + self.assert_metric_expected( + client_response_body_size, + [ + self.create_histogram_data_point( + count=1, + sum_data_point=expected_size, + max_data_point=expected_size, + min_data_point=expected_size, + attributes=attrs_new, + ) + ], + ) + # assert old semconv metrics + self.assertEqual(client_duration.name, "http.client.duration") + self.assert_metric_expected( + client_duration, + [ + self.create_histogram_data_point( + count=1, + sum_data_point=duration, + max_data_point=duration, + min_data_point=duration, + attributes=attrs_old, + ) + ], + est_value_delta=40, + ) + + self.assertEqual(client_request_size.name, "http.client.request.size") + self.assert_metric_expected( + client_request_size, + [ + self.create_histogram_data_point( + count=1, + sum_data_point=0, + max_data_point=0, + min_data_point=0, + attributes=attrs_old, + ) + ], + ) + + self.assertEqual( + client_response_size.name, "http.client.response.size" + ) + self.assert_metric_expected( + client_response_size, + [ + self.create_histogram_data_point( + count=1, + sum_data_point=expected_size, + max_data_point=expected_size, + min_data_point=expected_size, + attributes=attrs_old, + ) + ], + ) + @mock.patch("httpretty.http.HttpBaseClass.METHODS", ("NONSTANDARD",)) def test_basic_metrics_nonstandard_http_method(self): httpretty.register_uri( @@ -236,13 +359,13 @@ def test_basic_metrics_nonstandard_http_method(self): ) = metrics attrs_old = { - SpanAttributes.HTTP_STATUS_CODE: 405, - SpanAttributes.HTTP_HOST: "mock", - SpanAttributes.NET_PEER_PORT: 80, - SpanAttributes.NET_PEER_NAME: "mock", - SpanAttributes.HTTP_METHOD: "_OTHER", - SpanAttributes.HTTP_FLAVOR: "1.1", - SpanAttributes.HTTP_SCHEME: "http", + "http.status_code": 405, + "http.host": "mock", + "net.peer.port": 80, + "net.peer.name": "mock", + "http.method": "_OTHER", + "http.flavor": "1.1", + "http.scheme": "http", } self.assertEqual(client_duration.name, "http.client.duration") @@ -309,11 +432,11 @@ def test_basic_metrics_nonstandard_http_method_new_semconv(self): ) = metrics attrs_new = { - NETWORK_PROTOCOL_VERSION: "1.1", - SERVER_ADDRESS: "mock", - SERVER_PORT: 80, - HTTP_REQUEST_METHOD: "_OTHER", - HTTP_RESPONSE_STATUS_CODE: 405, + "network.protocol.version": "1.1", + "server.address": "mock", + "server.port": 80, + "http.request.method": "_OTHER", + "http.response.status_code": 405, "error.type": "405", # TODO: add URL_SCHEME to tests when supported in the implementation }