search

getsentry / sentry-python

The official Python SDK for Sentry.io
https://sentry.io/for/python/
MIT License
1.93k stars 510 forks source link

[DJANGO] Support sensitive_variables decorators in integration #2356

Open ryshu opened 1 year ago

ryshu commented 1 year ago

Problem Statement

In the Django framework, a utility exists to declare that a variable is sensitive or not.

https://docs.djangoproject.com/en/3.2/_modules/django/views/decorators/debug/

Would it be possible to support this functionality in the integration directly?

Solution Brainstorm

The SDK tries to read the sensitive_variables attribute on the functions it passes to add them to sensitive information not to be displayed clearly in sentry.

antonpirker commented 1 year ago

Hey @ryshu thanks for bringing this up. Sounds like a good idea. I put this on our internal backlog, but as we are a bit swamped right now it will take some time until we pick this up.

But PRs are always welcome if you want to give it a go!

ryshu commented 1 year ago

Hello @antonpirker,

Glad to hear it.

I'm not against helping you but I currently don't know anything about the internal workings of the package and the event scrubber so I don't think I'll be of much help to you on the issue.

If you have the time to give me some keys to a possible technical solution, I could look into implementing it later.

antonpirker commented 1 year ago

Hey @ryshu

That's fine, we just leave the issue here open and if I find some time to look into this (I also need to read some code to find out how to do this) I eventually ping you again! Thanks!

vagi8 commented 1 year ago

Hey @antonpirker , I wanted to contribute and I have taken a look at the Django integration and Django debug sensitive variables code.

  1. sensitive_variables decorator is used to declare the sensitive variables (SV).
  2. The SV keys are stored as decorator variables as sensitive_variables_wrapper.sensitive_variables = variables ,

we can loop through each exception frame to check if the decorator frame exists. Then extract all sensitive variables and hide them. I was also thinking its better to hide the sensitive_variable decorator frame as well.

All these have to be implemented in utils.serialize_frame which is not very specific to django integration. Need some suggestions on how the implementation should be. The only way to extract the declared sensitive variables is from the decorator frame.

Possible solution - we can use mechanism.type == django as a way to implement specifically for django integration inside utils but this feels bad.

antonpirker commented 1 year ago

Hey @vagi8 . Yea, we do not want to have code specifically for Django in utils. That is a no go.

I guess we can just say, that we can not support this at the moment. Thanks for looking into this @vagi8 this really helped taking some of the load from our shoulders!

vagi8 commented 1 year ago

Hey @antonpirker,

I have raised a PR for this.

antonpirker commented 1 year ago

Hey @vagi8 ! Wow, thanks for the PR! We will have a look at it. I can not promise any ETA yes, because right now PRs are kind of piling up :-)