From a13800504d39147d0f13f81c8c439e5e64e3e801 Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Thu, 26 Oct 2023 16:51:19 -0700 Subject: [PATCH 1/4] HTTP transition for flask and wsgi --- .../instrumentation/flask/__init__.py | 131 +++++-- .../tests/test_copy_context.py | 2 +- .../tests/test_programmatic.py | 335 ++++++++++++++-- .../instrumentation/wsgi/__init__.py | 357 +++++++++++++----- .../tests/test_wsgi_middleware.py | 232 +++++++++--- opentelemetry-instrumentation/pyproject.toml | 1 + .../opentelemetry/instrumentation/utils.py | 73 ++++ .../src/opentelemetry/util/http/__init__.py | 39 +- .../src/opentelemetry/util/http/httplib.py | 4 +- .../tests/test_http_base.py | 8 +- 10 files changed, 956 insertions(+), 226 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py index 432c6b1fbf..570d3c0789 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -255,7 +255,15 @@ def response_hook(span: Span, status: str, response_headers: List): from opentelemetry.instrumentation.propagators import ( get_global_response_propagator, ) -from opentelemetry.instrumentation.utils import _start_internal_or_server_span +from opentelemetry.instrumentation.utils import ( + _start_internal_or_server_span, + _OpenTelemetrySemanticConventionStability, + _OpenTelemetryStabilityMode, + _get_schema_url, + _report_new, + _report_old +) + from opentelemetry.metrics import get_meter from opentelemetry.semconv.metrics import MetricInstruments from opentelemetry.semconv.trace import SpanAttributes @@ -283,17 +291,20 @@ def _request_ctx_ref() -> weakref.ReferenceType: def get_default_span_name(): + # returns sanitized method since WSGI does not support routing + span_name = otel_wsgi.get_default_span_name(flask.request.environ) try: - span_name = flask.request.url_rule.rule + return span_name + " " + flask.request.url_rule.rule except AttributeError: - span_name = otel_wsgi.get_default_span_name(flask.request.environ) - return span_name + return span_name def _rewrapped_app( wsgi_app, active_requests_counter, - duration_histogram, + duration_histogram_old, + duration_histogram_new, + sem_conv_opt_in_mode, response_hook=None, excluded_urls=None, ): @@ -304,11 +315,10 @@ def _wrapped_app(wrapped_app_environ, start_response): # we better avoid it. wrapped_app_environ[_ENVIRON_STARTTIME_KEY] = time_ns() start = default_timer() - attributes = otel_wsgi.collect_request_attributes(wrapped_app_environ) + attributes = otel_wsgi.collect_request_attributes(wrapped_app_environ, sem_conv_opt_in_mode) active_requests_count_attrs = ( - otel_wsgi._parse_active_request_count_attrs(attributes) + otel_wsgi._filter_active_request_count_attrs(attributes, sem_conv_opt_in_mode) ) - duration_attrs = otel_wsgi._parse_duration_attrs(attributes) active_requests_counter.add(1, active_requests_count_attrs) def _start_response(status, response_headers, *args, **kwargs): @@ -327,13 +337,8 @@ def _start_response(status, response_headers, *args, **kwargs): if span: otel_wsgi.add_response_attributes( - span, status, response_headers + span, status, response_headers, attributes, sem_conv_opt_in_mode ) - status_code = otel_wsgi._parse_status_code(status) - if status_code is not None: - duration_attrs[ - SpanAttributes.HTTP_STATUS_CODE - ] = status_code if ( span.is_recording() and span.kind == trace.SpanKind.SERVER @@ -354,8 +359,16 @@ def _start_response(status, response_headers, *args, **kwargs): return start_response(status, response_headers, *args, **kwargs) result = wsgi_app(wrapped_app_environ, _start_response) - duration = max(round((default_timer() - start) * 1000), 0) - duration_histogram.record(duration, duration_attrs) + + # duration = max(round((default_timer() - start) * 1000), 0) + duration = default_timer() - start + if duration_histogram_old is not None: + duration_attrs_old = otel_wsgi._filter_duration_attrs(attributes, _OpenTelemetryStabilityMode.DEFAULT) + duration_histogram_old.record(max(round(duration * 1000), 0), duration_attrs_old) + if duration_histogram_new is not None: + duration_attrs_new = otel_wsgi._filter_duration_attrs(attributes, _OpenTelemetryStabilityMode.HTTP) + duration_histogram_new.record(duration, duration_attrs_new) + active_requests_counter.add(-1, active_requests_count_attrs) return result @@ -368,6 +381,7 @@ def _wrapped_before_request( excluded_urls=None, enable_commenter=True, commenter_options=None, + sem_conv_opt_in_mode=None, ): def _before_request(): if excluded_urls and excluded_urls.url_disabled(flask.request.url): @@ -376,7 +390,7 @@ def _before_request(): span_name = get_default_span_name() attributes = otel_wsgi.collect_request_attributes( - flask_request_environ + flask_request_environ, sem_conv_opt_in_mode ) if flask.request.url_rule: # For 404 that result from no route found, etc, we @@ -487,6 +501,7 @@ class _InstrumentedFlask(flask.Flask): _enable_commenter = True _commenter_options = None _meter_provider = None + _sem_conv_opt_in_mode = None def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -495,29 +510,48 @@ def __init__(self, *args, **kwargs): self._is_instrumented_by_opentelemetry = True meter = get_meter( - __name__, __version__, _InstrumentedFlask._meter_provider - ) - duration_histogram = meter.create_histogram( - name=MetricInstruments.HTTP_SERVER_DURATION, - unit="ms", - description="measures the duration of the inbound HTTP request", + __name__, + __version__, + _InstrumentedFlask._meter_provider, + schema_url=_get_schema_url(self._sem_conv_opt_in_mode) ) + + duration_histogram_old = None + if _report_old(self._sem_conv_opt_in_mode): + duration_histogram_old = meter.create_histogram( + name=MetricInstruments.HTTP_SERVER_DURATION, + unit="ms", + description="measures the duration of the inbound HTTP request", + ) + duration_histogram_new = None + if _report_new(self._sem_conv_opt_in_mode): + duration_histogram_new = meter.create_histogram( + name=otel_wsgi._METRIC_INSTRUMENTS_HTTP_SERVER_REQUEST_DURATION, + unit="s", + description="measures the duration of the inbound HTTP request", + ) + active_requests_counter = meter.create_up_down_counter( name=MetricInstruments.HTTP_SERVER_ACTIVE_REQUESTS, - unit="requests", + unit="{request}", description="measures the number of concurrent HTTP requests that are currently in-flight", ) self.wsgi_app = _rewrapped_app( self.wsgi_app, active_requests_counter, - duration_histogram, + duration_histogram_old, + duration_histogram_new, + self._sem_conv_opt_in_mode, _InstrumentedFlask._response_hook, excluded_urls=_InstrumentedFlask._excluded_urls, ) tracer = trace.get_tracer( - __name__, __version__, _InstrumentedFlask._tracer_provider + __name__, + __version__, + _InstrumentedFlask._tracer_provider, + schema_url=_get_schema_url(self._sem_conv_opt_in_mode), ) _before_request = _wrapped_before_request( @@ -526,6 +560,7 @@ def __init__(self, *args, **kwargs): excluded_urls=_InstrumentedFlask._excluded_urls, enable_commenter=_InstrumentedFlask._enable_commenter, commenter_options=_InstrumentedFlask._commenter_options, + sem_conv_opt_in_mode=self._sem_conv_opt_in_mode ) self._before_request = _before_request self.before_request(_before_request) @@ -569,6 +604,13 @@ def _instrument(self, **kwargs): _InstrumentedFlask._commenter_options = commenter_options meter_provider = kwargs.get("meter_provider") _InstrumentedFlask._meter_provider = meter_provider + + # initialize semantic conventions opt-in if needed + _OpenTelemetrySemanticConventionStability._initialize() + + sem_conv_opt_in_mode = kwargs.get("sem_conv_opt_in_mode", _OpenTelemetryStabilityMode.DEFAULT) + _InstrumentedFlask._sem_conv_opt_in_mode = sem_conv_opt_in_mode + flask.Flask = _InstrumentedFlask def _uninstrument(self, **kwargs): @@ -577,6 +619,7 @@ def _uninstrument(self, **kwargs): @staticmethod def instrument_app( app, + sem_conv_opt_in_mode =_OpenTelemetryStabilityMode.DEFAULT, request_hook=None, response_hook=None, tracer_provider=None, @@ -594,12 +637,26 @@ def instrument_app( if excluded_urls is not None else _excluded_urls_from_env ) - meter = get_meter(__name__, __version__, meter_provider) - duration_histogram = meter.create_histogram( - name=MetricInstruments.HTTP_SERVER_DURATION, - unit="ms", - description="measures the duration of the inbound HTTP request", + meter = get_meter( + __name__, + __version__, + meter_provider, + schema_url=_get_schema_url(sem_conv_opt_in_mode), ) + duration_histogram_old = None + if _report_old(sem_conv_opt_in_mode): + duration_histogram_old = meter.create_histogram( + name=MetricInstruments.HTTP_SERVER_DURATION, + unit="ms", + description="measures the duration of the inbound HTTP request", + ) + duration_histogram_new = None + if _report_new(sem_conv_opt_in_mode): + duration_histogram_new = meter.create_histogram( + name=otel_wsgi._METRIC_INSTRUMENTS_HTTP_SERVER_REQUEST_DURATION, + unit="s", + description="measures the duration of the inbound HTTP request", + ) active_requests_counter = meter.create_up_down_counter( name=MetricInstruments.HTTP_SERVER_ACTIVE_REQUESTS, unit="requests", @@ -610,12 +667,19 @@ def instrument_app( app.wsgi_app = _rewrapped_app( app.wsgi_app, active_requests_counter, - duration_histogram, + duration_histogram_old, + duration_histogram_new, + sem_conv_opt_in_mode, response_hook, excluded_urls=excluded_urls, ) - tracer = trace.get_tracer(__name__, __version__, tracer_provider) + tracer = trace.get_tracer( + __name__, + __version__, + tracer_provider, + schema_url=_get_schema_url(sem_conv_opt_in_mode), + ) _before_request = _wrapped_before_request( request_hook, @@ -625,6 +689,7 @@ def instrument_app( commenter_options=commenter_options if commenter_options else {}, + sem_conv_opt_in_mode=sem_conv_opt_in_mode ) app._before_request = _before_request app.before_request(_before_request) diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_copy_context.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_copy_context.py index 96268de5e7..7a57d01c43 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_copy_context.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_copy_context.py @@ -44,5 +44,5 @@ def test_copycontext(self): resp = client.get("/copy_context", headers={"x-req": "a-header"}) self.assertEqual(200, resp.status_code) - self.assertEqual("/copy_context", resp.json["span_name"]) + self.assertEqual("GET /copy_context", resp.json["span_name"]) self.assertEqual("a-header", resp.json["request_header"]) diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index bf641aaed4..cd3309b28f 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -26,8 +26,10 @@ ) from opentelemetry.instrumentation.wsgi import ( OpenTelemetryMiddleware, - _active_requests_count_attrs, - _duration_attrs, + _active_requests_count_attrs_old, + _duration_attrs_old, + _active_requests_count_attrs_new, + _duration_attrs_new, ) from opentelemetry.sdk.metrics.export import ( HistogramDataPoint, @@ -44,18 +46,18 @@ OTEL_PYTHON_INSTRUMENTATION_HTTP_CAPTURE_ALL_METHODS, get_excluded_urls, ) +from opentelemetry.instrumentation.utils import _OpenTelemetryStabilityMode # pylint: disable=import-error from .base_test import InstrumentationTest -def expected_attributes(override_attributes): +def expected_attributes_old(override_attributes): default_attributes = { SpanAttributes.HTTP_METHOD: "GET", - SpanAttributes.HTTP_SERVER_NAME: "localhost", + SpanAttributes.NET_HOST_NAME: "localhost", SpanAttributes.HTTP_SCHEME: "http", SpanAttributes.NET_HOST_PORT: 80, - SpanAttributes.HTTP_HOST: "localhost", SpanAttributes.HTTP_TARGET: "/", SpanAttributes.HTTP_FLAVOR: "1.1", SpanAttributes.HTTP_STATUS_CODE: 200, @@ -64,14 +66,47 @@ def expected_attributes(override_attributes): default_attributes[key] = val return default_attributes +def expected_attributes_new(override_attributes): + default_attributes = { + SpanAttributes.HTTP_REQUEST_METHOD: "GET", + SpanAttributes.SERVER_ADDRESS: "localhost", + SpanAttributes.URL_SCHEME: "http", + SpanAttributes.SERVER_PORT: 80, + SpanAttributes.URL_PATH: "/", + SpanAttributes.NETWORK_PROTOCOL_VERSION: "1.1", + SpanAttributes.HTTP_RESPONSE_STATUS_CODE: 200, + } + for key, val in override_attributes.items(): + default_attributes[key] = val + return default_attributes + +_expected_metric_names_old = [ + "http.server.active_requests", + "http.server.duration", +] +_expected_metrics_attrs_old = { + "http.server.active_requests": _active_requests_count_attrs_old, + "http.server.duration": _duration_attrs_old, +} + +_expected_metric_names_new = [ + "http.server.active_requests", + "http.server.request.duration", +] +_expected_metrics_attrs_new = { + "http.server.active_requests": _active_requests_count_attrs_new, + "http.server.request.duration": _duration_attrs_new, +} -_expected_metric_names = [ +_expected_metric_names_dup = [ "http.server.active_requests", + "http.server.request.duration", "http.server.duration", ] -_recommended_attrs = { - "http.server.active_requests": _active_requests_count_attrs, - "http.server.duration": _duration_attrs, +_expected_metrics_attrs_dup = { + "http.server.active_requests": _active_requests_count_attrs_new + _active_requests_count_attrs_old, + "http.server.request.duration": _duration_attrs_new, + "http.server.duration": _duration_attrs_old, } @@ -94,7 +129,7 @@ def setUp(self): self.exclude_patch.start() self.app = Flask(__name__) - FlaskInstrumentor().instrument_app(self.app) + FlaskInstrumentor().instrument_app(self.app, _OpenTelemetryStabilityMode.DEFAULT) self._common_initialization() @@ -160,7 +195,7 @@ def assert_environ(): self.assertEqual(nonstring_keys, set()) def test_simple(self): - expected_attrs = expected_attributes( + expected_attrs = expected_attributes_old( { SpanAttributes.HTTP_TARGET: "/hello/123", SpanAttributes.HTTP_ROUTE: "/hello/", @@ -170,7 +205,7 @@ def test_simple(self): span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) - self.assertEqual(span_list[0].name, "/hello/") + self.assertEqual(span_list[0].name, "GET /hello/") self.assertEqual(span_list[0].kind, trace.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) @@ -203,7 +238,7 @@ def test_not_recording(self): self.assertFalse(mock_span.set_status.called) def test_404(self): - expected_attrs = expected_attributes( + expected_attrs = expected_attributes_old( { SpanAttributes.HTTP_METHOD: "POST", SpanAttributes.HTTP_TARGET: "/bye", @@ -216,12 +251,12 @@ def test_404(self): resp.close() span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) - self.assertEqual(span_list[0].name, "POST /bye") + self.assertEqual(span_list[0].name, "POST") self.assertEqual(span_list[0].kind, trace.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) def test_internal_error(self): - expected_attrs = expected_attributes( + expected_attrs = expected_attributes_old( { SpanAttributes.HTTP_TARGET: "/hello/500", SpanAttributes.HTTP_ROUTE: "/hello/", @@ -233,7 +268,7 @@ def test_internal_error(self): resp.close() span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) - self.assertEqual(span_list[0].name, "/hello/") + self.assertEqual(span_list[0].name, "GET /hello/") self.assertEqual(span_list[0].kind, trace.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) @@ -291,7 +326,7 @@ def test_flask_metrics(self): for scope_metric in resource_metric.scope_metrics: self.assertTrue(len(scope_metric.metrics) != 0) for metric in scope_metric.metrics: - self.assertIn(metric.name, _expected_metric_names) + self.assertIn(metric.name, _expected_metric_names_old) data_points = list(metric.data.data_points) self.assertEqual(len(data_points), 1) for point in data_points: @@ -305,7 +340,7 @@ def test_flask_metrics(self): number_data_point_seen = True for attr in point.attributes: self.assertIn( - attr, _recommended_attrs[metric.name] + attr, _expected_metrics_attrs_old[metric.name] ) self.assertTrue(number_data_point_seen and histogram_data_point_seen) @@ -351,19 +386,18 @@ def test_basic_metric_success(self): self.client.get("/hello/756") expected_duration_attributes = { "http.method": "GET", - "http.host": "localhost", + "net.host.name": "localhost", "http.scheme": "http", "http.flavor": "1.1", - "http.server_name": "localhost", "net.host.port": 80, "http.status_code": 200, } expected_requests_count_attributes = { "http.method": "GET", - "http.host": "localhost", + "net.host.name": "localhost", + "net.host.port": 80, "http.scheme": "http", "http.flavor": "1.1", - "http.server_name": "localhost", } self._assert_basic_metric( expected_duration_attributes, @@ -373,26 +407,26 @@ def test_basic_metric_success(self): def test_basic_metric_nonstandard_http_method_success(self): self.client.open("/hello/756", method="NONSTANDARD") expected_duration_attributes = { - "http.method": "UNKNOWN", - "http.host": "localhost", + "http.method": "_OTHER", + "net.host.name": "localhost", "http.scheme": "http", "http.flavor": "1.1", - "http.server_name": "localhost", "net.host.port": 80, "http.status_code": 405, } expected_requests_count_attributes = { - "http.method": "UNKNOWN", - "http.host": "localhost", + "http.method": "_OTHER", + "net.host.name": "localhost", + "net.host.port": 80, "http.scheme": "http", - "http.flavor": "1.1", - "http.server_name": "localhost", + "http.flavor": "1.1" } self._assert_basic_metric( expected_duration_attributes, expected_requests_count_attributes, ) + @patch.dict( "os.environ", { @@ -403,19 +437,18 @@ def test_basic_metric_nonstandard_http_method_allowed_success(self): self.client.open("/hello/756", method="NONSTANDARD") expected_duration_attributes = { "http.method": "NONSTANDARD", - "http.host": "localhost", + "net.host.name": "localhost", "http.scheme": "http", "http.flavor": "1.1", - "http.server_name": "localhost", "net.host.port": 80, "http.status_code": 405, } expected_requests_count_attributes = { "http.method": "NONSTANDARD", - "http.host": "localhost", + "net.host.name": "localhost", + "net.host.port": 80, "http.scheme": "http", "http.flavor": "1.1", - "http.server_name": "localhost", } self._assert_basic_metric( expected_duration_attributes, @@ -467,7 +500,7 @@ def tearDown(self): FlaskInstrumentor().uninstrument_app(self.app) def test_hooks(self): - expected_attrs = expected_attributes( + expected_attrs = expected_attributes_old( { "http.target": "/hello/123", "http.route": "/hello/", @@ -479,6 +512,7 @@ def test_hooks(self): span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) self.assertEqual(span_list[0].name, "name from hook") + self.assertEqual(span_list[0].attributes, expected_attrs) self.assertEqual(resp.headers["hook_attr"], "hello otel") @@ -515,7 +549,7 @@ def tearDown(self): FlaskInstrumentor().uninstrument() def test_no_app_hooks(self): - expected_attrs = expected_attributes( + expected_attrs = expected_attributes_old( { "http.target": "/hello/123", "http.route": "/hello/", @@ -748,3 +782,234 @@ def test_custom_response_header_not_added_in_internal_span(self): self.assertEqual(span.kind, trace.SpanKind.INTERNAL) for key, _ in not_expected.items(): self.assertNotIn(key, span.attributes) + + +class TestOpInHttp(InstrumentationTest, WsgiTestBase): + def setUp(self): + super().setUp() + + self.env_patch = patch.dict( + "os.environ", + { + "OTEL_PYTHON_FLASK_EXCLUDED_URLS": "http://localhost/env_excluded_arg/123,env_excluded_noarg" + }, + ) + self.env_patch.start() + + self.exclude_patch = patch( + "opentelemetry.instrumentation.flask._excluded_urls_from_env", + get_excluded_urls("FLASK"), + ) + self.exclude_patch.start() + + self.app = Flask(__name__) + FlaskInstrumentor().instrument_app(self.app, _OpenTelemetryStabilityMode.HTTP) + + self._common_initialization() + + def tearDown(self): + super().tearDown() + self.env_patch.stop() + self.exclude_patch.stop() + with self.disable_logging(): + FlaskInstrumentor().uninstrument_app(self.app) + + def test_simple(self): + expected_attrs = expected_attributes_new( + { + SpanAttributes.URL_PATH: "/hello/123", + SpanAttributes.URL_QUERY: "foo=bar", + SpanAttributes.HTTP_ROUTE: "/hello/", + } + ) + self.client.get("/hello/123?foo=bar") + + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) + self.assertEqual(span_list[0].name, "GET /hello/") + self.assertEqual(span_list[0].kind, trace.SpanKind.SERVER) + self.assertEqual(span_list[0].attributes, expected_attrs) + + def test_404(self): + expected_attrs = expected_attributes_new( + { + SpanAttributes.HTTP_REQUEST_METHOD: "POST", + SpanAttributes.URL_PATH: "/bye", + SpanAttributes.HTTP_RESPONSE_STATUS_CODE: 404 + } + ) + + resp = self.client.post("/bye") + self.assertEqual(404, resp.status_code) + resp.close() + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) + self.assertEqual(span_list[0].name, "POST") + self.assertEqual(span_list[0].kind, trace.SpanKind.SERVER) + self.assertEqual(span_list[0].attributes, expected_attrs) + + def test_internal_error(self): + expected_attrs = expected_attributes_new( + { + SpanAttributes.URL_PATH: "/hello/500", + SpanAttributes.HTTP_ROUTE: "/hello/", + SpanAttributes.HTTP_RESPONSE_STATUS_CODE: 500, + "error.type" : "500" + } + ) + resp = self.client.get("/hello/500") + self.assertEqual(500, resp.status_code) + resp.close() + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) + self.assertEqual(span_list[0].name, "GET /hello/") + self.assertEqual(span_list[0].kind, trace.SpanKind.SERVER) + self.assertEqual(span_list[0].attributes, expected_attrs) + + def test_flask_metrics(self): + start = default_timer() + self.client.get("/hello/123") + self.client.get("/hello/321") + self.client.get("/hello/756") + duration = default_timer() - start + metrics_list = self.memory_metrics_reader.get_metrics_data() + number_data_point_seen = False + histogram_data_point_seen = False + self.assertTrue(len(metrics_list.resource_metrics) != 0) + for resource_metric in metrics_list.resource_metrics: + self.assertTrue(len(resource_metric.scope_metrics) != 0) + for scope_metric in resource_metric.scope_metrics: + self.assertTrue(len(scope_metric.metrics) != 0) + for metric in scope_metric.metrics: + self.assertIn(metric.name, _expected_metric_names_new) + data_points = list(metric.data.data_points) + self.assertEqual(len(data_points), 1) + for point in data_points: + if isinstance(point, HistogramDataPoint): + self.assertEqual(point.count, 3) + self.assertAlmostEqual( + duration, point.sum, delta=10 + ) + histogram_data_point_seen = True + if isinstance(point, NumberDataPoint): + number_data_point_seen = True + for attr in point.attributes: + self.assertIn( + attr, _expected_metrics_attrs_new[metric.name] + ) + self.assertTrue(number_data_point_seen and histogram_data_point_seen) + + def test_flask_metric_values(self): + start = default_timer() + self.client.post("/hello/756") + self.client.post("/hello/756") + self.client.post("/hello/756") + duration = default_timer() - start + metrics_list = self.memory_metrics_reader.get_metrics_data() + for resource_metric in metrics_list.resource_metrics: + for scope_metric in resource_metric.scope_metrics: + for metric in scope_metric.metrics: + for point in list(metric.data.data_points): + if isinstance(point, HistogramDataPoint): + self.assertEqual(point.count, 3) + self.assertAlmostEqual( + duration, point.sum, delta=10 + ) + if isinstance(point, NumberDataPoint): + self.assertEqual(point.value, 0) + + def _assert_basic_metric(self, expected_duration_attributes, expected_requests_count_attributes): + metrics_list = self.memory_metrics_reader.get_metrics_data() + for resource_metric in metrics_list.resource_metrics: + for scope_metrics in resource_metric.scope_metrics: + for metric in scope_metrics.metrics: + for point in list(metric.data.data_points): + if isinstance(point, HistogramDataPoint): + self.assertDictEqual( + expected_duration_attributes, + dict(point.attributes), + ) + self.assertEqual(point.count, 1) + elif isinstance(point, NumberDataPoint): + self.assertDictEqual( + expected_requests_count_attributes, + dict(point.attributes), + ) + self.assertEqual(point.value, 0) + + def test_basic_metric_success(self): + self.client.get("/hello/756") + + expected_duration_attributes = { + "http.request.method": "GET", + "url.scheme": "http", + "network.protocol.version": "1.1", + "http.response.status_code": 200, + } + expected_requests_count_attributes = { + "http.request.method": "GET", + "url.scheme": "http", + } + + self._assert_basic_metric( + expected_duration_attributes, + expected_requests_count_attributes, + ) + + def test_basic_metric_nonstandard_http_method_success(self): + self.client.open("/hello/756", method="NONSTANDARD") + expected_duration_attributes = { + "http.request.method": "_OTHER", + "url.scheme": "http", + "network.protocol.version": "1.1", + "http.response.status_code": 405, + } + expected_requests_count_attributes = { + "http.request.method": "_OTHER", + "url.scheme": "http" + } + self._assert_basic_metric( + expected_duration_attributes, + expected_requests_count_attributes, + ) + + @patch.dict( + "os.environ", + { + OTEL_PYTHON_INSTRUMENTATION_HTTP_CAPTURE_ALL_METHODS: "1", + }, + ) + def test_basic_metric_nonstandard_http_method_allowed_success(self): + self.client.open("/hello/756", method="NONSTANDARD") + expected_duration_attributes = { + "http.request.method": "NONSTANDARD", + "url.scheme": "http", + "network.protocol.version": "1.1", + "http.response.status_code": 405, + } + expected_requests_count_attributes = { + "http.request.method": "NONSTANDARD", + "url.scheme": "http", + } + self._assert_basic_metric( + expected_duration_attributes, + expected_requests_count_attributes, + ) + + def test_internal_error_metrics(self): + self.client.get("/hello/500") + expected_duration_attributes = { + "http.request.method": "GET", + "url.scheme": "http", + "network.protocol.version": "1.1", + "http.response.status_code": 500, + "error.type": "500", + } + expected_requests_count_attributes = { + "http.request.method": "GET", + "url.scheme": "http" + } + self._assert_basic_metric( + expected_duration_attributes, + expected_requests_count_attributes, + ) \ No newline at end of file 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 35e217264d..31217f8a15 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -209,13 +209,17 @@ def response_hook(span: Span, environ: WSGIEnvironment, status: str, response_he import functools import typing -import wsgiref.util as wsgiref_util from timeit import default_timer +from urllib.parse import urlparse from opentelemetry import context, trace from opentelemetry.instrumentation.utils import ( _start_internal_or_server_span, http_status_to_status_code, + _OpenTelemetryStabilityMode, + _get_schema_url, + _report_new, + _report_old ) from opentelemetry.instrumentation.wsgi.version import __version__ from opentelemetry.metrics import get_meter @@ -233,14 +237,17 @@ def response_hook(span: Span, environ: WSGIEnvironment, status: str, response_he normalise_response_header_name, remove_url_credentials, sanitize_method, + parse_http_host, ) _HTTP_VERSION_PREFIX = "HTTP/" _CARRIER_KEY_PREFIX = "HTTP_" _CARRIER_KEY_PREFIX_LEN = len(_CARRIER_KEY_PREFIX) +# TODO: will come through semconv package once updated +_SPAN_ATTRIBUTES_ERROR_TYPE = "error.type" +_METRIC_INSTRUMENTS_HTTP_SERVER_REQUEST_DURATION = "http.server.request.duration" -# List of recommended attributes -_duration_attrs = [ +_duration_attrs_old = [ SpanAttributes.HTTP_METHOD, SpanAttributes.HTTP_HOST, SpanAttributes.HTTP_SCHEME, @@ -251,14 +258,28 @@ def response_hook(span: Span, environ: WSGIEnvironment, status: str, response_he SpanAttributes.NET_HOST_PORT, ] -_active_requests_count_attrs = [ +_duration_attrs_new = [ + _SPAN_ATTRIBUTES_ERROR_TYPE, + SpanAttributes.HTTP_REQUEST_METHOD, + SpanAttributes.HTTP_RESPONSE_STATUS_CODE, + SpanAttributes.HTTP_ROUTE, + SpanAttributes.NETWORK_PROTOCOL_VERSION, + SpanAttributes.URL_SCHEME, +] + +_active_requests_count_attrs_old = [ SpanAttributes.HTTP_METHOD, SpanAttributes.HTTP_HOST, SpanAttributes.HTTP_SCHEME, SpanAttributes.HTTP_FLAVOR, - SpanAttributes.HTTP_SERVER_NAME, + SpanAttributes.NET_HOST_NAME, + SpanAttributes.NET_HOST_PORT, ] +_active_requests_count_attrs_new = [ + SpanAttributes.HTTP_REQUEST_METHOD, + SpanAttributes.URL_SCHEME, +] class WSGIGetter(Getter[dict]): def get( @@ -291,58 +312,147 @@ def keys(self, carrier): wsgi_getter = WSGIGetter() -def setifnotnone(dic, key, value): - if value is not None: +def set_string_attribute(dic, key, value): + if value is not None and not value == "": dic[key] = value - - -def collect_request_attributes(environ): + return True + return False + + +def set_int_attribute(dic, key, value): + if value is not None and not value == "": + dic[key] = int(value) + return True + return False + + +def _parse_target(target, result, sem_conv_opt_in_mode): + if not target: + return False + + parts = urlparse(target) + + _set_scheme(result, parts.scheme, sem_conv_opt_in_mode) + if parts.path and not parts.path == "": + target = parts.path + if parts.query and not parts.query == "": + target += "?" + parts.query + _set_target(result, target, parts.path, parts.query, sem_conv_opt_in_mode) + return True + + return False + +def _set_scheme(result, scheme, sem_conv_opt_in_mode): + if _report_old(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.HTTP_SCHEME, scheme) + if _report_new(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.URL_SCHEME, scheme) + +def _set_target(result, target, path, query, sem_conv_opt_in_mode): + if _report_old(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.HTTP_TARGET, target) + if _report_new(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.URL_PATH, path) + set_string_attribute(result, SpanAttributes.URL_QUERY, query) + +def _parse_scheme_path_and_query(environ, result, sem_conv_opt_in_mode): + path = environ.get("PATH_INFO") + if path is None or path == "": + if _parse_target(environ.get("REQUEST_URI"), result, sem_conv_opt_in_mode) or _parse_target( + environ.get("RAW_URI"), result, sem_conv_opt_in_mode + ): + return + + path = path or "/" + target = path + query = environ.get("QUERY_STRING") + if query and not query == "": + target += "?" + query + _set_target(result, target, path, query, sem_conv_opt_in_mode) + +def _set_http_method(result, method, sem_conv_opt_in_mode): + original = method.strip() + normalized = sanitize_method(original) + if normalized != original and sem_conv_opt_in_mode != _OpenTelemetryStabilityMode.DEFAULT: + set_string_attribute(result, SpanAttributes.HTTP_REQUEST_METHOD_ORIGINAL, original) + + if _report_old(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.HTTP_METHOD, normalized) + if _report_new(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.HTTP_REQUEST_METHOD, normalized) + +def _set_host_port(result, host, port, sem_conv_opt_in_mode): + # TODO: don't set port if default for scheme + if _report_old(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.NET_HOST_NAME, host) + set_int_attribute(result, SpanAttributes.NET_HOST_PORT, port) + if _report_new(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.SERVER_ADDRESS, host) + set_int_attribute(result, SpanAttributes.SERVER_PORT, port) + +def _set_peer_ip_port(result, environ, sem_conv_opt_in_mode): + # TODO: support forwarded#for, etc + x_forwarded_for = environ.get("X-Forwarded-For") + ip = None + port = None + if x_forwarded_for is not None: + ip = x_forwarded_for + else: + ip = environ.get("REMOTE_ADDR") + port = environ.get("REMOTE_PORT") + + if _report_old(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.NET_PEER_IP, ip) + set_int_attribute(result, SpanAttributes.NET_PEER_PORT, port) + if _report_new(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.CLIENT_ADDRESS, ip) + set_int_attribute(result, SpanAttributes.CLIENT_PORT, port) + +def _set_user_agent(result, user_agent, sem_conv_opt_in_mode): + if _report_old(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.HTTP_USER_AGENT, user_agent) + if _report_new(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.USER_AGENT_ORIGINAL, user_agent) + + +def _set_protocol_version(result, version, sem_conv_opt_in_mode): + if _report_old(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.HTTP_FLAVOR, version) + if _report_new(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.NETWORK_PROTOCOL_VERSION, version) + +def _parse_http_host(environ): + # TODO: support forwarded#host, etc + x_forwarded_host = environ.get("X-Forwarded-Host") + if x_forwarded_host is not None: + return parse_http_host(x_forwarded_host) + + (host, port) = parse_http_host(environ.get("HTTP_HOST")) + return (host or environ.get("SERVER_NAME"), port or environ.get("SERVER_PORT")) + +def collect_request_attributes(environ, sem_conv_opt_in_mode): """Collects HTTP request attributes from the PEP3333-conforming WSGI environ and returns a dictionary to be used as span creation attributes. """ - result = { - SpanAttributes.HTTP_METHOD: sanitize_method(environ.get("REQUEST_METHOD")), - SpanAttributes.HTTP_SERVER_NAME: environ.get("SERVER_NAME"), - SpanAttributes.HTTP_SCHEME: environ.get("wsgi.url_scheme"), - } + attributes = {} - host_port = environ.get("SERVER_PORT") - if host_port is not None and not host_port == "": - result.update({SpanAttributes.NET_HOST_PORT: int(host_port)}) + _set_http_method(attributes, environ.get("REQUEST_METHOD"), sem_conv_opt_in_mode) + _set_scheme(attributes, environ.get("wsgi.url_scheme"), sem_conv_opt_in_mode) - setifnotnone(result, SpanAttributes.HTTP_HOST, environ.get("HTTP_HOST")) - target = environ.get("RAW_URI") - if target is None: # Note: `"" or None is None` - target = environ.get("REQUEST_URI") - if target is not None: - result[SpanAttributes.HTTP_TARGET] = target - else: - result[SpanAttributes.HTTP_URL] = remove_url_credentials( - wsgiref_util.request_uri(environ) - ) + # following https://peps.python.org/pep-3333/#url-reconstruction + falling back to RAW_URI + REQUEST_URI + (host, port) = _parse_http_host(environ) + _set_host_port(attributes, host, port, sem_conv_opt_in_mode) + _parse_scheme_path_and_query(environ, attributes, sem_conv_opt_in_mode) + _set_user_agent(attributes, environ.get("HTTP_USER_AGENT"), sem_conv_opt_in_mode) + _set_peer_ip_port(attributes, environ, sem_conv_opt_in_mode) - remote_addr = environ.get("REMOTE_ADDR") - if remote_addr: - result[SpanAttributes.NET_PEER_IP] = remote_addr - remote_host = environ.get("REMOTE_HOST") - if remote_host and remote_host != remote_addr: - result[SpanAttributes.NET_PEER_NAME] = remote_host + http_version = environ.get("SERVER_PROTOCOL", "") + if http_version.upper().startswith(_HTTP_VERSION_PREFIX): + http_version = http_version[len(_HTTP_VERSION_PREFIX) :] + _set_protocol_version(attributes, http_version, sem_conv_opt_in_mode) - user_agent = environ.get("HTTP_USER_AGENT") - if user_agent is not None and len(user_agent) > 0: - result[SpanAttributes.HTTP_USER_AGENT] = user_agent - - setifnotnone( - result, SpanAttributes.NET_PEER_PORT, environ.get("REMOTE_PORT") - ) - flavor = environ.get("SERVER_PROTOCOL", "") - if flavor.upper().startswith(_HTTP_VERSION_PREFIX): - flavor = flavor[len(_HTTP_VERSION_PREFIX) :] - if flavor: - result[SpanAttributes.HTTP_FLAVOR] = flavor - - return result + return attributes def collect_custom_request_headers_attributes(environ): @@ -404,46 +514,71 @@ def _parse_status_code(resp_status): return None -def _parse_active_request_count_attrs(req_attrs): +def _filter_active_request_count_attrs(req_attrs, sem_conv_opt_in_mode): active_requests_count_attrs = {} - for attr_key in _active_requests_count_attrs: - if req_attrs.get(attr_key) is not None: - active_requests_count_attrs[attr_key] = req_attrs[attr_key] - return active_requests_count_attrs + if _report_old(sem_conv_opt_in_mode): + for attr_key in _active_requests_count_attrs_old: + if req_attrs.get(attr_key) is not None: + active_requests_count_attrs[attr_key] = req_attrs[attr_key] + if _report_new(sem_conv_opt_in_mode): + for attr_key in _active_requests_count_attrs_new: + if req_attrs.get(attr_key) is not None: + active_requests_count_attrs[attr_key] = req_attrs[attr_key] + return active_requests_count_attrs -def _parse_duration_attrs(req_attrs): +def _filter_duration_attrs(req_attrs, sem_conv_opt_in_mode): duration_attrs = {} - for attr_key in _duration_attrs: + # duration is two different metrics depending on sem_conv_opt_in_mode, so no DUP attributes + allowed_attributes = _duration_attrs_new if sem_conv_opt_in_mode == _OpenTelemetryStabilityMode.HTTP else _duration_attrs_old + for attr_key in allowed_attributes: if req_attrs.get(attr_key) is not None: duration_attrs[attr_key] = req_attrs[attr_key] return duration_attrs +def _set_status(span, metrics_attributes, status_code_str, status_code, sem_conv_opt_in_mode): + if (status_code < 0): + if _report_new(sem_conv_opt_in_mode): + span.set_attribute(_SPAN_ATTRIBUTES_ERROR_TYPE, status_code_str) + metrics_attributes[_SPAN_ATTRIBUTES_ERROR_TYPE] = status_code_str + + span.set_status( + Status( + StatusCode.ERROR, + "Non-integer HTTP status: " + status_code_str, + ) + ) + + status = http_status_to_status_code(status_code, server_span=True) + + if _report_old(sem_conv_opt_in_mode): + span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, status_code) + metrics_attributes[SpanAttributes.HTTP_STATUS_CODE] = status_code + if _report_new(sem_conv_opt_in_mode): + span.set_attribute(SpanAttributes.HTTP_RESPONSE_STATUS_CODE, status_code) + metrics_attributes[SpanAttributes.HTTP_RESPONSE_STATUS_CODE] = status_code + if status == StatusCode.ERROR: + span.set_attribute(_SPAN_ATTRIBUTES_ERROR_TYPE, status_code_str) + metrics_attributes[_SPAN_ATTRIBUTES_ERROR_TYPE] = status_code_str + span.set_status(Status(status)) def add_response_attributes( - span, start_response_status, response_headers + span, start_response_status, response_headers, duration_attrs, sem_conv_opt_in_mode ): # pylint: disable=unused-argument """Adds HTTP response attributes to span using the arguments passed to a PEP3333-conforming start_response callable. """ if not span.is_recording(): return - status_code, _ = start_response_status.split(" ", 1) + status_code_str, _ = start_response_status.split(" ", 1) + status_code = 0 try: - status_code = int(status_code) + status_code = int(status_code_str) except ValueError: - span.set_status( - Status( - StatusCode.ERROR, - "Non-integer HTTP status: " + repr(status_code), - ) - ) - else: - span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, status_code) - span.set_status( - Status(http_status_to_status_code(status_code, server_span=True)) - ) + status_code = -1 + + _set_status(span, duration_attrs, status_code_str, status_code, sem_conv_opt_in_mode) def get_default_span_name(environ): @@ -457,10 +592,11 @@ def get_default_span_name(environ): Returns: The span name. """ + method = sanitize_method(environ.get("REQUEST_METHOD", "").strip()) - path = environ.get("PATH_INFO", "").strip() - if method and path: - return f"{method} {path}" + if method == "_OTHER": + return "HTTP" + # There is no routing in WSGI and path should not be in the span name. return method @@ -488,33 +624,55 @@ def __init__( response_hook=None, tracer_provider=None, meter_provider=None, + sem_conv_opt_in_mode: _OpenTelemetryStabilityMode = _OpenTelemetryStabilityMode.DEFAULT ): self.wsgi = wsgi - self.tracer = trace.get_tracer(__name__, __version__, tracer_provider) - self.meter = get_meter(__name__, __version__, meter_provider) - self.duration_histogram = self.meter.create_histogram( - name=MetricInstruments.HTTP_SERVER_DURATION, - unit="ms", - description="measures the duration of the inbound HTTP request", + self.tracer = trace.get_tracer( + __name__, + __version__, + tracer_provider, + schema_url=_get_schema_url(sem_conv_opt_in_mode), + ) + self.meter = get_meter( + __name__, + __version__, + meter_provider, + schema_url=_get_schema_url(sem_conv_opt_in_mode), ) + + + self.duration_histogram_old = None + if _report_old(sem_conv_opt_in_mode): + self.duration_histogram_old = self.meter.create_histogram( + name=MetricInstruments.HTTP_SERVER_DURATION, + unit="ms", + description="measures the duration of the inbound HTTP request", + ) + self.duration_histogram_new = None + if _report_new(sem_conv_opt_in_mode): + self.duration_histogram_new = self.meter.create_histogram( + name=_METRIC_INSTRUMENTS_HTTP_SERVER_REQUEST_DURATION, + unit="s", + description="measures the duration of the inbound HTTP request", + ) + self.active_requests_counter = self.meter.create_up_down_counter( name=MetricInstruments.HTTP_SERVER_ACTIVE_REQUESTS, - unit="requests", + unit="{request}", description="measures the number of concurrent HTTP requests that are currently in-flight", ) self.request_hook = request_hook self.response_hook = response_hook + self.sem_conv_opt_in_mode = sem_conv_opt_in_mode @staticmethod def _create_start_response( - span, start_response, response_hook, duration_attrs + span, start_response, response_hook, duration_attrs, sem_conv_opt_in_mode ): @functools.wraps(start_response) def _start_response(status, response_headers, *args, **kwargs): - add_response_attributes(span, status, response_headers) - status_code = _parse_status_code(status) - if status_code is not None: - duration_attrs[SpanAttributes.HTTP_STATUS_CODE] = status_code + add_response_attributes(span, status, response_headers, duration_attrs, sem_conv_opt_in_mode) + if span.is_recording() and span.kind == trace.SpanKind.SERVER: custom_attributes = collect_custom_response_headers_attributes( response_headers @@ -535,11 +693,7 @@ def __call__(self, environ, start_response): environ: A WSGI environment. start_response: The WSGI start_response callable. """ - req_attrs = collect_request_attributes(environ) - active_requests_count_attrs = _parse_active_request_count_attrs( - req_attrs - ) - duration_attrs = _parse_duration_attrs(req_attrs) + attributes = collect_request_attributes(environ, self.sem_conv_opt_in_mode) span, token = _start_internal_or_server_span( tracer=self.tracer, @@ -547,7 +701,7 @@ def __call__(self, environ, start_response): start_time=None, context_carrier=environ, context_getter=wsgi_getter, - attributes=req_attrs, + attributes=attributes, ) if span.is_recording() and span.kind == trace.SpanKind.SERVER: custom_attributes = collect_custom_request_headers_attributes( @@ -564,24 +718,37 @@ def __call__(self, environ, start_response): response_hook = functools.partial(response_hook, span, environ) start = default_timer() + + active_requests_count_attrs = _filter_active_request_count_attrs( + attributes, self.sem_conv_opt_in_mode + ) self.active_requests_counter.add(1, active_requests_count_attrs) try: with trace.use_span(span): start_response = self._create_start_response( - span, start_response, response_hook, duration_attrs + span, start_response, response_hook, attributes, self.sem_conv_opt_in_mode ) iterable = self.wsgi(environ, start_response) return _end_span_after_iterating(iterable, span, token) except Exception as ex: - if span.is_recording(): - span.set_status(Status(StatusCode.ERROR, str(ex))) + if self.sem_conv_opt_in_mode != _OpenTelemetryStabilityMode.DEFAULT: + if span.is_recording(): + span.set_attribute(_SPAN_ATTRIBUTES_ERROR_TYPE, type(ex).__qualname__ ) + attributes[_SPAN_ATTRIBUTES_ERROR_TYPE] = type(ex).__qualname__ + span.set_status(Status(StatusCode.ERROR, str(ex))) span.end() if token is not None: context.detach(token) raise finally: - duration = max(round((default_timer() - start) * 1000), 0) - self.duration_histogram.record(duration, duration_attrs) + duration = default_timer() - start + if self.duration_histogram_old is not None: + duration_attrs_old = _filter_duration_attrs(attributes, _OpenTelemetryStabilityMode.DEFAULT) + self.duration_histogram_old.record(max(round(duration * 1000), 0), duration_attrs_old) + if self.duration_histogram_new is not None: + duration_attrs_new = _filter_duration_attrs(attributes, _OpenTelemetryStabilityMode.HTTP) + self.duration_histogram_new.record(duration, duration_attrs_new) + self.active_requests_counter.add(-1, active_requests_count_attrs) diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py index 6aef096218..a953f7f947 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py @@ -35,7 +35,7 @@ OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_RESPONSE, OTEL_PYTHON_INSTRUMENTATION_HTTP_CAPTURE_ALL_METHODS, ) - +from opentelemetry.instrumentation.utils import _OpenTelemetryStabilityMode class Response: def __init__(self): @@ -114,13 +114,33 @@ def wsgi_with_custom_response_headers(environ, start_response): return [b"*"] -_expected_metric_names = [ +_expected_metric_names_old = [ "http.server.active_requests", "http.server.duration", ] -_recommended_attrs = { - "http.server.active_requests": otel_wsgi._active_requests_count_attrs, - "http.server.duration": otel_wsgi._duration_attrs, +_expected_attrs_old = { + "http.server.active_requests": otel_wsgi._active_requests_count_attrs_old, + "http.server.duration": otel_wsgi._duration_attrs_old, +} + +_expected_metric_names_new = [ + "http.server.active_requests", + "http.server.request.duration", +] +_expected_attrs_new = { + "http.server.active_requests": otel_wsgi._active_requests_count_attrs_new, + "http.server.request.duration": otel_wsgi._duration_attrs_new, +} + +_expected_metric_names_dup = [ + "http.server.active_requests", + "http.server.request.duration", + "http.server.duration", +] +_expected_attrs_dup = { + "http.server.active_requests": otel_wsgi._active_requests_count_attrs_new + otel_wsgi._active_requests_count_attrs_old, + "http.server.request.duration": otel_wsgi._duration_attrs_new, + "http.server.duration": otel_wsgi._duration_attrs_old, } @@ -129,7 +149,7 @@ def validate_response( self, response, error=None, - span_name="GET /", + span_name="GET", http_method="GET", span_attributes=None, response_headers=None, @@ -159,17 +179,17 @@ def validate_response( self.assertEqual(span_list[0].name, span_name) self.assertEqual(span_list[0].kind, trace_api.SpanKind.SERVER) expected_attributes = { - SpanAttributes.HTTP_SERVER_NAME: "127.0.0.1", - SpanAttributes.HTTP_SCHEME: "http", + SpanAttributes.NET_HOST_NAME: "127.0.0.1", SpanAttributes.NET_HOST_PORT: 80, - SpanAttributes.HTTP_HOST: "127.0.0.1", + SpanAttributes.HTTP_SCHEME: "http", + SpanAttributes.HTTP_TARGET: "/", SpanAttributes.HTTP_FLAVOR: "1.0", - SpanAttributes.HTTP_URL: "http://127.0.0.1/", SpanAttributes.HTTP_STATUS_CODE: 200, } expected_attributes.update(span_attributes or {}) if http_method is not None: expected_attributes[SpanAttributes.HTTP_METHOD] = http_method + self.assertEqual(span_list[0].attributes, expected_attributes) def test_basic_wsgi_call(self): @@ -245,8 +265,8 @@ def test_wsgi_exc_info(self): response = app(self.environ, self.start_response) self.validate_response(response, error=ValueError) - def test_wsgi_internal_error(self): - app = otel_wsgi.OpenTelemetryMiddleware(error_wsgi_unhandled) + def test_wsgi_internal_error_old(self): + app = otel_wsgi.OpenTelemetryMiddleware(error_wsgi_unhandled, sem_conv_opt_in_mode=_OpenTelemetryStabilityMode.DEFAULT) self.assertRaises(ValueError, app, self.environ, self.start_response) span_list = self.memory_exporter.get_finished_spans() self.assertEqual(len(span_list), 1) @@ -255,8 +275,83 @@ def test_wsgi_internal_error(self): StatusCode.ERROR, ) - def test_wsgi_metrics(self): - app = otel_wsgi.OpenTelemetryMiddleware(error_wsgi_unhandled) + def test_wsgi_internal_error_new(self): + app = otel_wsgi.OpenTelemetryMiddleware(error_wsgi_unhandled, sem_conv_opt_in_mode=_OpenTelemetryStabilityMode.HTTP) + self.assertRaises(ValueError, app, self.environ, self.start_response) + span_list = self.memory_exporter.get_finished_spans() + self.assertEqual(len(span_list), 1) + self.assertEqual( + span_list[0].status.status_code, + StatusCode.ERROR, + ) + self.assertEqual( + span_list[0].attributes["error.type"], + "ValueError", + ) + + def test_wsgi_metrics_old(self): + app = otel_wsgi.OpenTelemetryMiddleware(error_wsgi_unhandled, sem_conv_opt_in_mode=_OpenTelemetryStabilityMode.DEFAULT) + self.assertRaises(ValueError, app, self.environ, self.start_response) + self.assertRaises(ValueError, app, self.environ, self.start_response) + self.assertRaises(ValueError, app, self.environ, self.start_response) + metrics_list = self.memory_metrics_reader.get_metrics_data() + number_data_point_seen = False + histogram_data_point_seen = False + + self.assertTrue(len(metrics_list.resource_metrics) != 0) + for resource_metric in metrics_list.resource_metrics: + self.assertTrue(len(resource_metric.scope_metrics) != 0) + for scope_metric in resource_metric.scope_metrics: + self.assertTrue(len(scope_metric.metrics) != 0) + for metric in scope_metric.metrics: + self.assertIn(metric.name, _expected_metric_names_old) + data_points = list(metric.data.data_points) + self.assertEqual(len(data_points), 1) + for point in data_points: + if isinstance(point, HistogramDataPoint): + self.assertEqual(point.count, 3) + histogram_data_point_seen = True + if isinstance(point, NumberDataPoint): + number_data_point_seen = True + for attr in point.attributes: + self.assertIn( + attr, _expected_attrs_old[metric.name] + ) + self.assertTrue(number_data_point_seen and histogram_data_point_seen) + + def test_wsgi_metrics_new(self): + app = otel_wsgi.OpenTelemetryMiddleware(error_wsgi_unhandled, sem_conv_opt_in_mode=_OpenTelemetryStabilityMode.HTTP) + self.assertRaises(ValueError, app, self.environ, self.start_response) + self.assertRaises(ValueError, app, self.environ, self.start_response) + self.assertRaises(ValueError, app, self.environ, self.start_response) + metrics_list = self.memory_metrics_reader.get_metrics_data() + number_data_point_seen = False + histogram_data_point_seen = False + + self.assertTrue(len(metrics_list.resource_metrics) != 0) + for resource_metric in metrics_list.resource_metrics: + self.assertTrue(len(resource_metric.scope_metrics) != 0) + for scope_metric in resource_metric.scope_metrics: + self.assertTrue(len(scope_metric.metrics) != 0) + for metric in scope_metric.metrics: + self.assertIn(metric.name, _expected_metric_names_new) + data_points = list(metric.data.data_points) + self.assertEqual(len(data_points), 1) + for point in data_points: + if isinstance(point, HistogramDataPoint): + self.assertEqual(point.count, 3) + histogram_data_point_seen = True + if isinstance(point, NumberDataPoint): + number_data_point_seen = True + for attr in point.attributes: + self.assertIn( + attr, _expected_attrs_new[metric.name] + ) + self.assertTrue(number_data_point_seen and histogram_data_point_seen) + + + def test_wsgi_metrics_dup(self): + app = otel_wsgi.OpenTelemetryMiddleware(error_wsgi_unhandled, sem_conv_opt_in_mode=_OpenTelemetryStabilityMode.HTTP_DUP) self.assertRaises(ValueError, app, self.environ, self.start_response) self.assertRaises(ValueError, app, self.environ, self.start_response) self.assertRaises(ValueError, app, self.environ, self.start_response) @@ -270,7 +365,7 @@ def test_wsgi_metrics(self): for scope_metric in resource_metric.scope_metrics: self.assertTrue(len(scope_metric.metrics) != 0) for metric in scope_metric.metrics: - self.assertIn(metric.name, _expected_metric_names) + self.assertIn(metric.name, _expected_metric_names_dup) data_points = list(metric.data.data_points) self.assertEqual(len(data_points), 1) for point in data_points: @@ -281,7 +376,7 @@ def test_wsgi_metrics(self): number_data_point_seen = True for attr in point.attributes: self.assertIn( - attr, _recommended_attrs[metric.name] + attr, _expected_attrs_dup[metric.name] ) self.assertTrue(number_data_point_seen and histogram_data_point_seen) @@ -289,7 +384,7 @@ def test_nonstandard_http_method(self): self.environ["REQUEST_METHOD"]= "NONSTANDARD" app = otel_wsgi.OpenTelemetryMiddleware(simple_wsgi) response = app(self.environ, self.start_response) - self.validate_response(response, span_name="UNKNOWN /", http_method="UNKNOWN") + self.validate_response(response, span_name="HTTP", http_method="_OTHER") @mock.patch.dict( "os.environ", @@ -301,7 +396,7 @@ def test_nonstandard_http_method_allowed(self): self.environ["REQUEST_METHOD"]= "NONSTANDARD" app = otel_wsgi.OpenTelemetryMiddleware(simple_wsgi) response = app(self.environ, self.start_response) - self.validate_response(response, span_name="NONSTANDARD /", http_method="NONSTANDARD") + self.validate_response(response, span_name="NONSTANDARD", http_method="NONSTANDARD") def test_default_span_name_missing_path_info(self): """Test that default span_names with missing path info.""" @@ -318,46 +413,84 @@ def setUp(self): wsgiref_util.setup_testing_defaults(self.environ) self.span = mock.create_autospec(trace_api.Span, spec_set=True) - def test_request_attributes(self): + def test_request_attributes_old(self): self.environ["QUERY_STRING"] = "foo=bar" - attrs = otel_wsgi.collect_request_attributes(self.environ) + attrs = otel_wsgi.collect_request_attributes(self.environ, _OpenTelemetryStabilityMode.DEFAULT) self.assertDictEqual( attrs, { SpanAttributes.HTTP_METHOD: "GET", - SpanAttributes.HTTP_HOST: "127.0.0.1", - SpanAttributes.HTTP_URL: "http://127.0.0.1/?foo=bar", + SpanAttributes.NET_HOST_NAME: "127.0.0.1", SpanAttributes.NET_HOST_PORT: 80, + SpanAttributes.HTTP_TARGET: "/?foo=bar", SpanAttributes.HTTP_SCHEME: "http", - SpanAttributes.HTTP_SERVER_NAME: "127.0.0.1", SpanAttributes.HTTP_FLAVOR: "1.0", }, ) + def test_request_attributes_new(self): + self.environ["QUERY_STRING"] = "foo=bar" + + attrs = otel_wsgi.collect_request_attributes(self.environ, _OpenTelemetryStabilityMode.HTTP) + self.assertDictEqual( + attrs, + { + SpanAttributes.HTTP_REQUEST_METHOD: "GET", + SpanAttributes.SERVER_ADDRESS: "127.0.0.1", + SpanAttributes.SERVER_PORT: 80, + SpanAttributes.URL_PATH: "/", + SpanAttributes.URL_QUERY: "foo=bar", + SpanAttributes.URL_SCHEME: "http", + SpanAttributes.NETWORK_PROTOCOL_VERSION: "1.0", + }, + ) + + def test_request_attributes_dup(self): + self.environ["QUERY_STRING"] = "foo=bar" + + attrs = otel_wsgi.collect_request_attributes(self.environ, _OpenTelemetryStabilityMode.HTTP_DUP) + self.assertDictEqual( + attrs, + { + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.NET_HOST_NAME: "127.0.0.1", + SpanAttributes.NET_HOST_PORT: 80, + SpanAttributes.HTTP_TARGET: "/?foo=bar", + SpanAttributes.HTTP_SCHEME: "http", + SpanAttributes.HTTP_FLAVOR: "1.0", + SpanAttributes.HTTP_REQUEST_METHOD: "GET", + SpanAttributes.SERVER_ADDRESS: "127.0.0.1", + SpanAttributes.SERVER_PORT: 80, + SpanAttributes.URL_PATH: "/", + SpanAttributes.URL_QUERY: "foo=bar", + SpanAttributes.URL_SCHEME: "http", + SpanAttributes.NETWORK_PROTOCOL_VERSION: "1.0", + }, + ) + def validate_url(self, expected_url, raw=False, has_host=True): parts = urlsplit(expected_url) expected = { SpanAttributes.HTTP_SCHEME: parts.scheme, SpanAttributes.NET_HOST_PORT: parts.port or (80 if parts.scheme == "http" else 443), - SpanAttributes.HTTP_SERVER_NAME: parts.hostname, # Not true in the general case, but for all tests. + SpanAttributes.NET_HOST_NAME: parts.hostname, # Not true in the general case, but for all tests. } if raw: - expected[SpanAttributes.HTTP_TARGET] = expected_url.split( - parts.netloc, 1 - )[1] - else: - expected[SpanAttributes.HTTP_URL] = expected_url + expected[SpanAttributes.HTTP_TARGET] = parts.path + ( + "?" + parts.query if parts.query else "" + ) + if has_host: - expected[SpanAttributes.HTTP_HOST] = parts.hostname + expected[SpanAttributes.NET_HOST_NAME] = parts.hostname - attrs = otel_wsgi.collect_request_attributes(self.environ) + attrs = otel_wsgi.collect_request_attributes(self.environ, _OpenTelemetryStabilityMode.DEFAULT) self.assertGreaterEqual( attrs.items(), expected.items(), expected_url + " expected." ) - def test_request_attributes_with_partial_raw_uri(self): + def test_request_attributes_with_fragment_raw_uri(self): self.environ["RAW_URI"] = "/#top" self.validate_url("http://127.0.0.1/#top", raw=True) @@ -406,24 +539,25 @@ def test_request_attributes_with_conflicting_nonstandard_port(self): "HTTP_HOST" ] += ":8080" # Note that we do not correct SERVER_PORT expected = { - SpanAttributes.HTTP_HOST: "127.0.0.1:8080", - SpanAttributes.HTTP_URL: "http://127.0.0.1:8080/", - SpanAttributes.NET_HOST_PORT: 80, + SpanAttributes.NET_HOST_NAME: "127.0.0.1", + SpanAttributes.NET_HOST_PORT: 8080, + SpanAttributes.HTTP_TARGET: "/", } self.assertGreaterEqual( - otel_wsgi.collect_request_attributes(self.environ).items(), + otel_wsgi.collect_request_attributes(self.environ, _OpenTelemetryStabilityMode.DEFAULT).items(), expected.items(), ) def test_request_attributes_with_faux_scheme_relative_raw_uri(self): + # // indicates absolute url per https://datatracker.ietf.org/doc/html/rfc1808.html#section-2.4.3, i.e. no path here self.environ["RAW_URI"] = "//127.0.0.1/?" - self.validate_url("http://127.0.0.1//127.0.0.1/?", raw=True) + self.validate_url("http://127.0.0.1/?", raw=True) def test_request_attributes_pathless(self): self.environ["RAW_URI"] = "" - expected = {SpanAttributes.HTTP_TARGET: ""} + expected = {SpanAttributes.HTTP_TARGET: "/"} self.assertGreaterEqual( - otel_wsgi.collect_request_attributes(self.environ).items(), + otel_wsgi.collect_request_attributes(self.environ, _OpenTelemetryStabilityMode.DEFAULT).items(), expected.items(), ) @@ -432,13 +566,14 @@ def test_request_attributes_with_full_request_uri(self): self.environ["REQUEST_METHOD"] = "CONNECT" self.environ[ "REQUEST_URI" - ] = "127.0.0.1:8080" # Might happen in a CONNECT request + ] = "http://127.0.0.1:8080" # Might happen in a CONNECT request expected = { - SpanAttributes.HTTP_HOST: "127.0.0.1:8080", - SpanAttributes.HTTP_TARGET: "127.0.0.1:8080", + SpanAttributes.NET_HOST_NAME: "127.0.0.1", + SpanAttributes.NET_HOST_PORT: 8080, + SpanAttributes.HTTP_TARGET: "/", } self.assertGreaterEqual( - otel_wsgi.collect_request_attributes(self.environ).items(), + otel_wsgi.collect_request_attributes(self.environ, _OpenTelemetryStabilityMode.DEFAULT).items(), expected.items(), ) @@ -446,12 +581,12 @@ def test_http_user_agent_attribute(self): self.environ["HTTP_USER_AGENT"] = "test-useragent" expected = {SpanAttributes.HTTP_USER_AGENT: "test-useragent"} self.assertGreaterEqual( - otel_wsgi.collect_request_attributes(self.environ).items(), + otel_wsgi.collect_request_attributes(self.environ, _OpenTelemetryStabilityMode.DEFAULT).items(), expected.items(), ) def test_response_attributes(self): - otel_wsgi.add_response_attributes(self.span, "404 Not Found", {}) + otel_wsgi.add_response_attributes(self.span, "404 Not Found", {}, {}, sem_conv_opt_in_mode=_OpenTelemetryStabilityMode.DEFAULT) expected = (mock.call(SpanAttributes.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) @@ -460,11 +595,12 @@ def test_credential_removal(self): self.environ["HTTP_HOST"] = "username:password@mock" self.environ["PATH_INFO"] = "/status/200" expected = { - SpanAttributes.HTTP_URL: "http://mock/status/200", SpanAttributes.NET_HOST_PORT: 80, + SpanAttributes.HTTP_TARGET: "/status/200", + SpanAttributes.NET_HOST_NAME: "mock", } self.assertGreaterEqual( - otel_wsgi.collect_request_attributes(self.environ).items(), + otel_wsgi.collect_request_attributes(self.environ, _OpenTelemetryStabilityMode.DEFAULT).items(), expected.items(), ) @@ -475,7 +611,7 @@ def validate_response( response, exporter, error=None, - span_name="GET /", + span_name="GET", http_method="GET", ): while True: diff --git a/opentelemetry-instrumentation/pyproject.toml b/opentelemetry-instrumentation/pyproject.toml index b781f51542..ca733db047 100644 --- a/opentelemetry-instrumentation/pyproject.toml +++ b/opentelemetry-instrumentation/pyproject.toml @@ -26,6 +26,7 @@ classifiers = [ ] dependencies = [ "opentelemetry-api ~= 1.4", + "opentelemetry-semantic-conventions == 0.42b0.dev", "setuptools >= 16.0", "wrapt >= 1.0.0, < 2.0.0", ] diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py index 35a55a1279..e1e615334b 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/utils.py @@ -12,6 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import os +import threading import urllib.parse from re import escape, sub from typing import Dict, Sequence @@ -24,6 +26,8 @@ # pylint: disable=E0611 from opentelemetry.context import _SUPPRESS_INSTRUMENTATION_KEY # noqa: F401 from opentelemetry.propagate import extract +from opentelemetry.semconv.resource import ResourceAttributes +from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import StatusCode from opentelemetry.trace.propagation.tracecontext import ( TraceContextTextMapPropagator, @@ -152,3 +156,72 @@ def _python_path_without_directory(python_path, directory, path_separator): "", python_path, ) + + +_OTEL_SEMCONV_STABILITY_OPT_IN_KEY = "OTEL_SEMCONV_STABILITY_OPT_IN" + +class _OpenTelemetryStabilitySignalType: + HTTP = "http" + + +class _OpenTelemetryStabilityMode: + # http - emit the new, stable HTTP and networking conventions ONLY + HTTP = "http" + # http/dup - emit both the old and the stable HTTP and networking conventions + HTTP_DUP = "http/dup" + # default - continue emitting old experimental HTTP and networking conventions + DEFAULT = "default" + + +def _report_new(mode): + return mode == _OpenTelemetryStabilityMode.HTTP or mode == _OpenTelemetryStabilityMode.HTTP_DUP + +def _report_old(mode): + return mode == _OpenTelemetryStabilityMode.DEFAULT or mode == _OpenTelemetryStabilityMode.HTTP_DUP + + +class _OpenTelemetrySemanticConventionStability: + _initialized = False + _lock = threading.Lock() + _OTEL_SEMCONV_STABILITY_SIGNAL_MAPPING = {} + + @classmethod + def _initialize(cls): + with _OpenTelemetrySemanticConventionStability._lock: + if not _OpenTelemetrySemanticConventionStability._initialized: + # Users can pass in comma delimited string for opt-in options + # Only values for http stability are supported for now + opt_in = os.environ.get(_OTEL_SEMCONV_STABILITY_OPT_IN_KEY, "") + opt_in_list = [] + if opt_in: + opt_in_list = [s.strip() for s in opt_in.split(",")] + http_opt_in = _OpenTelemetryStabilityMode.DEFAULT + if opt_in_list: + # Process http opt-in + # http/dup takes priority over http + if _OpenTelemetryStabilityMode.HTTP_DUP in opt_in_list: + http_opt_in = _OpenTelemetryStabilityMode.HTTP_DUP + elif _OpenTelemetryStabilityMode.HTTP in opt_in_list: + http_opt_in = _OpenTelemetryStabilityMode.HTTP + _OpenTelemetrySemanticConventionStability._OTEL_SEMCONV_STABILITY_SIGNAL_MAPPING[ + _OpenTelemetryStabilitySignalType.HTTP + ] = http_opt_in + _OpenTelemetrySemanticConventionStability._initialized = True + + + @classmethod + # Get OpenTelemetry opt-in mode based off of signal type (http, messaging, etc.) + def _get_opentelemetry_stability_opt_in_mode( + cls, + signal_type: _OpenTelemetryStabilitySignalType, + ) -> _OpenTelemetryStabilityMode: + return _OpenTelemetrySemanticConventionStability._OTEL_SEMCONV_STABILITY_SIGNAL_MAPPING.get( + signal_type, _OpenTelemetryStabilityMode.DEFAULT + ) + + +# Get schema version based off of opt-in mode +def _get_schema_url(mode: _OpenTelemetryStabilityMode) -> str: + if mode is _OpenTelemetryStabilityMode.DEFAULT: + return "https://opentelemetry.io/schemas/1.11.0" + return SpanAttributes.SCHEMA_URL diff --git a/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py b/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py index 4f4a5d0353..2f933a06ca 100644 --- a/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py +++ b/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py @@ -16,7 +16,7 @@ from re import IGNORECASE as RE_IGNORECASE from re import compile as re_compile from re import search -from typing import Iterable, List, Optional +from typing import Iterable, List, Optional, Tuple from urllib.parse import urlparse, urlunparse from opentelemetry.semconv.trace import SpanAttributes @@ -38,21 +38,20 @@ # List of recommended metrics attributes _duration_attrs = { SpanAttributes.HTTP_METHOD, - SpanAttributes.HTTP_HOST, + SpanAttributes.NET_HOST_NAME, + SpanAttributes.NET_HOST_PORT, SpanAttributes.HTTP_SCHEME, SpanAttributes.HTTP_STATUS_CODE, - SpanAttributes.HTTP_FLAVOR, - SpanAttributes.HTTP_SERVER_NAME, - SpanAttributes.NET_HOST_NAME, + SpanAttributes.NET_PROTOCOL_NAME, + SpanAttributes.NET_PROTOCOL_VERSION, SpanAttributes.NET_HOST_PORT, } _active_requests_count_attrs = { SpanAttributes.HTTP_METHOD, - SpanAttributes.HTTP_HOST, SpanAttributes.HTTP_SCHEME, - SpanAttributes.HTTP_FLAVOR, - SpanAttributes.HTTP_SERVER_NAME, + SpanAttributes.NET_HOST_NAME, + SpanAttributes.NET_HOST_PORT, } @@ -198,7 +197,7 @@ def sanitize_method(method: Optional[str]) -> Optional[str]: # Based on https://www.rfc-editor.org/rfc/rfc7231#section-4.1 and https://www.rfc-editor.org/rfc/rfc5789#section-2. method in ["GET", "HEAD", "POST", "PUT", "DELETE", "CONNECT", "OPTIONS", "TRACE", "PATCH"]): return method - return "UNKNOWN" + return "_OTHER" def get_custom_headers(env_var: str) -> List[str]: custom_headers = environ.get(env_var, []) @@ -210,6 +209,28 @@ def get_custom_headers(env_var: str) -> List[str]: return custom_headers +def parse_http_host(host_port) -> Tuple[str, str]: + if not host_port: + return (None, None) + + creds_end = host_port.find("@") + 1 + host_end = host_port.rfind(":", creds_end) + if host_end == -1: + return (host_port[creds_end:], None) + + return (host_port[creds_end:host_end], host_port[host_end + 1 :]) + + +def get_http_protocol_version(protocol_and_version) -> str: + if protocol_and_version == "HTTP/1.1": + return "1.1" + if protocol_and_version == "HTTP/1.0": + return "1.0" + if protocol_and_version == "HTTP/2": + return "2" + return None + + def _parse_active_request_count_attrs(req_attrs): active_requests_count_attrs = { key: req_attrs[key] diff --git a/util/opentelemetry-util-http/src/opentelemetry/util/http/httplib.py b/util/opentelemetry-util-http/src/opentelemetry/util/http/httplib.py index de95a0aa92..133d51e360 100644 --- a/util/opentelemetry-util-http/src/opentelemetry/util/http/httplib.py +++ b/util/opentelemetry-util-http/src/opentelemetry/util/http/httplib.py @@ -68,7 +68,7 @@ def _remove_nonrecording(spanlist: typing.List[Span]): def trysetip(conn: http.client.HTTPConnection, loglevel=logging.DEBUG) -> bool: - """Tries to set the net.peer.ip semantic attribute on the current span from the given + """Tries to set the net.sock.peer.addr semantic attribute on the current span from the given HttpConnection. Returns False if the connection is not yet established, False if the IP was captured @@ -105,7 +105,7 @@ def trysetip(conn: http.client.HTTPConnection, loglevel=logging.DEBUG) -> bool: ) else: for span in spanlist: - span.set_attribute(SpanAttributes.NET_PEER_IP, ip) + span.set_attribute(SpanAttributes.NET_SOCK_PEER_ADDR, ip) return True diff --git a/util/opentelemetry-util-http/tests/test_http_base.py b/util/opentelemetry-util-http/tests/test_http_base.py index 13f87a8f93..6988b9a090 100644 --- a/util/opentelemetry-util-http/tests/test_http_base.py +++ b/util/opentelemetry-util-http/tests/test_http_base.py @@ -64,7 +64,7 @@ def test_basic_with_span(self): assert resp.status == 200 assert body == b"Hello!" span = self.assert_span(num_spans=1) - self.assertEqual(span.attributes, {"net.peer.ip": "127.0.0.1"}) + self.assertEqual(span.attributes, {"net.sock.peer.addr": "127.0.0.1"}) def test_with_nested_span(self): tracer = trace.get_tracer(__name__) @@ -78,7 +78,9 @@ def test_with_nested_span(self): assert resp.status == 200 assert body == b"Hello!" for span in self.assert_span(num_spans=2): - self.assertEqual(span.attributes, {"net.peer.ip": "127.0.0.1"}) + self.assertEqual( + span.attributes, {"net.sock.peer.addr": "127.0.0.1"} + ) def test_with_nested_nonrecording_span(self): tracer = trace.get_tracer(__name__) @@ -92,7 +94,7 @@ def test_with_nested_nonrecording_span(self): assert resp.status == 200 assert body == b"Hello!" span = self.assert_span(num_spans=1) - self.assertEqual(span.attributes, {"net.peer.ip": "127.0.0.1"}) + self.assertEqual(span.attributes, {"net.sock.peer.addr": "127.0.0.1"}) def test_with_only_nonrecording_span(self): with trace.use_span(INVALID_SPAN), set_ip_on_next_http_connection( From 55252297a91f3269a9d497fbbcfffa39b15aef2f Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Thu, 26 Oct 2023 19:17:09 -0700 Subject: [PATCH 2/4] fix tests maybe --- .../src/opentelemetry/util/http/__init__.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py b/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py index 2f933a06ca..e286830827 100644 --- a/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py +++ b/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py @@ -42,9 +42,7 @@ SpanAttributes.NET_HOST_PORT, SpanAttributes.HTTP_SCHEME, SpanAttributes.HTTP_STATUS_CODE, - SpanAttributes.NET_PROTOCOL_NAME, - SpanAttributes.NET_PROTOCOL_VERSION, - SpanAttributes.NET_HOST_PORT, + SpanAttributes.HTTP_FLAVOR } _active_requests_count_attrs = { From 212b17b0c65b96652ea8f4219c7a900e45c09dfe Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Thu, 26 Oct 2023 19:21:36 -0700 Subject: [PATCH 3/4] removing unrelated --- .../src/opentelemetry/util/http/httplib.py | 4 ++-- util/opentelemetry-util-http/tests/test_http_base.py | 8 +++----- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/util/opentelemetry-util-http/src/opentelemetry/util/http/httplib.py b/util/opentelemetry-util-http/src/opentelemetry/util/http/httplib.py index 133d51e360..de95a0aa92 100644 --- a/util/opentelemetry-util-http/src/opentelemetry/util/http/httplib.py +++ b/util/opentelemetry-util-http/src/opentelemetry/util/http/httplib.py @@ -68,7 +68,7 @@ def _remove_nonrecording(spanlist: typing.List[Span]): def trysetip(conn: http.client.HTTPConnection, loglevel=logging.DEBUG) -> bool: - """Tries to set the net.sock.peer.addr semantic attribute on the current span from the given + """Tries to set the net.peer.ip semantic attribute on the current span from the given HttpConnection. Returns False if the connection is not yet established, False if the IP was captured @@ -105,7 +105,7 @@ def trysetip(conn: http.client.HTTPConnection, loglevel=logging.DEBUG) -> bool: ) else: for span in spanlist: - span.set_attribute(SpanAttributes.NET_SOCK_PEER_ADDR, ip) + span.set_attribute(SpanAttributes.NET_PEER_IP, ip) return True diff --git a/util/opentelemetry-util-http/tests/test_http_base.py b/util/opentelemetry-util-http/tests/test_http_base.py index 6988b9a090..13f87a8f93 100644 --- a/util/opentelemetry-util-http/tests/test_http_base.py +++ b/util/opentelemetry-util-http/tests/test_http_base.py @@ -64,7 +64,7 @@ def test_basic_with_span(self): assert resp.status == 200 assert body == b"Hello!" span = self.assert_span(num_spans=1) - self.assertEqual(span.attributes, {"net.sock.peer.addr": "127.0.0.1"}) + self.assertEqual(span.attributes, {"net.peer.ip": "127.0.0.1"}) def test_with_nested_span(self): tracer = trace.get_tracer(__name__) @@ -78,9 +78,7 @@ def test_with_nested_span(self): assert resp.status == 200 assert body == b"Hello!" for span in self.assert_span(num_spans=2): - self.assertEqual( - span.attributes, {"net.sock.peer.addr": "127.0.0.1"} - ) + self.assertEqual(span.attributes, {"net.peer.ip": "127.0.0.1"}) def test_with_nested_nonrecording_span(self): tracer = trace.get_tracer(__name__) @@ -94,7 +92,7 @@ def test_with_nested_nonrecording_span(self): assert resp.status == 200 assert body == b"Hello!" span = self.assert_span(num_spans=1) - self.assertEqual(span.attributes, {"net.sock.peer.addr": "127.0.0.1"}) + self.assertEqual(span.attributes, {"net.peer.ip": "127.0.0.1"}) def test_with_only_nonrecording_span(self): with trace.use_span(INVALID_SPAN), set_ip_on_next_http_connection( From f1c3e26c38703961564b61e53de464e1768dfa52 Mon Sep 17 00:00:00 2001 From: Liudmila Molkova Date: Thu, 26 Oct 2023 19:29:40 -0700 Subject: [PATCH 4/4] update sha --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8decdb1a42..ee1b852739 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -6,7 +6,7 @@ on: - 'release/*' pull_request: env: - CORE_REPO_SHA: 0ef76a5cc39626f783416ca75e43556e2bb2739d + CORE_REPO_SHA: d054dff47d2da663a39b9656d106c3d15f344269 jobs: build: