Make django middleware support 404 URL grouping based on sentry's fingerprint option

[fle]

Hi,

Sentry provides a grouping feature through a fingerprint argument : Customize Grouping with Fingerprints.

Raven supports fingerprint since this commit : 0075d95.

This allows to group similar 404 URLs based on defined patterns in django settings.

A dummy example :

GROUPING_404_URLS = (
    (re.compile(r'^/api/old/items/(?:\d+)/$'), ['old', 'url', 'pattern']),
)

allows to group in sentry all URLs matching this regex.

[fle]

I should add tests by the way

[fle]

Updated PR with some basic tests

[mattrobenolt]

Just throwing in my two cents, I'm not a big fan of this implementation. I'd rather see this be done as a callback style thing. Maybe you want to use arguments from the url in the fingerprint, etc? I think that processing should be opened up with a hook.

Also, as note, the re.compile is a bit anti-Django patterns. It'd be customary to just specify the string for the regexp, then inside the module, compile them. Similar to how the url patterns work.

Aside from that, I'm -0 on this feature as a whole. I feel like this would be better suited as just your own middleware that did whatever logic you want, rather than adding in an obscure hook like this imo. With that said, I'm not strongly against it and could be convinced otherwise.

[fle]

Got it @mattrobenolt.

I tried to stay near the existing handling of ignorable urls in this middleware and the django implementation of IGNORABLE_404_URLS (re.compile).

Go with my own middleware is OK for me too ;)

[mattrobenolt]

@fle To be clear, I was mostly unhappy with this implementation. I think there's value in doing this as a hook on the Sentry404CatchMiddleware middleware itself. That way you can extend this middleware and just hook in your own behavior.

So maybe something like:

class Sentry404CatchMiddleware(object):
    def get_fingerprint(self, request):
        return None

    def process_response(self, request, response):
        # ...
        fingerprint = self.get_fingerprint(request)
        if fingerprint is not None:
            # Add fingerprint into the kwargs

This would allow a subclass to just need to do:

class MySentry404CatchMiddleware(Sentry404CatchMiddleware):
    def get_fingerprint(self, request):
        return ['lol']

And do whatever logic someone wants there, instead of configuring through your settings.py.

Make sense?

[mattrobenolt]

Also, I guess you are right about how IGNORABLE_404_URLS was implemented. :) I've never once used that. I still don't think that pattern works well here fwiw. I think it'd be better suited as a hook like here.

This might even be useful to just have get_extra_data or something as a hook instead of just get_fingerprint so you could then spit back a dictionary that gets merged in and you can provide anything.

[fle]

I totally agree.