django-prometheus icon indicating copy to clipboard operation
django-prometheus copied to clipboard

Error with custom labels metrics

Open evgmoskalenko opened this issue 5 years ago • 11 comments

Hello, when I update this solution to the 2.0.0 version and try to use custom labels for metric - https://github.com/korfuri/django-prometheus/blob/master/django_prometheus/tests/end2end/testapp/test_middleware_custom_labels.py#L19-L46. I have an exception for each method.

My middleware settings:

MIDDLEWARE = [
    "metrics.middleware_custom_labels.AppMetricsBeforeMiddleware",
    # "django_prometheus.middleware.PrometheusBeforeMiddleware",
    'django.middleware.security.SecurityMiddleware',
    'django.middleware.common.CommonMiddleware',
    # "django_prometheus.middleware.PrometheusAfterMiddleware",
    "metrics.middleware_custom_labels.AppMetricsBeforeMiddleware",
]

My custom middleware:

import logging
from django.conf import settings
# from prometheus_client import REGISTRY
# from prometheus_client.metrics import MetricWrapperBase
from django_prometheus.middleware import (
    Metrics,
    PrometheusAfterMiddleware,
    PrometheusBeforeMiddleware,
)

logger = logging.getLogger(__name__)


class CustomMetrics(Metrics):
    def register_metric(
        self, metric_cls, name, documentation, labelnames=tuple(), **kwargs
    ):
        
        labels = list(labelnames)
        labels.extend(("app", "user_agent_type"))
        labelnames = tuple(labels)
        logger.info(labelnames)

        return super(CustomMetrics, self).register_metric(
            metric_cls, name, documentation, labelnames=labelnames, **kwargs
        )


class AppMetricsBeforeMiddleware(PrometheusBeforeMiddleware):
    metrics_cls = CustomMetrics


class AppMetricsAfterMiddleware(PrometheusAfterMiddleware):
    metrics_cls = CustomMetrics

    def label_metric(self, metric, request, response=None, **labels):
        new_labels = labels
        new_labels = {"app": "my-application-name", "user_agent_type": "api"}
        new_labels.update(labels)
        return super(AppMetricsAfterMiddleware, self).label_metric(
            metric, request, response=response, **new_labels
        )

Versions:

  • Django = "2.2.10"
  • django-prometheus = "2.0.0"

Exception in log application when I execute some rest method:

[2020-02-24 17:09:30,108 ERROR] django.request | Internal Server Error: /metrics
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/site-packages/django/core/handlers/exception.py", line 34, in inner
    response = get_response(request)
  File "/usr/local/lib/python3.7/site-packages/django/utils/deprecation.py", line 93, in __call__
    response = self.process_request(request)
  File "/usr/local/lib/python3.7/site-packages/django_prometheus/middleware.py", line 177, in process_request
    self.metrics.requests_total.inc()
  File "/usr/local/lib/python3.7/site-packages/prometheus_client/metrics.py", line 244, in inc
    self._value.inc(amount)
AttributeError: 'Counter' object has no attribute '_value'
Internal Server Error: /metrics
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/site-packages/django/core/handlers/exception.py", line 34, in inner
    response = get_response(request)
  File "/usr/local/lib/python3.7/site-packages/django/utils/deprecation.py", line 93, in __call__
    response = self.process_request(request)
  File "/usr/local/lib/python3.7/site-packages/django_prometheus/middleware.py", line 177, in process_request
    self.metrics.requests_total.inc()
  File "/usr/local/lib/python3.7/site-packages/prometheus_client/metrics.py", line 244, in inc
    self._value.inc(amount)
AttributeError: 'Counter' object has no attribute '_value'
[2020-02-24 17:09:30,108 ERROR] django.request | Internal Server Error: /metrics
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/site-packages/django/core/handlers/exception.py", line 34, in inner
    response = get_response(request)
  File "/usr/local/lib/python3.7/site-packages/django/utils/deprecation.py", line 93, in __call__
    response = self.process_request(request)
  File "/usr/local/lib/python3.7/site-packages/django_prometheus/middleware.py", line 177, in process_request
    self.metrics.requests_total.inc()
  File "/usr/local/lib/python3.7/site-packages/prometheus_client/metrics.py", line 244, in inc
    self._value.inc(amount)
AttributeError: 'Counter' object has no attribute '_value'

evgmoskalenko avatar Feb 24 '20 17:02 evgmoskalenko

And, Could you tell me please, how you can use this function in example - https://github.com/korfuri/django-prometheus/blob/master/django_prometheus/tests/end2end/testapp/test_middleware_custom_labels.py#L19-L46:

labelnames.extend(("view_type", "user_agent_type"))

If tuple() object has no attribute 'extend' :-)

AttributeError: 'tuple' object has no attribute 'extend'

evgmoskalenko avatar Feb 24 '20 17:02 evgmoskalenko

Working only if comment all counter metrics like this:

EXTENDED_METRICS = [
    M("requests_latency_seconds_by_view_method"),
    M("responses_total_by_status_view_method"),
    # M("requests_before_middlewares_total"),
    # M("responses_before_middlewares_total"),
    # M("requests_latency_including_middlewares_seconds"),
    # M("requests_unknown_latency_including_middlewares_total"),
    # M("requests_unknown_latency_total"),
    # M("ajax_requests_total"),
    # M("requests_total_by_transport"),
    # M("requests_total_by_view_transport_method"),
    # M("requests_body_total_bytes"),
    # M("responses_total_by_templatename"),
    # M("responses_total_by_status"),
    # M("responses_body_total_bytes"),
    # M("responses_total_by_charset"),
    # M("responses_streaming_total"),
    # M("exceptions_total_by_type"),
    # M("exceptions_total_by_view"),
]

evgmoskalenko avatar Feb 24 '20 18:02 evgmoskalenko

It seems this is related to when metrics are tracked without adding labels first. In the after middleware there is a label_metric() function that adds the labels. However the before middleware does not have an equivalent that does labeling when observing and incrementing: https://github.com/korfuri/django-prometheus/blob/2.1.0/django_prometheus/middleware.py#L197 https://github.com/korfuri/django-prometheus/blob/2.1.0/django_prometheus/middleware.py#L201 https://github.com/korfuri/django-prometheus/blob/2.1.0/django_prometheus/middleware.py#L203

As a result adding labels to those metrics will cause issues during the middleware processing. I was able to fix this in my own middleware by overriding process_request and process_response in the Before middleware to add labels where needed.

chrinor2002 avatar Sep 16 '20 20:09 chrinor2002

@chrinor2002 I'm having the same issue, Do you only add the labels to process_request and process_response and then call super() method? I don't understand completely what's the workaround

afdecastro879 avatar Sep 18 '20 20:09 afdecastro879

@afdecastro879 for us we ended up adding labels to where they were not being added within process_request and process_response (disclaimer below code is simplified from our source for demonstration purposes):

from django_prometheus.middleware import (
    PrometheusBeforeMiddleware,
    PrometheusAfterMiddleware,
)
from django_prometheus.utils import Time, TimeSince

class MyAppPrometheusBeforeMiddleware(PrometheusBeforeMiddleware):
    metrics_cls = MyAppMetrics

    def process_request(self, request):
        labels = myapp_global_labels()
        self.label_metric(
            self.metrics.requests_total,
            request,
            **labels
        ).inc()
        request.prometheus_before_middleware_event = Time()

    def process_response(self, request, response):
        labels = myapp_global_labels()
        self.label_metric(
            self.metrics.responses_total,
            request,
            response,
            **labels
        ).inc()
        if hasattr(request, 'prometheus_before_middleware_event'):
            self.label_metric(
                self.metrics.requests_latency_before,
                request,
                response,
                **labels
            ).observe(
                TimeSince(request.prometheus_before_middleware_event)
            )
        else:
            self.label_metric(
                self.metrics.requests_unknown_latency_before,
                request,
                response,
                **labels
            ).inc()
        return response

class MyAppPrometheusAfterMiddleware(PrometheusAfterMiddleware):
    metrics_cls = MyAppMetrics

Note: self.label_metric() is not actually implemented in the PrometheusBeforeMiddleware, only in PrometheusAfterMiddleware, you will want to re-implement it in the way you see fit. We ended up writing some super classes that took care of the logic for both before and after middleware classes.

@asherf let me know if you need any more explanation on this. I am a bit too slammed to submit a PR, but I can attempt to explain here when I have time slots available.

chrinor2002 avatar Sep 18 '20 21:09 chrinor2002

That makes a lot of sense. Thanks for the quick reply @chrinor2002 . Another quick question. By any change are you using db monitoring features? seems also like there's no way to label those metrics at all.

afdecastro879 avatar Sep 18 '20 21:09 afdecastro879

@afdecastro879 No we are not, although there was talk of a need to monitor so we might in the future. I did notice there was a few metrics in other parts of the module that are just not able to have labels added when they are observed. It's easy to add the labels when doing registration (per the provided examples in the docs) but the hard part is making sure when the metric is observed, the corresponding label value is added. Maybe there is a way to extend those areas with a custom class? maybe there would be a feature to add to this module to allow said extension? If only time was not at such a premium.

chrinor2002 avatar Sep 18 '20 21:09 chrinor2002

We've also run in to this problem, and would really love a solution that doesn't involve essentially copy pasting the whole middleware classes with a few tweaks.

samwho avatar Apr 09 '21 16:04 samwho

@afdecastro879 for us we ended up adding labels to where they were not being added within process_request and process_response (disclaimer below code is simplified from our source for demonstration purposes):

from django_prometheus.middleware import (
    PrometheusBeforeMiddleware,
    PrometheusAfterMiddleware,
)
from django_prometheus.utils import Time, TimeSince

class MyAppPrometheusBeforeMiddleware(PrometheusBeforeMiddleware):
    metrics_cls = MyAppMetrics

    def process_request(self, request):
        labels = myapp_global_labels()
        self.label_metric(
            self.metrics.requests_total,
            request,
            **labels
        ).inc()
        request.prometheus_before_middleware_event = Time()

    def process_response(self, request, response):
        labels = myapp_global_labels()
        self.label_metric(
            self.metrics.responses_total,
            request,
            response,
            **labels
        ).inc()
        if hasattr(request, 'prometheus_before_middleware_event'):
            self.label_metric(
                self.metrics.requests_latency_before,
                request,
                response,
                **labels
            ).observe(
                TimeSince(request.prometheus_before_middleware_event)
            )
        else:
            self.label_metric(
                self.metrics.requests_unknown_latency_before,
                request,
                response,
                **labels
            ).inc()
        return response

class MyAppPrometheusAfterMiddleware(PrometheusAfterMiddleware):
    metrics_cls = MyAppMetrics

Note: self.label_metric() is not actually implemented in the PrometheusBeforeMiddleware, only in PrometheusAfterMiddleware, you will want to re-implement it in the way you see fit. We ended up writing some super classes that took care of the logic for both before and after middleware classes.

@asherf let me know if you need any more explanation on this. I am a bit too slammed to submit a PR, but I can attempt to explain here when I have time slots available.

Hi @chrinor2002 - I am facing this same issue, and thanks for providing a solution, but I was unable to implement it the way you mentioned. Would you be able to share your code on how you got it to work? Thank you.

amarmahtani avatar Jun 16 '21 13:06 amarmahtani

@amarmahtani Unfortunately company policy does not allow us to share our source code.

chrinor2002 avatar Jun 16 '21 20:06 chrinor2002

Thank you @chrinor2002 for the suggestions. I ran into the same issue. Just being verbose about my understanding, the self.metrics.requests_total.inc() call in process_request of the provided Before middleware becomes invalid if we set labelnames to the requests_total metric. Because in that case, requests_total has not been initialized with labelvalues yet.

Although, I went with a little bit different approach. Let's say we are adding the environment type as a label

class CustomMetrics(Metrics):
    def register_metric(self, metric_cls, name, documentation, labelnames=(), **kwargs):
        ln_list = list(labelnames)
        if len(ln_list) == 0:
            labels = {
                ENV_LABEL: settings.ENV_TYPE
            }
            return super().register_metric(
                metric_cls, name, documentation, labelnames=tuple([ENV_LABEL]), **kwargs
            ).labels(**labels)
        ln_list.append(ENV_LABEL)
        labelnames = tuple(ln_list)
        return super().register_metric(
            metric_cls, name, documentation, labelnames=labelnames, **kwargs
        )


class AppMetricsBeforeMiddleware(PrometheusBeforeMiddleware):
    metrics_cls = CustomMetrics


class AppMetricsAfterMiddleware(PrometheusAfterMiddleware):
    metrics_cls = CustomMetrics

    def label_metric(self, metric, request, response=None, **labels):
        new_labels = {
            ENV_LABEL: settings.ENV_TYPE
        }
        new_labels.update(labels)
        try:
            return super().label_metric(metric, request, response=response, **new_labels)
        except ValueError:
            return super().label_metric(metric, request, response=response)

hsinghc-plantx avatar Nov 02 '21 16:11 hsinghc-plantx