add missing tests to urllib3 after semconv migration (#2748)

2025-08-02 19:47:17 +08:00 · 2024-07-30 13:43:13 -03:00
parent dbfa6818bf
commit 35cc6f2854
2 changed files with 205 additions and 91 deletions
--- a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py
+++ b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_integration.py
@ -36,13 +36,6 @@ from opentelemetry.instrumentation.utils import (
    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 @@ class TestURLLib3Instrumentor(TestBase):
        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 @@ class TestURLLib3Instrumentor(TestBase):
    ):
        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 @@ class TestURLLib3Instrumentor(TestBase):
        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 @@ class TestURLLib3Instrumentor(TestBase):
        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 @@ class TestURLLib3Instrumentor(TestBase):
        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 @@ class TestURLLib3Instrumentor(TestBase):
        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 @@ class TestURLLib3Instrumentor(TestBase):
        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 @@ class TestURLLib3Instrumentor(TestBase):
        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"
+            span.attributes.get("http.request.method_original"), "NONSTANDARD"
        )
-        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"
-        )
-        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"
--- a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_metrics.py
+++ b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_metrics.py
@ -26,18 +26,6 @@ from opentelemetry.instrumentation._semconv import (
    _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 @@ class TestURLLib3InstrumentorMetric(HttpTestBase, TestBase):
        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 @@ class TestURLLib3InstrumentorMetric(HttpTestBase, TestBase):
        ) = 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 @@ class TestURLLib3InstrumentorMetric(HttpTestBase, TestBase):
        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 @@ class TestURLLib3InstrumentorMetric(HttpTestBase, TestBase):
        ) = 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 @@ class TestURLLib3InstrumentorMetric(HttpTestBase, TestBase):
            ],
        )

+    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 @@ class TestURLLib3InstrumentorMetric(HttpTestBase, TestBase):
        ) = 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 @@ class TestURLLib3InstrumentorMetric(HttpTestBase, TestBase):
        ) = 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
        }