Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore metrics in django #1208

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
9c88692
#1043: restore metrics in django
sanketmehta28 Jul 29, 2022
4457dc6
adding eentry in changelog.md
sanketmehta28 Jul 29, 2022
137919d
resolving generate command errors
sanketmehta28 Jul 29, 2022
dd0c870
Merge branch 'main' into develop/restore-metrics-django
srikanthccv Aug 4, 2022
0caaab4
removing numpy as it was unnecessary
sanketmehta28 Aug 4, 2022
4857919
Merge branch 'main' into develop/restore-metrics-django
sanketmehta28 Aug 4, 2022
bf61f1e
resolving linting error
sanketmehta28 Aug 4, 2022
dca6b04
added check for duration and point.values
sanketmehta28 Aug 11, 2022
306b866
Merge branch 'main' into develop/restore-metrics-django
sanketmehta28 Aug 11, 2022
d0dc0a4
resolving linting issues
sanketmehta28 Aug 11, 2022
396df46
Adding None Checks before adding values to metrics. Adding test for u…
sanketmehta28 Aug 17, 2022
f3523c2
Resolving Lint errors
sanketmehta28 Aug 18, 2022
f4ffef0
resolving review comments
sanketmehta28 Aug 18, 2022
4ad66b6
changing import paths for metrics attributes
sanketmehta28 Aug 18, 2022
3f905e8
Resolving linting errors
sanketmehta28 Aug 18, 2022
804e42b
added response status code to duration_attrs
sanketmehta28 Aug 22, 2022
0ded68e
checking for duration_attr for none and start the timer after span is…
sanketmehta28 Aug 23, 2022
98756b4
Merge branch 'main' into develop/restore-metrics-django
sanketmehta28 Aug 23, 2022
91c1d2a
resolving merge conflicts in changelog.md
sanketmehta28 Aug 23, 2022
73a8161
resolving linting issues
sanketmehta28 Aug 23, 2022
55d5726
restoring deleted entries
sanketmehta28 Aug 24, 2022
71ecd20
Merge branch 'main' into develop/restore-metrics-django
sanketmehta28 Aug 24, 2022
7b54477
Resolved merge conflicts
sanketmehta28 Aug 24, 2022
4ed54e3
Resolving merging conflicts in changelog.md file
sanketmehta28 Aug 24, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `opentelemetry-instrumentation-boto3sqs` Make propagation compatible with other SQS instrumentations, add 'messaging.url' span attribute, and fix missing package dependencies.
srikanthccv marked this conversation as resolved.
Show resolved Hide resolved
([#1234](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1234))

- restoring metrics in django framework
([#1208](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1208))

## [1.12.0-0.33b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.12.0-0.33b0) - 2022-08-08

- Adding multiple db connections support for django-instrumentation's sqlcommenter
Expand Down
2 changes: 1 addition & 1 deletion instrumentation/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
| [opentelemetry-instrumentation-celery](./opentelemetry-instrumentation-celery) | celery >= 4.0, < 6.0 | No
| [opentelemetry-instrumentation-confluent-kafka](./opentelemetry-instrumentation-confluent-kafka) | confluent-kafka ~= 1.8.2 | No
| [opentelemetry-instrumentation-dbapi](./opentelemetry-instrumentation-dbapi) | dbapi | No
| [opentelemetry-instrumentation-django](./opentelemetry-instrumentation-django) | django >= 1.10 | No
| [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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ def response_hook(span, request, response):
from opentelemetry.instrumentation.django.package import _instruments
from opentelemetry.instrumentation.django.version import __version__
from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
from opentelemetry.metrics import get_meter
from opentelemetry.trace import get_tracer

DJANGO_2_0 = django_version >= (2, 0)
Expand Down Expand Up @@ -244,19 +245,29 @@ def _instrument(self, **kwargs):
return

tracer_provider = kwargs.get("tracer_provider")
meter_provider = kwargs.get("meter_provider")
tracer = get_tracer(
__name__,
__version__,
tracer_provider=tracer_provider,
)

meter = get_meter(__name__, __version__, meter_provider=meter_provider)
_DjangoMiddleware._tracer = tracer

_DjangoMiddleware._meter = meter
_DjangoMiddleware._otel_request_hook = kwargs.pop("request_hook", None)
_DjangoMiddleware._otel_response_hook = kwargs.pop(
"response_hook", None
)

_DjangoMiddleware._duration_histogram = meter.create_histogram(
name="http.server.duration",
unit="ms",
description="measures the duration of the inbound http request",
)
_DjangoMiddleware._active_request_counter = meter.create_up_down_counter(
name="http.server.active_requests",
unit="requests",
description="measures the number of concurent HTTP requests those are currently in flight",
)
# This can not be solved, but is an inherent problem of this approach:
# the order of middleware entries matters, and here you have no control
# on that:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import types
from logging import getLogger
from time import time
from timeit import default_timer
from typing import Callable

from django import VERSION as django_version
Expand All @@ -41,7 +42,12 @@
from opentelemetry.instrumentation.wsgi import wsgi_getter
from opentelemetry.semconv.trace import SpanAttributes
from opentelemetry.trace import Span, SpanKind, use_span
from opentelemetry.util.http import get_excluded_urls, get_traced_request_attrs
from opentelemetry.util.http import (
_parse_active_request_count_attrs,
_parse_duration_attrs,
get_excluded_urls,
get_traced_request_attrs,
)

try:
from django.core.urlresolvers import ( # pylint: disable=no-name-in-module
Expand Down Expand Up @@ -139,10 +145,19 @@ class _DjangoMiddleware(MiddlewareMixin):
_environ_token = "opentelemetry-instrumentor-django.token"
_environ_span_key = "opentelemetry-instrumentor-django.span_key"
_environ_exception_key = "opentelemetry-instrumentor-django.exception_key"

_environ_active_request_attr_key = (
"opentelemetry-instrumentor-django.active_request_attr_key"
)
_environ_duration_attr_key = (
"opentelemetry-instrumentor-django.duration_attr_key"
)
_environ_timer_key = "opentelemetry-instrumentor-django.timer_key"
_traced_request_attrs = get_traced_request_attrs("DJANGO")
_excluded_urls = get_excluded_urls("DJANGO")
_tracer = None
_meter = None
_duration_histogram = None
_active_request_counter = None

_otel_request_hook: Callable[[Span, HttpRequest], None] = None
_otel_response_hook: Callable[
Expand Down Expand Up @@ -171,6 +186,7 @@ def _get_span_name(request):
except Resolver404:
return f"HTTP {request.method}"

# pylint: disable=too-many-locals
def process_request(self, request):
# request.META is a dictionary containing all available HTTP headers
# Read more about request.META here:
Expand All @@ -185,7 +201,6 @@ def process_request(self, request):

# pylint:disable=W0212
request._otel_start_time = time()

request_meta = request.META

if is_asgi_request:
Expand All @@ -208,7 +223,16 @@ def process_request(self, request):
)

attributes = collect_request_attributes(carrier)
active_requests_count_attrs = _parse_active_request_count_attrs(
attributes
)
duration_attrs = _parse_duration_attrs(attributes)

request.META[
self._environ_active_request_attr_key
] = active_requests_count_attrs
request.META[self._environ_duration_attr_key] = duration_attrs
self._active_request_counter.add(1, active_requests_count_attrs)
if span.is_recording():
attributes = extract_attributes_from_object(
request, self._traced_request_attrs, attributes
Expand Down Expand Up @@ -242,7 +266,8 @@ def process_request(self, request):

activation = use_span(span, end_on_exit=True)
activation.__enter__() # pylint: disable=E1101

request_start_time = default_timer()
request.META[self._environ_timer_key] = request_start_time
request.META[self._environ_activation_key] = activation
request.META[self._environ_span_key] = span
if token:
Expand Down Expand Up @@ -281,6 +306,7 @@ def process_exception(self, request, exception):
request.META[self._environ_exception_key] = exception

# pylint: disable=too-many-branches
# pylint: disable=too-many-locals
def process_response(self, request, response):
if self._excluded_urls.url_disabled(request.build_absolute_uri("?")):
return response
Expand All @@ -291,6 +317,17 @@ def process_response(self, request, response):

activation = request.META.pop(self._environ_activation_key, None)
span = request.META.pop(self._environ_span_key, None)
active_requests_count_attrs = request.META.pop(
self._environ_active_request_attr_key, None
)
duration_attrs = request.META.pop(
sanketmehta28 marked this conversation as resolved.
Show resolved Hide resolved
self._environ_duration_attr_key, None
)
if duration_attrs:
duration_attrs[
SpanAttributes.HTTP_STATUS_CODE
] = response.status_code
request_start_time = request.META.pop(self._environ_timer_key, None)

if activation and span:
if is_asgi_request:
Expand Down Expand Up @@ -341,6 +378,12 @@ def process_response(self, request, response):
else:
activation.__exit__(None, None, None)

if request_start_time is not None:
duration = max(
round((default_timer() - request_start_time) * 1000), 0
)
self._duration_histogram.record(duration, duration_attrs)
self._active_request_counter.add(-1, active_requests_count_attrs)
if request.META.get(self._environ_token, None) is not None:
detach(request.META.get(self._environ_token))
request.META.pop(self._environ_token)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,4 @@


_instruments = ("django >= 1.10",)
_supports_metrics = True
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
# pylint: disable=E0611

from sys import modules
from timeit import default_timer
from unittest.mock import Mock, patch

from django import VERSION, conf
Expand All @@ -32,6 +33,10 @@
set_global_response_propagator,
)
from opentelemetry.sdk import resources
from opentelemetry.sdk.metrics.export import (
HistogramDataPoint,
NumberDataPoint,
)
from opentelemetry.sdk.trace import Span
from opentelemetry.sdk.trace.id_generator import RandomIdGenerator
from opentelemetry.semconv.trace import SpanAttributes
Expand All @@ -45,6 +50,8 @@
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,
get_traced_request_attrs,
)
Expand Down Expand Up @@ -406,6 +413,64 @@ def test_trace_response_headers(self):
)
self.memory_exporter.clear()

# pylint: disable=too-many-locals
def test_wsgi_metrics(self):
_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,
}
start = default_timer()
for _ in range(3):
response = Client().get("/span_name/1234/")
self.assertEqual(response.status_code, 200)
duration = max(round((default_timer() - start) * 1000), 0)
metrics_list = self.memory_metrics_reader.get_metrics_data()
number_data_point_seen = False
histrogram_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)
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)
sanketmehta28 marked this conversation as resolved.
Show resolved Hide resolved
histrogram_data_point_seen = True
self.assertAlmostEqual(
duration, point.sum, delta=100
)
if isinstance(point, NumberDataPoint):
number_data_point_seen = True
self.assertEqual(point.value, 0)
for attr in point.attributes:
self.assertIn(
attr, _recommended_attrs[metric.name]
)
self.assertTrue(histrogram_data_point_seen and number_data_point_seen)

sanketmehta28 marked this conversation as resolved.
Show resolved Hide resolved
def test_wsgi_metrics_unistrument(self):
srikanthccv marked this conversation as resolved.
Show resolved Hide resolved
Client().get("/span_name/1234/")
_django_instrumentor.uninstrument()
Client().get("/span_name/1234/")
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(1, point.count)
if isinstance(point, NumberDataPoint):
self.assertEqual(0, point.value)


class TestMiddlewareWithTracerProvider(WsgiTestBase):
@classmethod
Expand Down