feat: more django context #252
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Enhanced Django integration with additional request context capture and broader version compatibility, focusing on user and path information extraction for better error tracking.
- Expanding version support to Django 1.8+ introduces potential security risks, should verify security implications for older versions
- Header lookup change from lowercase to capitalized (e.g., 'Traceparent') may break existing implementations
- Added valuable request context (
request.path, user ID, email) for better error diagnostics - Consider privacy implications of automatically capturing user email addresses in error reports
- Tests now use Django's
RequestFactoryinstead of mocks, providing more realistic coverage
2 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile
| traceparent = headers.get("Traceparent") | ||
| tracestate = headers.get("Tracestate") |
There was a problem hiding this comment.
logic: Header case sensitivity change may break existing integrations. HTTP headers should be case-insensitive per RFC 2616. Use case-insensitive lookup like .get('traceparent') or .get('Traceparent')
There was a problem hiding this comment.
Tests were failing with traceparent so I went with the capitalized version which works (even when specified as traceparent in the request)
| # Extract traceparent and tracestate headers | ||
| traceparent = headers.get("traceparent") | ||
| tracestate = headers.get("tracestate") | ||
| traceparent = headers.get("Traceparent") |
There was a problem hiding this comment.
Django has been normalizing header keys since 2.2 (source: ChatGPT)
We could be doing a better job at instrumenting Django requests with path and user info