diff --git a/CHANGELOG.md b/CHANGELOG.md index 3bcc845bcc..ceb97433f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2372](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2372)) - Drop support for instrumenting elasticsearch client < 6` ([#2422](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2422)) +- `opentelemetry-instrumentation-wsgi` Add `http.method` to `span.name` + ([#2425](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2425)) +- `opentelemetry-instrumentation-flask` Add `http.method` to `span.name` + ([#2454](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2454)) ### Added @@ -23,6 +27,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#2382](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2382)) - `opentelemetry-instrumentation-wsgi` Implement new semantic convention opt-in with stable http semantic conventions ([#2425](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2425)) +- `opentelemetry-instrumentation-flask` Implement new semantic convention opt-in with stable http semantic conventions + ([#2454](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2454)) - `opentelemetry-instrumentation-threading` Initial release for threading ([#2253](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2253)) - `opentelemetry-instrumentation-pika` Instrumentation for `channel.consume()` (supported diff --git a/instrumentation/README.md b/instrumentation/README.md index 284037d707..c73d0f7c0a 100644 --- a/instrumentation/README.md +++ b/instrumentation/README.md @@ -20,7 +20,7 @@ | [opentelemetry-instrumentation-elasticsearch](./opentelemetry-instrumentation-elasticsearch) | elasticsearch >= 2.0 | No | experimental | [opentelemetry-instrumentation-falcon](./opentelemetry-instrumentation-falcon) | falcon >= 1.4.1, < 4.0.0 | Yes | experimental | [opentelemetry-instrumentation-fastapi](./opentelemetry-instrumentation-fastapi) | fastapi ~= 0.58 | Yes | experimental -| [opentelemetry-instrumentation-flask](./opentelemetry-instrumentation-flask) | flask >= 1.0 | Yes | experimental +| [opentelemetry-instrumentation-flask](./opentelemetry-instrumentation-flask) | flask >= 1.0 | Yes | migration | [opentelemetry-instrumentation-grpc](./opentelemetry-instrumentation-grpc) | grpcio ~= 1.27 | No | experimental | [opentelemetry-instrumentation-httpx](./opentelemetry-instrumentation-httpx) | httpx >= 0.18.0 | No | experimental | [opentelemetry-instrumentation-jinja2](./opentelemetry-instrumentation-jinja2) | jinja2 >= 2.7, < 4.0 | No | experimental @@ -48,4 +48,4 @@ | [opentelemetry-instrumentation-tortoiseorm](./opentelemetry-instrumentation-tortoiseorm) | tortoise-orm >= 0.17.0 | No | experimental | [opentelemetry-instrumentation-urllib](./opentelemetry-instrumentation-urllib) | urllib | Yes | experimental | [opentelemetry-instrumentation-urllib3](./opentelemetry-instrumentation-urllib3) | urllib3 >= 1.0.0, < 3.0.0 | Yes | experimental -| [opentelemetry-instrumentation-wsgi](./opentelemetry-instrumentation-wsgi) | wsgi | Yes | experimental \ No newline at end of file +| [opentelemetry-instrumentation-wsgi](./opentelemetry-instrumentation-wsgi) | wsgi | Yes | migration \ No newline at end of file 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 a17f83a877..f2e0ee34cc 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/__init__.py @@ -250,6 +250,15 @@ def response_hook(span: Span, status: str, response_headers: List): import opentelemetry.instrumentation.wsgi as otel_wsgi from opentelemetry import context, trace +from opentelemetry.instrumentation._semconv import ( + _METRIC_ATTRIBUTES_SERVER_DURATION_NAME, + _get_schema_url, + _HTTPStabilityMode, + _OpenTelemetrySemanticConventionStability, + _OpenTelemetryStabilitySignalType, + _report_new, + _report_old, +) from opentelemetry.instrumentation.flask.package import _instruments from opentelemetry.instrumentation.flask.version import __version__ from opentelemetry.instrumentation.instrumentor import BaseInstrumentor @@ -260,7 +269,11 @@ def response_hook(span: Span, status: str, response_headers: List): from opentelemetry.metrics import get_meter from opentelemetry.semconv.metrics import MetricInstruments from opentelemetry.semconv.trace import SpanAttributes -from opentelemetry.util.http import get_excluded_urls, parse_excluded_urls +from opentelemetry.util.http import ( + get_excluded_urls, + parse_excluded_urls, + sanitize_method, +) _logger = getLogger(__name__) @@ -286,8 +299,13 @@ def _request_ctx_ref() -> weakref.ReferenceType: def get_default_span_name(): + method = sanitize_method( + flask.request.environ.get("REQUEST_METHOD", "").strip() + ) + if method == "_OTHER": + method = "HTTP" try: - span_name = flask.request.url_rule.rule + span_name = f"{method} {flask.request.url_rule.rule}" except AttributeError: span_name = otel_wsgi.get_default_span_name(flask.request.environ) return span_name @@ -296,9 +314,11 @@ def get_default_span_name(): def _rewrapped_app( wsgi_app, active_requests_counter, - duration_histogram, + duration_histogram_old=None, response_hook=None, excluded_urls=None, + sem_conv_opt_in_mode=_HTTPStabilityMode.DEFAULT, + duration_histogram_new=None, ): def _wrapped_app(wrapped_app_environ, start_response): # We want to measure the time for route matching, etc. @@ -307,11 +327,16 @@ 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._parse_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): @@ -330,13 +355,12 @@ 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 @@ -357,8 +381,21 @@ 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_s = default_timer() - start + if duration_histogram_old: + duration_attrs_old = otel_wsgi._parse_duration_attrs( + attributes, _HTTPStabilityMode.DEFAULT + ) + duration_histogram_old.record( + max(round(duration_s * 1000), 0), duration_attrs_old + ) + if duration_histogram_new: + duration_attrs_new = otel_wsgi._parse_duration_attrs( + attributes, _HTTPStabilityMode.HTTP + ) + duration_histogram_new.record( + max(duration_s, 0), duration_attrs_new + ) active_requests_counter.add(-1, active_requests_count_attrs) return result @@ -371,6 +408,7 @@ def _wrapped_before_request( excluded_urls=None, enable_commenter=True, commenter_options=None, + sem_conv_opt_in_mode=_HTTPStabilityMode.DEFAULT, ): def _before_request(): if excluded_urls and excluded_urls.url_disabled(flask.request.url): @@ -379,7 +417,8 @@ 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=sem_conv_opt_in_mode, ) if flask.request.url_rule: # For 404 that result from no route found, etc, we @@ -490,6 +529,7 @@ class _InstrumentedFlask(flask.Flask): _enable_commenter = True _commenter_options = None _meter_provider = None + _sem_conv_opt_in_mode = _HTTPStabilityMode.DEFAULT def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -503,11 +543,20 @@ def __init__(self, *args, **kwargs): _InstrumentedFlask._meter_provider, schema_url="https://opentelemetry.io/schemas/1.11.0", ) - duration_histogram = meter.create_histogram( - name=MetricInstruments.HTTP_SERVER_DURATION, - unit="ms", - description="Duration of HTTP client requests.", - ) + duration_histogram_old = None + if _report_old(_InstrumentedFlask._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(_InstrumentedFlask._sem_conv_opt_in_mode): + duration_histogram_new = meter.create_histogram( + name=_METRIC_ATTRIBUTES_SERVER_DURATION_NAME, + 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", @@ -517,9 +566,11 @@ def __init__(self, *args, **kwargs): self.wsgi_app = _rewrapped_app( self.wsgi_app, active_requests_counter, - duration_histogram, + duration_histogram_old, _InstrumentedFlask._response_hook, excluded_urls=_InstrumentedFlask._excluded_urls, + sem_conv_opt_in_mode=_InstrumentedFlask._sem_conv_opt_in_mode, + duration_histogram_new=duration_histogram_new, ) tracer = trace.get_tracer( @@ -535,6 +586,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=_InstrumentedFlask._sem_conv_opt_in_mode, ) self._before_request = _before_request self.before_request(_before_request) @@ -578,11 +630,19 @@ def _instrument(self, **kwargs): _InstrumentedFlask._commenter_options = commenter_options meter_provider = kwargs.get("meter_provider") _InstrumentedFlask._meter_provider = meter_provider + + sem_conv_opt_in_mode = _OpenTelemetrySemanticConventionStability._get_opentelemetry_stability_opt_in_mode( + _OpenTelemetryStabilitySignalType.HTTP, + ) + + _InstrumentedFlask._sem_conv_opt_in_mode = sem_conv_opt_in_mode + flask.Flask = _InstrumentedFlask def _uninstrument(self, **kwargs): flask.Flask = self._original_flask + # pylint: disable=too-many-locals @staticmethod def instrument_app( app, @@ -598,6 +658,11 @@ def instrument_app( app._is_instrumented_by_opentelemetry = False if not app._is_instrumented_by_opentelemetry: + # initialize semantic conventions opt-in if needed + _OpenTelemetrySemanticConventionStability._initialize() + sem_conv_opt_in_mode = _OpenTelemetrySemanticConventionStability._get_opentelemetry_stability_opt_in_mode( + _OpenTelemetryStabilitySignalType.HTTP, + ) excluded_urls = ( parse_excluded_urls(excluded_urls) if excluded_urls is not None @@ -607,33 +672,44 @@ def instrument_app( __name__, __version__, meter_provider, - schema_url="https://opentelemetry.io/schemas/1.11.0", - ) - duration_histogram = meter.create_histogram( - name=MetricInstruments.HTTP_SERVER_DURATION, - unit="ms", - description="Duration of HTTP client requests.", + 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=_METRIC_ATTRIBUTES_SERVER_DURATION_NAME, + 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", - description="measures the number of concurrent HTTP requests that are currently in-flight", + unit="{request}", + description="Number of active HTTP server requests.", ) app._original_wsgi_app = app.wsgi_app app.wsgi_app = _rewrapped_app( app.wsgi_app, active_requests_counter, - duration_histogram, - response_hook, + duration_histogram_old, + response_hook=response_hook, excluded_urls=excluded_urls, + sem_conv_opt_in_mode=sem_conv_opt_in_mode, + duration_histogram_new=duration_histogram_new, ) tracer = trace.get_tracer( __name__, __version__, tracer_provider, - schema_url="https://opentelemetry.io/schemas/1.11.0", + schema_url=_get_schema_url(sem_conv_opt_in_mode), ) _before_request = _wrapped_before_request( @@ -644,6 +720,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/src/opentelemetry/instrumentation/flask/package.py b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/package.py index d83adbede0..150ca0ca9e 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/package.py +++ b/instrumentation/opentelemetry-instrumentation-flask/src/opentelemetry/instrumentation/flask/package.py @@ -16,3 +16,5 @@ _instruments = ("flask >= 1.0",) _supports_metrics = True + +_semconv_status = "migration" 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 82ca88460c..d30a100b0e 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +# pylint: disable=too-many-lines from timeit import default_timer from unittest.mock import Mock, patch @@ -19,7 +20,12 @@ from opentelemetry import trace from opentelemetry.instrumentation._semconv import ( + _SPAN_ATTRIBUTES_ERROR_TYPE, + OTEL_SEMCONV_STABILITY_OPT_IN, + _OpenTelemetrySemanticConventionStability, + _server_active_requests_count_attrs_new, _server_active_requests_count_attrs_old, + _server_duration_attrs_new, _server_duration_attrs_old, ) from opentelemetry.instrumentation.flask import FlaskInstrumentor @@ -65,26 +71,71 @@ def expected_attributes(override_attributes): return default_attributes -_expected_metric_names = [ +def expected_attributes_new(override_attributes): + default_attributes = { + SpanAttributes.HTTP_REQUEST_METHOD: "GET", + SpanAttributes.SERVER_PORT: 80, + SpanAttributes.SERVER_ADDRESS: "localhost", + SpanAttributes.URL_PATH: "/hello/123", + 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", ] -_recommended_attrs = { +_expected_metric_names_new = [ + "http.server.active_requests", + "http.server.request.duration", +] +_recommended_metrics_attrs_old = { "http.server.active_requests": _server_active_requests_count_attrs_old, "http.server.duration": _server_duration_attrs_old, } +_recommended_metrics_attrs_new = { + "http.server.active_requests": _server_active_requests_count_attrs_new, + "http.server.request.duration": _server_duration_attrs_new, +} +_server_active_requests_count_attrs_both = ( + _server_active_requests_count_attrs_old +) +_server_active_requests_count_attrs_both.extend( + _server_active_requests_count_attrs_new +) +_recommended_metrics_attrs_both = { + "http.server.active_requests": _server_active_requests_count_attrs_both, + "http.server.duration": _server_duration_attrs_old, + "http.server.request.duration": _server_duration_attrs_new, +} +# pylint: disable=too-many-public-methods class TestProgrammatic(InstrumentationTest, WsgiTestBase): def setUp(self): super().setUp() + test_name = "" + if hasattr(self, "_testMethodName"): + test_name = self._testMethodName + sem_conv_mode = "default" + if "new_semconv" in test_name: + sem_conv_mode = "http" + elif "both_semconv" in test_name: + sem_conv_mode = "http/dup" + self.env_patch = patch.dict( "os.environ", { - "OTEL_PYTHON_FLASK_EXCLUDED_URLS": "http://localhost/env_excluded_arg/123,env_excluded_noarg" + "OTEL_PYTHON_FLASK_EXCLUDED_URLS": "http://localhost/env_excluded_arg/123,env_excluded_noarg", + OTEL_SEMCONV_STABILITY_OPT_IN: sem_conv_mode, }, ) + _OpenTelemetrySemanticConventionStability._initialized = False self.env_patch.start() self.exclude_patch = patch( @@ -170,7 +221,45 @@ 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) + + def test_simple_new_semconv(self): + expected_attrs = expected_attributes_new( + { + SpanAttributes.HTTP_ROUTE: "/hello/", + SpanAttributes.URL_SCHEME: "http", + } + ) + self.client.get("/hello/123") + + 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_simple_both_semconv(self): + expected_attrs = expected_attributes( + { + SpanAttributes.HTTP_TARGET: "/hello/123", + SpanAttributes.HTTP_ROUTE: "/hello/", + } + ) + expected_attrs.update( + expected_attributes_new( + { + SpanAttributes.HTTP_ROUTE: "/hello/", + SpanAttributes.URL_SCHEME: "http", + } + ) + ) + self.client.get("/hello/123") + + 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) @@ -220,6 +309,53 @@ def test_404(self): self.assertEqual(span_list[0].kind, trace.SpanKind.SERVER) self.assertEqual(span_list[0].attributes, expected_attrs) + def test_404_new_semconv(self): + expected_attrs = expected_attributes_new( + { + SpanAttributes.HTTP_REQUEST_METHOD: "POST", + SpanAttributes.HTTP_RESPONSE_STATUS_CODE: 404, + SpanAttributes.URL_PATH: "/bye", + SpanAttributes.URL_SCHEME: "http", + } + ) + + 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 /bye") + self.assertEqual(span_list[0].kind, trace.SpanKind.SERVER) + self.assertEqual(span_list[0].attributes, expected_attrs) + + def test_404_both_semconv(self): + expected_attrs = expected_attributes( + { + SpanAttributes.HTTP_METHOD: "POST", + SpanAttributes.HTTP_TARGET: "/bye", + SpanAttributes.HTTP_STATUS_CODE: 404, + } + ) + expected_attrs.update( + expected_attributes_new( + { + SpanAttributes.HTTP_REQUEST_METHOD: "POST", + SpanAttributes.HTTP_RESPONSE_STATUS_CODE: 404, + SpanAttributes.URL_PATH: "/bye", + SpanAttributes.URL_SCHEME: "http", + } + ) + ) + + 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 /bye") + 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( { @@ -233,7 +369,53 @@ 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) + + def test_internal_error_new_semconv(self): + expected_attrs = expected_attributes_new( + { + SpanAttributes.URL_PATH: "/hello/500", + SpanAttributes.HTTP_ROUTE: "/hello/", + SpanAttributes.HTTP_RESPONSE_STATUS_CODE: 500, + _SPAN_ATTRIBUTES_ERROR_TYPE: "500", + SpanAttributes.URL_SCHEME: "http", + } + ) + 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_internal_error_both_semconv(self): + expected_attrs = expected_attributes( + { + SpanAttributes.HTTP_TARGET: "/hello/500", + SpanAttributes.HTTP_ROUTE: "/hello/", + SpanAttributes.HTTP_STATUS_CODE: 500, + } + ) + expected_attrs.update( + expected_attributes_new( + { + SpanAttributes.URL_PATH: "/hello/500", + SpanAttributes.HTTP_RESPONSE_STATUS_CODE: 500, + _SPAN_ATTRIBUTES_ERROR_TYPE: "500", + SpanAttributes.URL_SCHEME: "http", + } + ) + ) + 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) @@ -291,7 +473,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 +487,42 @@ def test_flask_metrics(self): number_data_point_seen = True for attr in point.attributes: self.assertIn( - attr, _recommended_attrs[metric.name] + attr, + _recommended_metrics_attrs_old[metric.name], + ) + self.assertTrue(number_data_point_seen and histogram_data_point_seen) + + def test_flask_metrics_new_semconv(self): + start = default_timer() + self.client.get("/hello/123") + self.client.get("/hello/321") + self.client.get("/hello/756") + duration = max(round((default_timer() - start) * 1000), 0) + 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, + _recommended_metrics_attrs_new[metric.name], ) self.assertTrue(number_data_point_seen and histogram_data_point_seen) @@ -375,6 +592,23 @@ def test_basic_metric_success(self): expected_requests_count_attributes, ) + def test_basic_metric_success_new_semconv(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 = { @@ -401,32 +635,42 @@ def test_basic_metric_nonstandard_http_method_success(self): expected_requests_count_attributes, ) + def test_basic_metric_nonstandard_http_method_success_new_semconv(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): + def test_basic_metric_nonstandard_http_method_allowed_success_new_semconv( + self, + ): self.client.open("/hello/756", method="NONSTANDARD") expected_duration_attributes = { - "http.method": "NONSTANDARD", - "http.host": "localhost", - "http.scheme": "http", - "http.flavor": "1.1", - "http.server_name": "localhost", - "net.host.port": 80, - "http.status_code": 405, - "net.host.name": "localhost", + "http.request.method": "NONSTANDARD", + "url.scheme": "http", + "network.protocol.version": "1.1", + "http.response.status_code": 405, } expected_requests_count_attributes = { - "http.method": "NONSTANDARD", - "http.host": "localhost", - "http.scheme": "http", - "http.flavor": "1.1", - "http.server_name": "localhost", - "net.host.name": "localhost", - "net.host.port": 80, + "http.request.method": "NONSTANDARD", + "url.scheme": "http", } self._assert_basic_metric( expected_duration_attributes, diff --git a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py index f468ff87ff..12797d6f5e 100644 --- a/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-requests/src/opentelemetry/instrumentation/requests/__init__.py @@ -172,9 +172,11 @@ def get_or_create_headers(): try: parsed_url = urlparse(url) if parsed_url.scheme: - _set_http_scheme( - metric_labels, parsed_url.scheme, sem_conv_opt_in_mode - ) + if _report_old(sem_conv_opt_in_mode): + # TODO: Support opt-in for url.scheme in new semconv + _set_http_scheme( + metric_labels, parsed_url.scheme, sem_conv_opt_in_mode + ) if parsed_url.hostname: _set_http_host( metric_labels, parsed_url.hostname, sem_conv_opt_in_mode diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/package.py b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/package.py index 942f175da1..1bb8350a06 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/package.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/package.py @@ -16,3 +16,5 @@ _instruments = tuple() _supports_metrics = True + +_semconv_status = "migration" diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py index b55ac6808f..2b26cbb5f9 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py @@ -241,6 +241,7 @@ def validate_response( SpanAttributes.SERVER_ADDRESS: "127.0.0.1", SpanAttributes.NETWORK_PROTOCOL_VERSION: "1.0", SpanAttributes.HTTP_RESPONSE_STATUS_CODE: 200, + SpanAttributes.URL_SCHEME: "http", } if old_sem_conv: expected_attributes.update(expected_attributes_old) @@ -522,6 +523,7 @@ def test_request_attributes_new_semconv(self): SpanAttributes.NETWORK_PROTOCOL_VERSION: "1.0", SpanAttributes.URL_PATH: "/", SpanAttributes.URL_QUERY: "foo=bar", + SpanAttributes.URL_SCHEME: "http", }, ) diff --git a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py index 8f236b5479..efe3c75f70 100644 --- a/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py +++ b/opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py @@ -65,8 +65,7 @@ SpanAttributes.HTTP_RESPONSE_STATUS_CODE, SpanAttributes.HTTP_ROUTE, SpanAttributes.NETWORK_PROTOCOL_VERSION, - # TODO: Support opt-in for scheme in new semconv - # SpanAttributes.URL_SCHEME, + SpanAttributes.URL_SCHEME, ] _server_active_requests_count_attrs_old = [ @@ -234,9 +233,8 @@ def _set_http_url(result, url, sem_conv_opt_in_mode): def _set_http_scheme(result, scheme, sem_conv_opt_in_mode): if _report_old(sem_conv_opt_in_mode): set_string_attribute(result, SpanAttributes.HTTP_SCHEME, scheme) - # TODO: Support opt-in for scheme in new semconv - # if _report_new(sem_conv_opt_in_mode): - # set_string_attribute(result, SpanAttributes.URL_SCHEME, scheme) + if _report_new(sem_conv_opt_in_mode): + set_string_attribute(result, SpanAttributes.URL_SCHEME, scheme) def _set_http_host(result, host, sem_conv_opt_in_mode):