diff --git a/CHANGELOG.md b/CHANGELOG.md index 242dd4935c..4c34019d18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +- Fix exception in Urllib3 when dealing with filelike body. + ([#1399](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1399)) + ### Added - Add connection attributes to sqlalchemy connect span 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 02f701068b..6e0da3be36 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py @@ -64,7 +64,9 @@ def response_hook(span, request, response): --- """ +import collections.abc import contextlib +import io import typing from timeit import default_timer from typing import Collection @@ -213,8 +215,9 @@ def instrumented_urlopen(wrapped, instance, args, kwargs): if callable(response_hook): response_hook(span, instance, response) - request_size = 0 if body is None else len(body) + request_size = _get_body_size(body) response_size = int(response.headers.get("Content-Length", 0)) + metric_attributes = _create_metric_attributes( instance, response, method ) @@ -222,9 +225,10 @@ def instrumented_urlopen(wrapped, instance, args, kwargs): duration_histogram.record( elapsed_time, attributes=metric_attributes ) - request_size_histogram.record( - request_size, attributes=metric_attributes - ) + if request_size is not None: + request_size_histogram.record( + request_size, attributes=metric_attributes + ) response_size_histogram.record( response_size, attributes=metric_attributes ) @@ -268,6 +272,16 @@ def _get_url( return url +def _get_body_size(body: object) -> typing.Optional[int]: + if body is None: + return 0 + if isinstance(body, collections.abc.Sized): + return len(body) + if isinstance(body, io.BytesIO): + return body.getbuffer().nbytes + return None + + def _should_append_port(scheme: str, port: typing.Optional[int]) -> bool: if not port: return False diff --git a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_metrics.py b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_metrics.py index 203bf537d6..ca691ebd47 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_metrics.py +++ b/instrumentation/opentelemetry-instrumentation-urllib3/tests/test_urllib3_metrics.py @@ -12,8 +12,10 @@ # See the License for the specific language governing permissions and # limitations under the License. +import io from timeit import default_timer +import httpretty import urllib3 import urllib3.exceptions from urllib3.request import encode_multipart_formdata @@ -23,181 +25,238 @@ from opentelemetry.test.test_base import TestBase -class TestUrllib3MetricsInstrumentation(HttpTestBase, TestBase): +class TestURLLib3InstrumentorMetric(HttpTestBase, TestBase): + HTTP_URL = "http://httpbin.org/status/200" + def setUp(self): super().setUp() - self.assert_ip = self.server.server_address[0] - self.assert_port = self.server.server_address[1] - self.http_host = ":".join(map(str, self.server.server_address[:2])) - self.http_url_base = "http://" + self.http_host - self.http_url = self.http_url_base + "/status/200" - URLLib3Instrumentor().instrument(meter_provider=self.meter_provider) + URLLib3Instrumentor().instrument() + httpretty.enable(allow_net_connect=False) + httpretty.register_uri(httpretty.GET, self.HTTP_URL, body="Hello!") + httpretty.register_uri(httpretty.POST, self.HTTP_URL, body="Hello!") + self.pool = urllib3.PoolManager() def tearDown(self): super().tearDown() + self.pool.clear() URLLib3Instrumentor().uninstrument() - # Return Sequence with one histogram - def create_histogram_data_points(self, sum_data_point, attributes): - return [ - self.create_histogram_data_point( - sum_data_point, 1, sum_data_point, sum_data_point, attributes - ) - ] - - def test_basic_metric_check_client_size_get(self): - with urllib3.PoolManager() as pool: - start_time = default_timer() - response = pool.request("GET", self.http_url) - client_duration_estimated = (default_timer() - start_time) * 1000 - - metrics = self.get_sorted_metrics() - - ( - client_duration, - client_request_size, - client_response_size, - ) = metrics - - self.assertEqual(client_duration.name, "http.client.duration") - self.assert_metric_expected( - client_duration, - self.create_histogram_data_points( - client_duration_estimated, + httpretty.disable() + httpretty.reset() + + def test_basic_metrics(self): + start_time = default_timer() + response = self.pool.request("GET", self.HTTP_URL) + client_duration_estimated = (default_timer() - start_time) * 1000 + + metrics = self.get_sorted_metrics() + + ( + client_duration, + client_request_size, + client_response_size, + ) = 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=client_duration_estimated, + max_data_point=client_duration_estimated, + min_data_point=client_duration_estimated, attributes={ - "http.status_code": 200, - "http.host": self.assert_ip, - "http.method": "GET", "http.flavor": "1.1", + "http.host": "httpbin.org", + "http.method": "GET", "http.scheme": "http", - "net.peer.name": self.assert_ip, - "net.peer.port": self.assert_port, + "http.status_code": 200, + "net.peer.name": "httpbin.org", + "net.peer.port": 80, }, - ), - est_value_delta=200, - ) - - self.assertEqual( - client_request_size.name, "http.client.request.size" - ) - self.assert_metric_expected( - client_request_size, - self.create_histogram_data_points( - 0, + ) + ], + est_value_delta=200, + ) + + 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={ - "http.status_code": 200, - "http.host": self.assert_ip, - "http.method": "GET", "http.flavor": "1.1", + "http.host": "httpbin.org", + "http.method": "GET", "http.scheme": "http", - "net.peer.name": self.assert_ip, - "net.peer.port": self.assert_port, + "http.status_code": 200, + "net.peer.name": "httpbin.org", + "net.peer.port": 80, }, - ), - ) - - self.assertEqual( - client_response_size.name, "http.client.response.size" - ) - self.assert_metric_expected( - client_response_size, - self.create_histogram_data_points( - len(response.data), + ) + ], + ) + + expected_size = len(response.data) + 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={ - "http.status_code": 200, - "http.host": self.assert_ip, - "http.method": "GET", "http.flavor": "1.1", + "http.host": "httpbin.org", + "http.method": "GET", "http.scheme": "http", - "net.peer.name": self.assert_ip, - "net.peer.port": self.assert_port, + "http.status_code": 200, + "net.peer.name": "httpbin.org", + "net.peer.port": 80, }, - ), - ) - - def test_basic_metric_check_client_size_post(self): - with urllib3.PoolManager() as pool: - start_time = default_timer() - data_fields = {"data": "test"} - response = pool.request("POST", self.http_url, fields=data_fields) - client_duration_estimated = (default_timer() - start_time) * 1000 - body = encode_multipart_formdata(data_fields)[0] - - metrics = self.get_sorted_metrics() - - ( - client_duration, - client_request_size, - client_response_size, - ) = metrics - - self.assertEqual(client_duration.name, "http.client.duration") - self.assert_metric_expected( - client_duration, - self.create_histogram_data_points( - client_duration_estimated, + ) + ], + ) + + def test_str_request_body_size_metrics(self): + self.pool.request("POST", self.HTTP_URL, body="foobar") + + metrics = self.get_sorted_metrics() + (_, client_request_size, _) = metrics + + 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=6, + max_data_point=6, + min_data_point=6, attributes={ - "http.status_code": 501, - "http.host": self.assert_ip, - "http.method": "POST", "http.flavor": "1.1", + "http.host": "httpbin.org", + "http.method": "POST", "http.scheme": "http", - "net.peer.name": self.assert_ip, - "net.peer.port": self.assert_port, + "http.status_code": 200, + "net.peer.name": "httpbin.org", + "net.peer.port": 80, }, - ), - est_value_delta=200, - ) - - self.assertEqual( - client_request_size.name, "http.client.request.size" - ) - client_request_size_expected = len(body) - self.assert_metric_expected( - client_request_size, - self.create_histogram_data_points( - client_request_size_expected, + ) + ], + ) + + def test_bytes_request_body_size_metrics(self): + self.pool.request("POST", self.HTTP_URL, body=b"foobar") + + metrics = self.get_sorted_metrics() + (_, client_request_size, _) = metrics + + 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=6, + max_data_point=6, + min_data_point=6, attributes={ - "http.status_code": 501, - "http.host": self.assert_ip, - "http.method": "POST", "http.flavor": "1.1", + "http.host": "httpbin.org", + "http.method": "POST", "http.scheme": "http", - "net.peer.name": self.assert_ip, - "net.peer.port": self.assert_port, + "http.status_code": 200, + "net.peer.name": "httpbin.org", + "net.peer.port": 80, }, - ), - ) - - self.assertEqual( - client_response_size.name, "http.client.response.size" - ) - client_response_size_expected = len(response.data) - self.assert_metric_expected( - client_response_size, - self.create_histogram_data_points( - client_response_size_expected, + ) + ], + ) + + def test_fields_request_body_size_metrics(self): + self.pool.request("POST", self.HTTP_URL, fields={"foo": "bar"}) + + metrics = self.get_sorted_metrics() + (_, client_request_size, _) = metrics + + self.assertEqual(client_request_size.name, "http.client.request.size") + expected_value = len(encode_multipart_formdata({"foo": "bar"})[0]) + self.assert_metric_expected( + client_request_size, + [ + self.create_histogram_data_point( + count=1, + sum_data_point=expected_value, + max_data_point=expected_value, + min_data_point=expected_value, attributes={ - "http.status_code": 501, - "http.host": self.assert_ip, + "http.flavor": "1.1", + "http.host": "httpbin.org", "http.method": "POST", + "http.scheme": "http", + "http.status_code": 200, + "net.peer.name": "httpbin.org", + "net.peer.port": 80, + }, + ) + ], + ) + + def test_bytesio_request_body_size_metrics(self): + self.pool.request("POST", self.HTTP_URL, body=io.BytesIO(b"foobar")) + + metrics = self.get_sorted_metrics() + (_, client_request_size, _) = metrics + + 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=6, + max_data_point=6, + min_data_point=6, + attributes={ "http.flavor": "1.1", + "http.host": "httpbin.org", + "http.method": "POST", "http.scheme": "http", - "net.peer.name": self.assert_ip, - "net.peer.port": self.assert_port, + "http.status_code": 200, + "net.peer.name": "httpbin.org", + "net.peer.port": 80, }, - ), - ) + ) + ], + ) + + def test_generator_request_body_size_metrics(self): + self.pool.request( + "POST", self.HTTP_URL, body=(b for b in (b"foo", b"bar")) + ) + + metrics = self.get_sorted_metrics() + self.assertEqual(len(metrics), 2) + self.assertNotIn("http.client.request.size", [m.name for m in metrics]) def test_metric_uninstrument(self): - with urllib3.PoolManager() as pool: - pool.request("GET", self.http_url) - URLLib3Instrumentor().uninstrument() - pool.request("GET", self.http_url) + self.pool.request("GET", self.HTTP_URL) + URLLib3Instrumentor().uninstrument() + self.pool.request("GET", self.HTTP_URL) - metrics = self.get_sorted_metrics() - self.assertEqual(len(metrics), 3) + metrics = self.get_sorted_metrics() + self.assertEqual(len(metrics), 3) - for metric in metrics: - for point in list(metric.data.data_points): - self.assertEqual(point.count, 1) + for metric in metrics: + for point in list(metric.data.data_points): + self.assertEqual(point.count, 1)