Django instrumentation - Trace request attributes also in the response

[Zajozor]

Sorry, but I'm slightly confused whether I should create an issue in this repository or https://github.com/open-telemetry/opentelemetry-python-contrib. If it's the other one, i'll gladly reopen it there.

---

Is your feature request related to a problem?
According to the docs and the implementation, you can use OTEL_PYTHON_DJANGO_TRACED_REQUEST_ATTRS to specify what request attributes should be traced.

The middleware uses these options during process_request in https://github.com/open-telemetry/opentelemetry-python/blob/master/instrumentation/opentelemetry-instrumentation-django/src/opentelemetry/instrumentation/django/middleware.py#L147

The problem is that some of the request attributes that one could wish to trace are available only after some other middlewares run. For example (pretty common setups):

• django.contrib.auth.middleware.AuthenticationMiddleware adds the user attribute
• django.contrib.sites.middleware.CurrentSiteMiddleware adds the site attribute

Of course, we cannot mess with the middleware order, but these attributes are available in the process_response part of the middleware.

Describe the solution you'd like
It would be nice if we could specify attributes that would be extracted in the process_response, so that we can extract these as well using the instrumentation library.
I do not have a preference, and I am describing the options that come to my mind in the next section.

Describe alternatives you've considered

1. Extract all attributes in process_response instead of process_request
2. Include additional configuration variable so that we somehow distinguish between the two extraction places.
3. Try extracting the attributes in process_request as well as process_response (maybe only try to extract the unextracted ones again?)

Additional context
I am willing to create a PR for this upon the decision to do it one way or another.

Thanks.

[lzchen]

Sorry, but I'm slightly confused whether I should create an issue in this repository or https://github.com/open-telemetry/opentelemetry-python-contrib. If it's the other one, i'll gladly reopen it there.

For now, this is the right place to open up issues related to instrumentations.

[owais]

1. Extract all attributes in process_response instead of process_request

This sounds good to me but we might have to do the same in process_exception in case process_response is not called in some exceptional cases. Disregard if it is always called.

[piotrjez]

Perhaps it would be a good idea to add an environment variable to control what range of data in the logged-in user area would be exported. I can imagine cases where someone might not want to export data such as email address, but only the pk of the user object.
Additionally, it might be worth considering an environment variable that would allow exporting custom attributes added by the middleware to the self.request attribute.
What are your thoughts on this? I think it is worth considering. The current solution is to create a custom middleware that overrides the current span using the .set_attribute() method which in turn triggers a warning: Calling end() on an ended span.

class AuthenticatedUserTracingMiddleware:
    def __init__(self, get_response):
        self.get_response = get_response

    def __call__(self, request):
        if request.user.is_authenticated:
            with trace.get_current_span() as span:
                span.set_attribute("enduser.id", request.user.pk)
        response = self.get_response(request)
        return response

EDIT:
The use of the above code causes a problem where, for some queries, if the dependency returns an error, the response code for the request is 0 (obfuscation of the response code)

[piotrjez]

@owais what do you think about my previous comment? Do you think that it's okay to implement extended logic for authenticated user attribute?

[lzchen]

@piotrjez

Would your scenario be addressed by adding a OTEL_PYTHON_DJANGO_TRACED_RESPONSE_ATTRS configuration?

[piotrjez]

@lzchen
3 weeks ago open-telemetry/opentelemetry-python#4104 was merged and introduced new user attributes.
https://github.com/open-telemetry/opentelemetry-python/pull/4104/files#diff-f65bec0a6ba06083299cbc3f5e4419a62d58e8df8b0be7b353da9795b7024fd0

I think that we should extract new variable which gives control of user_attributes:
OTEL_PYTHON_DJANGO_TRACED_RESPONSE_USER_ATTRS='id=user.pk,email=user.email' -> span._attributes["user.id"] = response.user.pk, span._attributes["user.email"] = response.user.email

This approach is flexible and gives control to the user which attributes export as group of user attributes.

What do you thin about this approach?

[lzchen]

@piotrjez

I am not sure if the original issue is specific to user attributes. Any response to this question?

[piotrjez]

@piotrjez

Would your scenario be addressed by adding a OTEL_PYTHON_DJANGO_TRACED_RESPONSE_ATTRS configuration?

@lzchen
Adding OTEL_PYTHON_DJANGO_TRACED_RESPONSE_ATTRS will not be sufficient because it will not be possible to control what user data is to be transmitted. Specifying OTEL_PYTHON_DJANGO_TRACED_RESPONSE_ATTRS='user' will not allow you to explicitly specify that you want to pass user.pk and user.email.
My suggestion is to add the environment variable OTEL_PYTHON_DJANGO_TRACED_USER_ATTRS='id=user.pk,email=user.email' which will be equivalent to span._attributes["user.id"] = request.user.pk, span._attributes["user.email"] = request.user.email.
This solution will allow full control over the scope of logged-in user data transferred, with future support for attributes that will appear in opentelemetry for the user scope.

It is worth mentioning that for the above solution to work properly, OTEL_PYTHON_DJANGO_MIDDLEWARE_POSITION must be defined in such a way that Django middleware appears after django.contrib.auth.middleware.AuthenticationMiddleware (default OTEL_PYTHON_DJANGO_MIDDLEWARE_POSITION=0 and comes before Django AuthenticationMiddleware). Otherwise, the user attribute will not be already added in the request object. It is worth describing this environment variable in the documentation, because currently there is no such description; I came across it in the source code.

[piotrjez]

After many attempts, I managed to find a solution that meets my expectations and does not require changes in the opentelemetry-instrumentation-celery area. Below is the code that allows you to pass attributes after Django middlewares run. This strategy only works for response_hook, not for request_hook, because at the time request_hook is called, the middleware is not yet running.

def response_hook(span, request, response):
    if request.user.is_authenticated:
        span.set_attribute("enduser.id", request.user.pk)

DjangoInstrumentor().instrument(response_hook=response_hook)

@Zajozor, does the above code suggestion not meet your needs?

[piotrjez]

I have provided supplementary documentation for this area as part of: #3923