From 6ae8d5995262fd25e19c14b8ad4acb51fe51826e Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Tue, 14 Nov 2023 09:13:36 -0600 Subject: [PATCH] Move all key access to a single spot and deduplicate context managers --- .../instrumentation/botocore/__init__.py | 5 +- .../tests/test_botocore_instrumentation.py | 4 +- .../instrumentation/cassandra/__init__.py | 11 +- .../tests/test_cassandra_integration.py | 24 +--- .../instrumentation/celery/__init__.py | 3 +- .../instrumentation/celery/utils.py | 2 +- .../confluent_kafka/__init__.py | 2 +- .../instrumentation/confluent_kafka/utils.py | 2 +- .../tests/test_instrumentation.py | 14 +- .../tests/test_programmatic.py | 7 +- .../instrumentation/httpx/__init__.py | 9 +- .../tests/test_system_metrics.py | 13 +- .../instrumentation/urllib/__init__.py | 5 +- .../instrumentation/wsgi/__init__.py | 4 +- .../tests/test_wsgi_middleware.py | 14 +- .../resource/detector/azure/app_service.py | 39 ++---- .../resource/detector/azure/vm.py | 25 ++-- .../tests/test_app_service.py | 131 +++++++----------- .../tests/test_vm.py | 16 +-- .../src/opentelemetry/util/http/__init__.py | 20 +-- .../tests/test_sanitize_method.py | 3 +- 21 files changed, 122 insertions(+), 231 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py b/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py index 1581714c85..6a4f337ec5 100644 --- a/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-botocore/src/opentelemetry/instrumentation/botocore/__init__.py @@ -210,10 +210,9 @@ def _patched_api_call(self, original_func, instance, args, kwargs): raise else: _apply_response_attributes(span, result) - _safe_invoke(extension.on_success, span, result) - finally: - _safe_invoke(extension.after_service_call) + _safe_invoke(extension.on_success, span, result) finally: + _safe_invoke(extension.after_service_call) self._call_response_hook(span, call_context, result) return result diff --git a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py b/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py index 4d8b998c62..993faa97a4 100644 --- a/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py @@ -28,7 +28,7 @@ from opentelemetry import trace as trace_api from opentelemetry.instrumentation.botocore import BotocoreInstrumentor -from opentelemetry.instrumentation.utils import suppress_instrumentation +from opentelemetry.instrumentation.utils import suppress_http_instrumentation, suppress_instrumentation from opentelemetry.propagate import get_global_textmap, set_global_textmap from opentelemetry.propagators.aws.aws_xray_propagator import TRACE_HEADER_KEY from opentelemetry.semconv.trace import SpanAttributes @@ -342,7 +342,7 @@ def test_suppress_instrumentation_xray_client(self): @mock_xray def test_suppress_http_instrumentation_xray_client(self): - with suppress_instrumentation(): + with suppress_http_instrumentation(): xray_client.put_trace_segments(TraceSegmentDocuments=["str1"]) xray_client.put_trace_segments(TraceSegmentDocuments=["str2"]) self.assertEqual(2, len(self.get_finished_spans())) diff --git a/instrumentation/opentelemetry-instrumentation-cassandra/src/opentelemetry/instrumentation/cassandra/__init__.py b/instrumentation/opentelemetry-instrumentation-cassandra/src/opentelemetry/instrumentation/cassandra/__init__.py index 2a9239f152..75d2bcfe38 100644 --- a/instrumentation/opentelemetry-instrumentation-cassandra/src/opentelemetry/instrumentation/cassandra/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-cassandra/src/opentelemetry/instrumentation/cassandra/__init__.py @@ -43,9 +43,9 @@ from wrapt import wrap_function_wrapper from opentelemetry import trace +from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.cassandra.package import _instruments from opentelemetry.instrumentation.cassandra.version import __version__ -from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.utils import unwrap from opentelemetry.semconv.trace import SpanAttributes @@ -70,10 +70,7 @@ def _traced_execute_async(func, instance, args, kwargs): if span.is_recording(): span.set_attribute(SpanAttributes.DB_NAME, instance.keyspace) span.set_attribute(SpanAttributes.DB_SYSTEM, "cassandra") - span.set_attribute( - SpanAttributes.NET_PEER_NAME, - instance.cluster.contact_points, - ) + span.set_attribute(SpanAttributes.NET_PEER_NAME, instance.cluster.contact_points) if include_db_statement: query = args[0] @@ -82,9 +79,7 @@ def _traced_execute_async(func, instance, args, kwargs): response = func(*args, **kwargs) return response - wrap_function_wrapper( - "cassandra.cluster", "Session.execute_async", _traced_execute_async - ) + wrap_function_wrapper("cassandra.cluster", "Session.execute_async", _traced_execute_async) class CassandraInstrumentor(BaseInstrumentor): diff --git a/instrumentation/opentelemetry-instrumentation-cassandra/tests/test_cassandra_integration.py b/instrumentation/opentelemetry-instrumentation-cassandra/tests/test_cassandra_integration.py index ed488ab07f..6977e1b2a2 100644 --- a/instrumentation/opentelemetry-instrumentation-cassandra/tests/test_cassandra_integration.py +++ b/instrumentation/opentelemetry-instrumentation-cassandra/tests/test_cassandra_integration.py @@ -46,25 +46,15 @@ def tearDown(self): def test_instrument_uninstrument(self): instrumentation = CassandraInstrumentor() instrumentation.instrument() - self.assertTrue( - isinstance( - cassandra.cluster.Session.execute_async, BoundFunctionWrapper - ) - ) + self.assertTrue(isinstance(cassandra.cluster.Session.execute_async, BoundFunctionWrapper)) instrumentation.uninstrument() - self.assertFalse( - isinstance( - cassandra.cluster.Session.execute_async, BoundFunctionWrapper - ) - ) + self.assertFalse(isinstance(cassandra.cluster.Session.execute_async, BoundFunctionWrapper)) @mock.patch("cassandra.cluster.Cluster.connect") @mock.patch("cassandra.cluster.Session.__init__") @mock.patch("cassandra.cluster.Session._create_response_future") - def test_instrumentor( - self, mock_create_response_future, mock_session_init, mock_connect - ): + def test_instrumentor(self, mock_create_response_future, mock_session_init, mock_connect): mock_create_response_future.return_value = mock.Mock() mock_session_init.return_value = None mock_connect.return_value = cassandra.cluster.Session() @@ -95,9 +85,7 @@ def test_instrumentor( @mock.patch("cassandra.cluster.Cluster.connect") @mock.patch("cassandra.cluster.Session.__init__") @mock.patch("cassandra.cluster.Session._create_response_future") - def test_custom_tracer_provider( - self, mock_create_response_future, mock_session_init, mock_connect - ): + def test_custom_tracer_provider(self, mock_create_response_future, mock_session_init, mock_connect): mock_create_response_future.return_value = mock.Mock() mock_session_init.return_value = None mock_connect.return_value = cassandra.cluster.Session() @@ -119,9 +107,7 @@ def test_custom_tracer_provider( @mock.patch("cassandra.cluster.Cluster.connect") @mock.patch("cassandra.cluster.Session.__init__") @mock.patch("cassandra.cluster.Session._create_response_future") - def test_instrument_connection_no_op_tracer_provider( - self, mock_create_response_future, mock_session_init, mock_connect - ): + def test_instrument_connection_no_op_tracer_provider(self, mock_create_response_future, mock_session_init, mock_connect): mock_create_response_future.return_value = mock.Mock() mock_session_init.return_value = None mock_connect.return_value = cassandra.cluster.Session() diff --git a/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py b/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py index 94cac68b70..8baddcca94 100644 --- a/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/__init__.py @@ -63,7 +63,6 @@ def add(x, y): from timeit import default_timer from typing import Collection, Iterable -from billiard import VERSION from billiard.einfo import ExceptionInfo from celery import signals # pylint: disable=no-name-in-module @@ -77,6 +76,8 @@ def add(x, y): from opentelemetry.propagators.textmap import Getter from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace.status import Status, StatusCode +from billiard import VERSION + if VERSION >= (4, 0, 1): from billiard.einfo import ExceptionWithTraceback diff --git a/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/utils.py b/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/utils.py index 05f791e41a..f92c5e03c8 100644 --- a/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/utils.py +++ b/instrumentation/opentelemetry-instrumentation-celery/src/opentelemetry/instrumentation/celery/utils.py @@ -14,8 +14,8 @@ import logging -from billiard import VERSION from celery import registry # pylint: disable=no-name-in-module +from billiard import VERSION from opentelemetry.semconv.trace import SpanAttributes diff --git a/instrumentation/opentelemetry-instrumentation-confluent-kafka/src/opentelemetry/instrumentation/confluent_kafka/__init__.py b/instrumentation/opentelemetry-instrumentation-confluent-kafka/src/opentelemetry/instrumentation/confluent_kafka/__init__.py index 7a3804fcdc..45a16fcffb 100644 --- a/instrumentation/opentelemetry-instrumentation-confluent-kafka/src/opentelemetry/instrumentation/confluent_kafka/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-confluent-kafka/src/opentelemetry/instrumentation/confluent_kafka/__init__.py @@ -112,8 +112,8 @@ def instrument_consumer(consumer: Consumer, tracer_provider=None) from .package import _instruments from .utils import ( KafkaPropertiesExtractor, - _create_new_consume_span, _end_current_consume_span, + _create_new_consume_span, _enrich_span, _get_span_name, _kafka_getter, diff --git a/instrumentation/opentelemetry-instrumentation-confluent-kafka/src/opentelemetry/instrumentation/confluent_kafka/utils.py b/instrumentation/opentelemetry-instrumentation-confluent-kafka/src/opentelemetry/instrumentation/confluent_kafka/utils.py index 4769f2a88f..2029960703 100644 --- a/instrumentation/opentelemetry-instrumentation-confluent-kafka/src/opentelemetry/instrumentation/confluent_kafka/utils.py +++ b/instrumentation/opentelemetry-instrumentation-confluent-kafka/src/opentelemetry/instrumentation/confluent_kafka/utils.py @@ -2,13 +2,13 @@ from typing import List, Optional from opentelemetry import context, propagate +from opentelemetry.trace import SpanKind, Link from opentelemetry.propagators import textmap from opentelemetry.semconv.trace import ( MessagingDestinationKindValues, MessagingOperationValues, SpanAttributes, ) -from opentelemetry.trace import Link, SpanKind _LOG = getLogger(__name__) diff --git a/instrumentation/opentelemetry-instrumentation-confluent-kafka/tests/test_instrumentation.py b/instrumentation/opentelemetry-instrumentation-confluent-kafka/tests/test_instrumentation.py index 21d5bd6f83..d7ac343dbf 100644 --- a/instrumentation/opentelemetry-instrumentation-confluent-kafka/tests/test_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-confluent-kafka/tests/test_instrumentation.py @@ -14,6 +14,13 @@ # pylint: disable=no-name-in-module +from opentelemetry.semconv.trace import ( + SpanAttributes, + MessagingDestinationKindValues, +) +from opentelemetry.test.test_base import TestBase +from .utils import MockConsumer, MockedMessage + from confluent_kafka import Consumer, Producer from opentelemetry.instrumentation.confluent_kafka import ( @@ -25,13 +32,6 @@ KafkaContextGetter, KafkaContextSetter, ) -from opentelemetry.semconv.trace import ( - MessagingDestinationKindValues, - SpanAttributes, -) -from opentelemetry.test.test_base import TestBase - -from .utils import MockConsumer, MockedMessage class TestConfluentKafka(TestBase): diff --git a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py index 3bd2f74ba8..bf641aaed4 100644 --- a/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py +++ b/instrumentation/opentelemetry-instrumentation-flask/tests/test_programmatic.py @@ -40,6 +40,7 @@ OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SANITIZE_FIELDS, OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST, OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_RESPONSE, + OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_RESPONSE, OTEL_PYTHON_INSTRUMENTATION_HTTP_CAPTURE_ALL_METHODS, get_excluded_urls, ) @@ -327,9 +328,7 @@ def test_flask_metric_values(self): if isinstance(point, NumberDataPoint): self.assertEqual(point.value, 0) - def _assert_basic_metric( - self, expected_duration_attributes, expected_requests_count_attributes - ): + 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: @@ -395,7 +394,7 @@ def test_basic_metric_nonstandard_http_method_success(self): ) @patch.dict( - "os.environ", + "os.environ", { OTEL_PYTHON_INSTRUMENTATION_HTTP_CAPTURE_ALL_METHODS: "1", }, diff --git a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py index 09639b54bc..f5d34b3c40 100644 --- a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py @@ -170,10 +170,7 @@ def response_hook(span, request, response): from opentelemetry.instrumentation.httpx.package import _instruments from opentelemetry.instrumentation.httpx.version import __version__ from opentelemetry.instrumentation.instrumentor import BaseInstrumentor -from opentelemetry.instrumentation.utils import ( - http_status_to_status_code, - is_http_instrumentation_enabled, -) +from opentelemetry.instrumentation.utils import http_status_to_status_code from opentelemetry.propagate import inject from opentelemetry.semconv.trace import SpanAttributes from opentelemetry.trace import SpanKind, TracerProvider, get_tracer @@ -319,7 +316,7 @@ def handle_request( httpx.Response, ]: """Add request info to span.""" - if not is_http_instrumentation_enabled(): + if context.get_value("suppress_instrumentation"): return self._transport.handle_request(*args, **kwargs) method, url, headers, stream, extensions = _extract_parameters( @@ -412,7 +409,7 @@ async def handle_async_request( httpx.Response, ]: """Add request info to span.""" - if not is_http_instrumentation_enabled(): + if context.get_value("suppress_instrumentation"): return await self._transport.handle_async_request(*args, **kwargs) method, url, headers, stream, extensions = _extract_parameters( diff --git a/instrumentation/opentelemetry-instrumentation-system-metrics/tests/test_system_metrics.py b/instrumentation/opentelemetry-instrumentation-system-metrics/tests/test_system_metrics.py index c85e7199ac..e28c437009 100644 --- a/instrumentation/opentelemetry-instrumentation-system-metrics/tests/test_system_metrics.py +++ b/instrumentation/opentelemetry-instrumentation-system-metrics/tests/test_system_metrics.py @@ -18,13 +18,14 @@ from platform import python_implementation from unittest import mock -from opentelemetry.instrumentation.system_metrics import ( - SystemMetricsInstrumentor, -) from opentelemetry.sdk.metrics import MeterProvider from opentelemetry.sdk.metrics.export import InMemoryMetricReader from opentelemetry.test.test_base import TestBase +from opentelemetry.instrumentation.system_metrics import ( + SystemMetricsInstrumentor, +) + def _mock_netconnection(): NetConnection = namedtuple( @@ -170,9 +171,9 @@ def _assert_metrics(self, observer_name, reader, expected): for data_point in metric.data.data_points: for expect in expected: if ( - dict(data_point.attributes) - == expect.attributes - and metric.name == observer_name + dict(data_point.attributes) + == expect.attributes + and metric.name == observer_name ): self.assertEqual( data_point.value, diff --git a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py index c5025371b7..05d7425a15 100644 --- a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py @@ -85,14 +85,13 @@ def response_hook(span, request_obj, response) Request, ) -from opentelemetry import context from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.urllib.package import _instruments from opentelemetry.instrumentation.urllib.version import __version__ from opentelemetry.instrumentation.utils import ( - http_status_to_status_code, is_http_instrumentation_enabled, suppress_http_instrumentation, + http_status_to_status_code, ) from opentelemetry.metrics import Histogram, get_meter from opentelemetry.propagate import inject @@ -233,8 +232,8 @@ def _instrumented_open_call( inject(headers) with suppress_http_instrumentation(): + start_time = default_timer() try: - start_time = default_timer() result = call_wrapped() # *** PROCEED except Exception as exc: # pylint: disable=W0703 exception = exc 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 87c73cc737..36fe69daec 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/src/opentelemetry/instrumentation/wsgi/__init__.py @@ -302,9 +302,7 @@ def collect_request_attributes(environ): """ result = { - SpanAttributes.HTTP_METHOD: sanitize_method( - environ.get("REQUEST_METHOD") - ), + 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"), } diff --git a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py index bc78a787ca..6aef096218 100644 --- a/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py +++ b/instrumentation/opentelemetry-instrumentation-wsgi/tests/test_wsgi_middleware.py @@ -286,26 +286,22 @@ def test_wsgi_metrics(self): self.assertTrue(number_data_point_seen and histogram_data_point_seen) def test_nonstandard_http_method(self): - self.environ["REQUEST_METHOD"] = "NONSTANDARD" + 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="UNKNOWN /", http_method="UNKNOWN") @mock.patch.dict( - "os.environ", + "os.environ", { OTEL_PYTHON_INSTRUMENTATION_HTTP_CAPTURE_ALL_METHODS: "1", }, ) def test_nonstandard_http_method_allowed(self): - self.environ["REQUEST_METHOD"] = "NONSTANDARD" + 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.""" diff --git a/resource/opentelemetry-resource-detector-azure/src/opentelemetry/resource/detector/azure/app_service.py b/resource/opentelemetry-resource-detector-azure/src/opentelemetry/resource/detector/azure/app_service.py index 50a2f41167..823aac30fd 100644 --- a/resource/opentelemetry-resource-detector-azure/src/opentelemetry/resource/detector/azure/app_service.py +++ b/resource/opentelemetry-resource-detector-azure/src/opentelemetry/resource/detector/azure/app_service.py @@ -14,12 +14,8 @@ from os import environ -from opentelemetry.sdk.resources import Resource, ResourceDetector -from opentelemetry.semconv.resource import ( - CloudPlatformValues, - CloudProviderValues, - ResourceAttributes, -) +from opentelemetry.sdk.resources import ResourceDetector, Resource +from opentelemetry.semconv.resource import ResourceAttributes, CloudPlatformValues, CloudProviderValues _AZURE_APP_SERVICE_STAMP_RESOURCE_ATTRIBUTE = "azure.app.service.stamp" _REGION_NAME = "REGION_NAME" @@ -40,49 +36,38 @@ _AZURE_APP_SERVICE_STAMP_RESOURCE_ATTRIBUTE: _WEBSITE_HOME_STAMPNAME, } - class AzureAppServiceResourceDetector(ResourceDetector): def detect(self) -> Resource: attributes = {} website_site_name = environ.get(_WEBSITE_SITE_NAME) if website_site_name: attributes[ResourceAttributes.SERVICE_NAME] = website_site_name - attributes[ - ResourceAttributes.CLOUD_PROVIDER - ] = CloudProviderValues.AZURE.value - attributes[ - ResourceAttributes.CLOUD_PLATFORM - ] = CloudPlatformValues.AZURE_APP_SERVICE.value + attributes[ResourceAttributes.CLOUD_PROVIDER] = CloudProviderValues.AZURE.value + attributes[ResourceAttributes.CLOUD_PLATFORM] = CloudPlatformValues.AZURE_APP_SERVICE.value azure_resource_uri = _get_azure_resource_uri(website_site_name) if azure_resource_uri: - attributes[ - ResourceAttributes.CLOUD_RESOURCE_ID - ] = azure_resource_uri - for key, env_var in _APP_SERVICE_ATTRIBUTE_ENV_VARS.items(): + attributes[ResourceAttributes.CLOUD_RESOURCE_ID] = azure_resource_uri + for (key, env_var) in _APP_SERVICE_ATTRIBUTE_ENV_VARS.items(): value = environ.get(env_var) if value: attributes[key] = value return Resource(attributes) - def _get_azure_resource_uri(website_site_name): website_resource_group = environ.get(_WEBSITE_RESOURCE_GROUP) website_owner_name = environ.get(_WEBSITE_OWNER_NAME) subscription_id = website_owner_name - if website_owner_name and "+" in website_owner_name: - subscription_id = website_owner_name[0 : website_owner_name.index("+")] + if website_owner_name and '+' in website_owner_name: + subscription_id = website_owner_name[0:website_owner_name.index('+')] if not (website_resource_group and subscription_id): return None - return ( - "/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Web/sites/%s" - % ( - subscription_id, - website_resource_group, - website_site_name, - ) + return "/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Web/sites/%s" % ( + subscription_id, + website_resource_group, + website_site_name, ) diff --git a/resource/opentelemetry-resource-detector-azure/src/opentelemetry/resource/detector/azure/vm.py b/resource/opentelemetry-resource-detector-azure/src/opentelemetry/resource/detector/azure/vm.py index 70cda2a91c..11b04ebff3 100644 --- a/resource/opentelemetry-resource-detector-azure/src/opentelemetry/resource/detector/azure/vm.py +++ b/resource/opentelemetry-resource-detector-azure/src/opentelemetry/resource/detector/azure/vm.py @@ -15,16 +15,17 @@ from json import loads from logging import getLogger from os import environ -from urllib.error import URLError from urllib.request import Request, urlopen +from urllib.error import URLError -from opentelemetry.sdk.resources import Resource, ResourceDetector +from opentelemetry.sdk.resources import ResourceDetector, Resource from opentelemetry.semconv.resource import ( + ResourceAttributes, CloudPlatformValues, CloudProviderValues, - ResourceAttributes, ) + # TODO: Remove when cloud resource id is no longer missing in Resource Attributes _AZURE_VM_METADATA_ENDPOINT = "http://169.254.169.254/metadata/instance/compute?api-version=2021-12-13&format=json" _AZURE_VM_SCALE_SET_NAME_ATTRIBUTE = "azure.vm.scaleset.name" @@ -46,25 +47,17 @@ ResourceAttributes.SERVICE_INSTANCE_ID, ] - class AzureVMResourceDetector(ResourceDetector): # pylint: disable=no-self-use def detect(self) -> "Resource": attributes = {} - metadata_json = ( - _AzureVMMetadataServiceRequestor().get_azure_vm_metadata() - ) + metadata_json = _AzureVMMetadataServiceRequestor().get_azure_vm_metadata() if not metadata_json: return Resource(attributes) for attribute_key in EXPECTED_AZURE_AMS_ATTRIBUTES: - attributes[ - attribute_key - ] = _AzureVMMetadataServiceRequestor().get_attribute_from_metadata( - metadata_json, attribute_key - ) + attributes[attribute_key] = _AzureVMMetadataServiceRequestor().get_attribute_from_metadata(metadata_json, attribute_key) return Resource(attributes) - class _AzureVMMetadataServiceRequestor: def get_azure_vm_metadata(self): request = Request(_AZURE_VM_METADATA_ENDPOINT) @@ -93,10 +86,8 @@ def get_attribute_from_metadata(self, metadata_json, attribute_key): ams_value = metadata_json["location"] elif attribute_key == ResourceAttributes.CLOUD_RESOURCE_ID: ams_value = metadata_json["resourceId"] - elif ( - attribute_key == ResourceAttributes.HOST_ID - or attribute_key == ResourceAttributes.SERVICE_INSTANCE_ID - ): + elif attribute_key == ResourceAttributes.HOST_ID or \ + attribute_key == ResourceAttributes.SERVICE_INSTANCE_ID: ams_value = metadata_json["vmId"] elif attribute_key == ResourceAttributes.HOST_NAME: ams_value = metadata_json["name"] diff --git a/resource/opentelemetry-resource-detector-azure/tests/test_app_service.py b/resource/opentelemetry-resource-detector-azure/tests/test_app_service.py index 209df39134..f5d6a0dd3d 100644 --- a/resource/opentelemetry-resource-detector-azure/tests/test_app_service.py +++ b/resource/opentelemetry-resource-detector-azure/tests/test_app_service.py @@ -28,22 +28,17 @@ TEST_WEBSITE_RESOURCE_GROUP = "TEST_WEBSITE_RESOURCE_GROUP" TEST_WEBSITE_OWNER_NAME = "TEST_WEBSITE_OWNER_NAME" - class TestAzureAppServiceResourceDetector(unittest.TestCase): - @patch.dict( - "os.environ", - { - "WEBSITE_SITE_NAME": TEST_WEBSITE_SITE_NAME, - "REGION_NAME": TEST_REGION_NAME, - "WEBSITE_SLOT_NAME": TEST_WEBSITE_SLOT_NAME, - "WEBSITE_HOSTNAME": TEST_WEBSITE_HOSTNAME, - "WEBSITE_INSTANCE_ID": TEST_WEBSITE_INSTANCE_ID, - "WEBSITE_HOME_STAMPNAME": TEST_WEBSITE_HOME_STAMPNAME, - "WEBSITE_RESOURCE_GROUP": TEST_WEBSITE_RESOURCE_GROUP, - "WEBSITE_OWNER_NAME": TEST_WEBSITE_OWNER_NAME, - }, - clear=True, - ) + @patch.dict("os.environ", { + "WEBSITE_SITE_NAME": TEST_WEBSITE_SITE_NAME, + "REGION_NAME": TEST_REGION_NAME, + "WEBSITE_SLOT_NAME": TEST_WEBSITE_SLOT_NAME, + "WEBSITE_HOSTNAME": TEST_WEBSITE_HOSTNAME, + "WEBSITE_INSTANCE_ID": TEST_WEBSITE_INSTANCE_ID, + "WEBSITE_HOME_STAMPNAME": TEST_WEBSITE_HOME_STAMPNAME, + "WEBSITE_RESOURCE_GROUP": TEST_WEBSITE_RESOURCE_GROUP, + "WEBSITE_OWNER_NAME": TEST_WEBSITE_OWNER_NAME, + }, clear=True) def test_on_app_service(self): resource = AzureAppServiceResourceDetector().detect() attributes = resource.attributes @@ -51,36 +46,24 @@ def test_on_app_service(self): self.assertEqual(attributes["cloud.provider"], "azure") self.assertEqual(attributes["cloud.platform"], "azure_app_service") - self.assertEqual( - attributes["cloud.resource_id"], - f"/subscriptions/{TEST_WEBSITE_OWNER_NAME}/resourceGroups/{TEST_WEBSITE_RESOURCE_GROUP}/providers/Microsoft.Web/sites/{TEST_WEBSITE_SITE_NAME}", - ) + self.assertEqual(attributes["cloud.resource_id"], \ + f"/subscriptions/{TEST_WEBSITE_OWNER_NAME}/resourceGroups/{TEST_WEBSITE_RESOURCE_GROUP}/providers/Microsoft.Web/sites/{TEST_WEBSITE_SITE_NAME}") self.assertEqual(attributes["cloud.region"], TEST_REGION_NAME) - self.assertEqual( - attributes["deployment.environment"], TEST_WEBSITE_SLOT_NAME - ) + self.assertEqual(attributes["deployment.environment"], TEST_WEBSITE_SLOT_NAME) self.assertEqual(attributes["host.id"], TEST_WEBSITE_HOSTNAME) - self.assertEqual( - attributes["service.instance.id"], TEST_WEBSITE_INSTANCE_ID - ) - self.assertEqual( - attributes["azure.app.service.stamp"], TEST_WEBSITE_HOME_STAMPNAME - ) + self.assertEqual(attributes["service.instance.id"], TEST_WEBSITE_INSTANCE_ID) + self.assertEqual(attributes["azure.app.service.stamp"], TEST_WEBSITE_HOME_STAMPNAME) - @patch.dict( - "os.environ", - { - "WEBSITE_SITE_NAME": TEST_WEBSITE_SITE_NAME, - "REGION_NAME": TEST_REGION_NAME, - "WEBSITE_SLOT_NAME": TEST_WEBSITE_SLOT_NAME, - "WEBSITE_HOSTNAME": TEST_WEBSITE_HOSTNAME, - "WEBSITE_INSTANCE_ID": TEST_WEBSITE_INSTANCE_ID, - "WEBSITE_HOME_STAMPNAME": TEST_WEBSITE_HOME_STAMPNAME, - "WEBSITE_OWNER_NAME": TEST_WEBSITE_OWNER_NAME, - }, - clear=True, - ) + @patch.dict("os.environ", { + "WEBSITE_SITE_NAME": TEST_WEBSITE_SITE_NAME, + "REGION_NAME": TEST_REGION_NAME, + "WEBSITE_SLOT_NAME": TEST_WEBSITE_SLOT_NAME, + "WEBSITE_HOSTNAME": TEST_WEBSITE_HOSTNAME, + "WEBSITE_INSTANCE_ID": TEST_WEBSITE_INSTANCE_ID, + "WEBSITE_HOME_STAMPNAME": TEST_WEBSITE_HOME_STAMPNAME, + "WEBSITE_OWNER_NAME": TEST_WEBSITE_OWNER_NAME, + }, clear=True) def test_on_app_service_no_resource_group(self): resource = AzureAppServiceResourceDetector().detect() attributes = resource.attributes @@ -91,30 +74,20 @@ def test_on_app_service_no_resource_group(self): self.assertTrue("cloud.resource_id" not in attributes) self.assertEqual(attributes["cloud.region"], TEST_REGION_NAME) - self.assertEqual( - attributes["deployment.environment"], TEST_WEBSITE_SLOT_NAME - ) + self.assertEqual(attributes["deployment.environment"], TEST_WEBSITE_SLOT_NAME) self.assertEqual(attributes["host.id"], TEST_WEBSITE_HOSTNAME) - self.assertEqual( - attributes["service.instance.id"], TEST_WEBSITE_INSTANCE_ID - ) - self.assertEqual( - attributes["azure.app.service.stamp"], TEST_WEBSITE_HOME_STAMPNAME - ) + self.assertEqual(attributes["service.instance.id"], TEST_WEBSITE_INSTANCE_ID) + self.assertEqual(attributes["azure.app.service.stamp"], TEST_WEBSITE_HOME_STAMPNAME) - @patch.dict( - "os.environ", - { - "WEBSITE_SITE_NAME": TEST_WEBSITE_SITE_NAME, - "REGION_NAME": TEST_REGION_NAME, - "WEBSITE_SLOT_NAME": TEST_WEBSITE_SLOT_NAME, - "WEBSITE_HOSTNAME": TEST_WEBSITE_HOSTNAME, - "WEBSITE_INSTANCE_ID": TEST_WEBSITE_INSTANCE_ID, - "WEBSITE_HOME_STAMPNAME": TEST_WEBSITE_HOME_STAMPNAME, - "WEBSITE_OWNER_NAME": TEST_WEBSITE_OWNER_NAME, - }, - clear=True, - ) + @patch.dict("os.environ", { + "WEBSITE_SITE_NAME": TEST_WEBSITE_SITE_NAME, + "REGION_NAME": TEST_REGION_NAME, + "WEBSITE_SLOT_NAME": TEST_WEBSITE_SLOT_NAME, + "WEBSITE_HOSTNAME": TEST_WEBSITE_HOSTNAME, + "WEBSITE_INSTANCE_ID": TEST_WEBSITE_INSTANCE_ID, + "WEBSITE_HOME_STAMPNAME": TEST_WEBSITE_HOME_STAMPNAME, + "WEBSITE_OWNER_NAME": TEST_WEBSITE_OWNER_NAME, + }, clear=True) def test_on_app_service_no_owner(self): resource = AzureAppServiceResourceDetector().detect() attributes = resource.attributes @@ -125,29 +98,19 @@ def test_on_app_service_no_owner(self): self.assertTrue("cloud.resource_id" not in attributes) self.assertEqual(attributes["cloud.region"], TEST_REGION_NAME) - self.assertEqual( - attributes["deployment.environment"], TEST_WEBSITE_SLOT_NAME - ) + self.assertEqual(attributes["deployment.environment"], TEST_WEBSITE_SLOT_NAME) self.assertEqual(attributes["host.id"], TEST_WEBSITE_HOSTNAME) - self.assertEqual( - attributes["service.instance.id"], TEST_WEBSITE_INSTANCE_ID - ) - self.assertEqual( - attributes["azure.app.service.stamp"], TEST_WEBSITE_HOME_STAMPNAME - ) + self.assertEqual(attributes["service.instance.id"], TEST_WEBSITE_INSTANCE_ID) + self.assertEqual(attributes["azure.app.service.stamp"], TEST_WEBSITE_HOME_STAMPNAME) - @patch.dict( - "os.environ", - { - "REGION_NAME": TEST_REGION_NAME, - "WEBSITE_SLOT_NAME": TEST_WEBSITE_SLOT_NAME, - "WEBSITE_HOSTNAME": TEST_WEBSITE_HOSTNAME, - "WEBSITE_INSTANCE_ID": TEST_WEBSITE_INSTANCE_ID, - "WEBSITE_HOME_STAMPNAME": TEST_WEBSITE_HOME_STAMPNAME, - "WEBSITE_OWNER_NAME": TEST_WEBSITE_OWNER_NAME, - }, - clear=True, - ) + @patch.dict("os.environ", { + "REGION_NAME": TEST_REGION_NAME, + "WEBSITE_SLOT_NAME": TEST_WEBSITE_SLOT_NAME, + "WEBSITE_HOSTNAME": TEST_WEBSITE_HOSTNAME, + "WEBSITE_INSTANCE_ID": TEST_WEBSITE_INSTANCE_ID, + "WEBSITE_HOME_STAMPNAME": TEST_WEBSITE_HOME_STAMPNAME, + "WEBSITE_OWNER_NAME": TEST_WEBSITE_OWNER_NAME, + }, clear=True) def test_off_app_service(self): resource = AzureAppServiceResourceDetector().detect() self.assertEqual(resource.attributes, {}) diff --git a/resource/opentelemetry-resource-detector-azure/tests/test_vm.py b/resource/opentelemetry-resource-detector-azure/tests/test_vm.py index cb412eae63..0531fa02b1 100644 --- a/resource/opentelemetry-resource-detector-azure/tests/test_vm.py +++ b/resource/opentelemetry-resource-detector-azure/tests/test_vm.py @@ -12,10 +12,12 @@ # See the License for the specific language governing permissions and # limitations under the License. import unittest -from unittest.mock import Mock, patch +from unittest.mock import patch, Mock -from opentelemetry.resource.detector.azure.vm import AzureVMResourceDetector from opentelemetry.semconv.resource import ResourceAttributes +from opentelemetry.resource.detector.azure.vm import ( + AzureVMResourceDetector, +) LINUX_JSON = """ { @@ -173,7 +175,7 @@ "zone": "1" } """ -WINDOWS_JSON = """ +WINDOWS_JSON =""" { "additionalCapabilities": { "hibernationEnabled": "false" @@ -368,9 +370,7 @@ def test_linux(self, mock_urlopen): mock_open.read.return_value = LINUX_JSON attributes = AzureVMResourceDetector().detect().attributes for attribute_key in LINUX_ATTRIBUTES: - self.assertEqual( - attributes[attribute_key], LINUX_ATTRIBUTES[attribute_key] - ) + self.assertEqual(attributes[attribute_key], LINUX_ATTRIBUTES[attribute_key]) @patch("opentelemetry.resource.detector.azure.vm.urlopen") def test_windows(self, mock_urlopen): @@ -379,6 +379,4 @@ def test_windows(self, mock_urlopen): mock_open.read.return_value = WINDOWS_JSON attributes = AzureVMResourceDetector().detect().attributes for attribute_key in WINDOWS_ATTRIBUTES: - self.assertEqual( - attributes[attribute_key], WINDOWS_ATTRIBUTES[attribute_key] - ) + self.assertEqual(attributes[attribute_key], WINDOWS_ATTRIBUTES[attribute_key]) 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 054ade6d27..4f4a5d0353 100644 --- a/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py +++ b/util/opentelemetry-util-http/src/opentelemetry/util/http/__init__.py @@ -190,32 +190,16 @@ def normalise_response_header_name(header: str) -> str: key = header.lower().replace("-", "_") return f"http.response.header.{key}" - def sanitize_method(method: Optional[str]) -> Optional[str]: if method is None: return None method = method.upper() - if ( - environ.get(OTEL_PYTHON_INSTRUMENTATION_HTTP_CAPTURE_ALL_METHODS) - or + if (environ.get(OTEL_PYTHON_INSTRUMENTATION_HTTP_CAPTURE_ALL_METHODS) or # 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", - ] - ): + method in ["GET", "HEAD", "POST", "PUT", "DELETE", "CONNECT", "OPTIONS", "TRACE", "PATCH"]): return method return "UNKNOWN" - def get_custom_headers(env_var: str) -> List[str]: custom_headers = environ.get(env_var, []) if custom_headers: diff --git a/util/opentelemetry-util-http/tests/test_sanitize_method.py b/util/opentelemetry-util-http/tests/test_sanitize_method.py index b4095324a6..a488ef589e 100644 --- a/util/opentelemetry-util-http/tests/test_sanitize_method.py +++ b/util/opentelemetry-util-http/tests/test_sanitize_method.py @@ -20,7 +20,6 @@ sanitize_method, ) - class TestSanitizeMethod(unittest.TestCase): def test_standard_method_uppercase(self): method = sanitize_method("GET") @@ -35,7 +34,7 @@ def test_nonstandard_method(self): self.assertEqual(method, "NONSTANDARD") @patch.dict( - "os.environ", + "os.environ", { OTEL_PYTHON_INSTRUMENTATION_HTTP_CAPTURE_ALL_METHODS: "1", },