-
Notifications
You must be signed in to change notification settings - Fork 631
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
Django instrumentation - Trace request attributes also in the response #142
Comments
For now, this is the right place to open up issues related to instrumentations. |
Fixes open-telemetry#142 Enabling --strict mode for mypy. Added a test in the sdk and the same test in the api to test the different behaviours between the Tracer, Span and Metric classes.
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. |
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. 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: |
@owais what do you think about my previous comment? Do you think that it's okay to implement extended logic for authenticated user attribute? |
Would your scenario be addressed by adding a |
@lzchen I think that we should extract new variable which gives control of 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? |
I am not sure if the original issue is specific to user attributes. Any response to this question? |
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#L147The 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 theuser
attributedjango.contrib.sites.middleware.CurrentSiteMiddleware
adds thesite
attributeOf 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
process_response
instead ofprocess_request
process_request
as well asprocess_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.
The text was updated successfully, but these errors were encountered: