Merge branch 'main' into metrics-instrumentation-starlette

Author: rahulmukherjee68

CHANGELOG.md

@@ -10,15 +10,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 - Flask sqlalchemy psycopg2 integration
   ([#1224](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1224))
+- Add metric instrumentation in fastapi
+  ([#1199](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1199))
+- Add metric instrumentation in Pyramid
+  ([#1242](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1242))
 
 ### Fixed
 
 - `opentelemetry-instrumentation-boto3sqs` Make propagation compatible with other SQS instrumentations, add 'messaging.url' span attribute, and fix missing package dependencies.
-  ([#1234](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1234))
+  ([#1234](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1234)) 
+- `opentelemetry-instrumentation-pymongo` Change span names to not contain queries but only database name and command name
+  ([#1247](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1247)) 
 - restoring metrics in django framework
   ([#1208](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1208))
 - `opentelemetry-instrumentation-aiohttp-client` Fix producing additional spans with each newly created ClientSession
 - ([#1246](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1246))
+- Add _is_openetlemetry_instrumented check in _InstrumentedFastAPI class
+  ([#1313](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1313))
 
 ## [1.12.0-0.33b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.12.0-0.33b0) - 2022-08-08
 

instrumentation/README.md

@@ -16,7 +16,7 @@
 | [opentelemetry-instrumentation-django](./opentelemetry-instrumentation-django) | django >= 1.10 | Yes
 | [opentelemetry-instrumentation-elasticsearch](./opentelemetry-instrumentation-elasticsearch) | elasticsearch >= 2.0 | No
 | [opentelemetry-instrumentation-falcon](./opentelemetry-instrumentation-falcon) | falcon >= 1.4.1, < 4.0.0 | No
-| [opentelemetry-instrumentation-fastapi](./opentelemetry-instrumentation-fastapi) | fastapi ~= 0.58 | No
+| [opentelemetry-instrumentation-fastapi](./opentelemetry-instrumentation-fastapi) | fastapi ~= 0.58 | Yes
 | [opentelemetry-instrumentation-flask](./opentelemetry-instrumentation-flask) | flask >= 1.0, < 3.0 | Yes
 | [opentelemetry-instrumentation-grpc](./opentelemetry-instrumentation-grpc) | grpcio ~= 1.27 | No
 | [opentelemetry-instrumentation-httpx](./opentelemetry-instrumentation-httpx) | httpx >= 0.18.0 | No

instrumentation/opentelemetry-instrumentation-asgi/src/opentelemetry/instrumentation/asgi/__init__.py

@@ -396,10 +396,15 @@ def __init__(
         client_response_hook: _ClientResponseHookT = None,
         tracer_provider=None,
         meter_provider=None,
+        meter=None,
     ):
         self.app = guarantee_single_callable(app)
         self.tracer = trace.get_tracer(__name__, __version__, tracer_provider)
-        self.meter = get_meter(__name__, __version__, meter_provider)
+        self.meter = (
+            get_meter(__name__, __version__, meter_provider)
+            if meter is None
+            else meter
+        )
         self.duration_histogram = self.meter.create_histogram(
             name="http.server.duration",
             unit="ms",

instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/__init__.py

@@ -137,7 +137,9 @@ def client_response_hook(span: Span, message: dict):
 
 from opentelemetry.instrumentation.asgi import OpenTelemetryMiddleware
 from opentelemetry.instrumentation.asgi.package import _instruments
+from opentelemetry.instrumentation.fastapi.version import __version__
 from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
+from opentelemetry.metrics import get_meter
 from opentelemetry.semconv.trace import SpanAttributes
 from opentelemetry.trace import Span
 from opentelemetry.util.http import get_excluded_urls, parse_excluded_urls
@@ -165,6 +167,7 @@ def instrument_app(
         client_request_hook: _ClientRequestHookT = None,
         client_response_hook: _ClientResponseHookT = None,
         tracer_provider=None,
+        meter_provider=None,
         excluded_urls=None,
     ):
         """Instrument an uninstrumented FastAPI application."""
@@ -176,6 +179,7 @@ def instrument_app(
                 excluded_urls = _excluded_urls_from_env
             else:
                 excluded_urls = parse_excluded_urls(excluded_urls)
+            meter = get_meter(__name__, __version__, meter_provider)
 
             app.add_middleware(
                 OpenTelemetryMiddleware,
@@ -185,6 +189,7 @@ def instrument_app(
                 client_request_hook=client_request_hook,
                 client_response_hook=client_response_hook,
                 tracer_provider=tracer_provider,
+                meter=meter,
             )
             app._is_instrumented_by_opentelemetry = True
         else:
@@ -223,6 +228,7 @@ def _instrument(self, **kwargs):
             if _excluded_urls is None
             else parse_excluded_urls(_excluded_urls)
         )
+        _InstrumentedFastAPI._meter_provider = kwargs.get("meter_provider")
         fastapi.FastAPI = _InstrumentedFastAPI
 
     def _uninstrument(self, **kwargs):
@@ -231,13 +237,17 @@ def _uninstrument(self, **kwargs):
 
 class _InstrumentedFastAPI(fastapi.FastAPI):
     _tracer_provider = None
+    _meter_provider = None
     _excluded_urls = None
     _server_request_hook: _ServerRequestHookT = None
     _client_request_hook: _ClientRequestHookT = None
     _client_response_hook: _ClientResponseHookT = None
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
+        meter = get_meter(
+            __name__, __version__, _InstrumentedFastAPI._meter_provider
+        )
         self.add_middleware(
             OpenTelemetryMiddleware,
             excluded_urls=_InstrumentedFastAPI._excluded_urls,
@@ -246,7 +256,9 @@ def __init__(self, *args, **kwargs):
             client_request_hook=_InstrumentedFastAPI._client_request_hook,
             client_response_hook=_InstrumentedFastAPI._client_response_hook,
             tracer_provider=_InstrumentedFastAPI._tracer_provider,
+            meter=meter,
         )
+        self._is_instrumented_by_opentelemetry = True
 
 
 def _get_route_details(scope):

instrumentation/opentelemetry-instrumentation-fastapi/src/opentelemetry/instrumentation/fastapi/package.py

@@ -14,3 +14,5 @@
 
 
 _instruments = ("fastapi ~= 0.58",)
+
+_supports_metrics = True

instrumentation/opentelemetry-instrumentation-fastapi/tests/test_fastapi_instrumentation.py

@@ -13,6 +13,7 @@
 # limitations under the License.
 
 import unittest
+from timeit import default_timer
 from unittest.mock import patch
 
 import fastapi
@@ -22,16 +23,31 @@
 import opentelemetry.instrumentation.fastapi as otel_fastapi
 from opentelemetry import trace
 from opentelemetry.instrumentation.asgi import OpenTelemetryMiddleware
+from opentelemetry.sdk.metrics.export import (
+    HistogramDataPoint,
+    NumberDataPoint,
+)
 from opentelemetry.sdk.resources import Resource
 from opentelemetry.semconv.trace import SpanAttributes
 from opentelemetry.test.globals_test import reset_trace_globals
 from opentelemetry.test.test_base import TestBase
 from opentelemetry.util.http import (
     OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_REQUEST,
     OTEL_INSTRUMENTATION_HTTP_CAPTURE_HEADERS_SERVER_RESPONSE,
+    _active_requests_count_attrs,
+    _duration_attrs,
     get_excluded_urls,
 )
 
+_expected_metric_names = [
+    "http.server.active_requests",
+    "http.server.duration",
+]
+_recommended_attrs = {
+    "http.server.active_requests": _active_requests_count_attrs,
+    "http.server.duration": _duration_attrs,
+}
+
 
 class TestFastAPIManualInstrumentation(TestBase):
     def _create_app(self):
@@ -161,6 +177,124 @@ def test_fastapi_excluded_urls_not_env(self):
         spans = self.memory_exporter.get_finished_spans()
         self.assertEqual(len(spans), 0)
 
+    def test_fastapi_metrics(self):
+        self._client.get("/foobar")
+        self._client.get("/foobar")
+        self._client.get("/foobar")
+        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) == 1)
+        for resource_metric in metrics_list.resource_metrics:
+            self.assertTrue(len(resource_metric.scope_metrics) == 1)
+            for scope_metric in resource_metric.scope_metrics:
+                self.assertTrue(len(scope_metric.metrics) == 2)
+                for metric in scope_metric.metrics:
+                    self.assertIn(metric.name, _expected_metric_names)
+                    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, _recommended_attrs[metric.name]
+                            )
+        self.assertTrue(number_data_point_seen and histogram_data_point_seen)
+
+    def test_basic_metric_success(self):
+        start = default_timer()
+        self._client.get("/foobar")
+        duration = max(round((default_timer() - start) * 1000), 0)
+        expected_duration_attributes = {
+            "http.method": "GET",
+            "http.host": "testserver",
+            "http.scheme": "http",
+            "http.flavor": "1.1",
+            "http.server_name": "testserver",
+            "net.host.port": 80,
+            "http.status_code": 200,
+        }
+        expected_requests_count_attributes = {
+            "http.method": "GET",
+            "http.host": "testserver",
+            "http.scheme": "http",
+            "http.flavor": "1.1",
+            "http.server_name": "testserver",
+        }
+        metrics_list = self.memory_metrics_reader.get_metrics_data()
+        for metric in (
+            metrics_list.resource_metrics[0].scope_metrics[0].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)
+                    self.assertAlmostEqual(duration, point.sum, delta=20)
+                if isinstance(point, NumberDataPoint):
+                    self.assertDictEqual(
+                        expected_requests_count_attributes,
+                        dict(point.attributes),
+                    )
+                    self.assertEqual(point.value, 0)
+
+    def test_basic_post_request_metric_success(self):
+        start = default_timer()
+        self._client.post("/foobar")
+        duration = max(round((default_timer() - start) * 1000), 0)
+        metrics_list = self.memory_metrics_reader.get_metrics_data()
+        for metric in (
+            metrics_list.resource_metrics[0].scope_metrics[0].metrics
+        ):
+            for point in list(metric.data.data_points):
+                if isinstance(point, HistogramDataPoint):
+                    self.assertEqual(point.count, 1)
+                    self.assertAlmostEqual(duration, point.sum, delta=30)
+                if isinstance(point, NumberDataPoint):
+                    self.assertEqual(point.value, 0)
+
+    def test_metric_uninstruemnt_app(self):
+        self._client.get("/foobar")
+        self._instrumentor.uninstrument_app(self._app)
+        self._client.get("/foobar")
+        metrics_list = self.memory_metrics_reader.get_metrics_data()
+        for metric in (
+            metrics_list.resource_metrics[0].scope_metrics[0].metrics
+        ):
+            for point in list(metric.data.data_points):
+                if isinstance(point, HistogramDataPoint):
+                    self.assertEqual(point.count, 1)
+                if isinstance(point, NumberDataPoint):
+                    self.assertEqual(point.value, 0)
+
+    def test_metric_uninstrument(self):
+        # instrumenting class and creating app to send request
+        self._instrumentor.instrument()
+        app = self._create_fastapi_app()
+        client = TestClient(app)
+        client.get("/foobar")
+        # uninstrumenting class and creating the app again
+        self._instrumentor.uninstrument()
+        app = self._create_fastapi_app()
+        client = TestClient(app)
+        client.get("/foobar")
+
+        metrics_list = self.memory_metrics_reader.get_metrics_data()
+        for metric in (
+            metrics_list.resource_metrics[0].scope_metrics[0].metrics
+        ):
+            for point in list(metric.data.data_points):
+                if isinstance(point, HistogramDataPoint):
+                    self.assertEqual(point.count, 1)
+                if isinstance(point, NumberDataPoint):
+                    self.assertEqual(point.value, 0)
+
     @staticmethod
     def _create_fastapi_app():
         app = fastapi.FastAPI()
@@ -274,6 +408,14 @@ def test_request(self):
             self.assertEqual(span.resource.attributes["key1"], "value1")
             self.assertEqual(span.resource.attributes["key2"], "value2")
 
+    def test_mulitple_way_instrumentation(self):
+        self._instrumentor.instrument_app(self._app)
+        count = 0
+        for middleware in self._app.user_middleware:
+            if middleware.cls is OpenTelemetryMiddleware:
+                count += 1
+        self.assertEqual(count, 1)
+
     def tearDown(self):
         self._instrumentor.uninstrument()
         super().tearDown()

instrumentation/opentelemetry-instrumentation-pymongo/src/opentelemetry/instrumentation/pymongo/__init__.py

@@ -121,10 +121,10 @@ def started(self, event: monitoring.CommandStartedEvent):
         ):
             return
         command = event.command.get(event.command_name, "")
-        name = event.command_name
+        name = event.database_name
+        name += "." + event.command_name
         statement = event.command_name
         if command:
-            name += "." + str(command)
             statement += " " + str(command)
 
         try:

instrumentation/opentelemetry-instrumentation-pymongo/tests/test_pymongo.py

@@ -40,7 +40,6 @@ def test_pymongo_instrumentor(self):
         )
         with patch:
             PymongoInstrumentor().instrument()
-
         self.assertTrue(mock_register.called)
 
     def test_started(self):
@@ -59,7 +58,7 @@ def test_started(self):
         # pylint: disable=protected-access
         span = command_tracer._pop_span(mock_event)
         self.assertIs(span.kind, trace_api.SpanKind.CLIENT)
-        self.assertEqual(span.name, "command_name.find")
+        self.assertEqual(span.name, "database_name.command_name")
         self.assertEqual(span.attributes[SpanAttributes.DB_SYSTEM], "mongodb")
         self.assertEqual(
             span.attributes[SpanAttributes.DB_NAME], "database_name"
@@ -189,8 +188,7 @@ def test_int_command(self):
 
         self.assertEqual(len(spans_list), 1)
         span = spans_list[0]
-
-        self.assertEqual(span.name, "command_name.123")
+        self.assertEqual(span.name, "database_name.command_name")
 
 
 class MockCommand: