Warning about Channels memory leakage unclear

[vanschelven]

Core or SDK?

Platform/SDK

Which part? Which one?

python (django (channels))

Description

In the Python troubleshooting section it says

A Django application using Channels 2.0 will be correctly instrumented under Python 3.7. For older versions of Python, install aiocontextvars from PyPI or your application will not start.

If you experience memory leaks in your channels' consumers while using the SDK, you need to wrap your entire application in Sentry's ASGI middleware. Unfortunately the SDK is not able to do so by itself, as Channels is missing some hooks for instrumentation.

This is confusing on a number of levels:

1. "will be correctly instrumented" and "memory leaks" are in direct contradiction.
2. "if you experience memory leaks"... from the text it is not clear why such leaks could be expected, but it does seem clear that the author does indeed expect it. Also, "if" seems a bit misplaced here (in general): either the application is expected to leak memory, or it is not. The "if" here may be technically correct, in the sense that "experiencing" memory leaks may only happen if you have less memory available than you're leaking... but that wouldn't seem like a good reason to include it :-)
3. The instructions for wrapping in ASGI middleware are quite implicit. Should this be done in addition to the regular sentry_sdk.init, or instead of it?
4. In the linked issue it is mentioned that "Be that as it may, Django 3 will have the signals we need, so I guess time will solve this." but it is not clear whether those "signals we need" have actually been made use of since their creation.

@untitaker probably knows most about this subject

Suggested Solution

Either remove this section (if the problem has since been solved) or provide clarification on the points 1 - 4.

[getsantry]

Assigning to @getsentry/support for routing, due by Thursday, February 9th at 09:00 (yyz). ⏲️

[untitaker]

"will be correctly instrumented" and "memory leaks" are in direct contradiction.

there are a lot of nuances to how software can fail. you can get good instrumentation data and still have the SDK cause memory leaks.

"if you experience memory leaks"... from the text it is not clear why such leaks could be expected, but it does seem clear that the author does indeed expect it. Also, "if" seems a bit misplaced here (in general): either the application is expected to leak memory, or it is not.

the entire premise of this page is to help the user figure out, (or at least work around) spurious, deployment-specific issues with the SDK to which there is no single answer (or at least not a single googleable one)

of course the SDK is supposed to not leak memory in your application. if you end up on this page you're already having a bad time with sentry.

Should this be done in addition to the regular sentry_sdk.init, or instead of it?

In addition, and only when you experience that issue, as a potential workaround.

In the linked issue it is mentioned that "Be that as it may, Django 3 will have the signals we need, so I guess time will solve this." but it is not clear whether those "signals we need" have actually been made use of since their creation.

django 3 has been released with async support, and the SDK gained support it since then. the signals i am referring to have always been present in django, and as such they are also available for async views.

I have found it hard to implement any sort of integration with django channels due to those missing signals and as such, instrumentation for applications using django channels is lacking or almost non-existent (for the particular areas where they use channels to handle requests)

Is there a particular problem you're trying to solve?

[vanschelven]

you can get good instrumentation data and still have the SDK cause memory leaks.

fair enough.

the entire premise of this page is to help the user figure out, (or at least work around) spurious, deployment-specific issues with the SDK to which there is no single answer (or at least not a single googleable one)

of course the SDK is supposed to not leak memory in your application. if you end up on this page you're already having a bad time with sentry.

still, the inclusion of this note in the docs seems to indicate that [a] someone had a mental model of how such leaks might occur in practice, or [b] that actual people came forward with leaks that they had observed in practice. It would be helpful to know more about either of those cases to be able to determine beforehand whether my setup will start leaking memory before I actually deploy it.

In addition, and only when you experience that issue, as a potential workaround.

Clear

In the linked issue it is mentioned that "Be that as it may, Django 3 will have the signals we need, so I guess time will solve this." but it is not clear whether those "signals we need" have actually been made use of since their creation.

django 3 has been released with async support, and the SDK gained support it since then. the signals i am referring to have always been present in django, and as such they are also available for async views.

In the linked issue you say "Be that as it may, Django 3 will have the signals we need, so I guess time will solve this." This was the final word on that issue. you requested such signals in the linked issue, presumably with an idea to hook up some behavior to them (to avoid memory leaks?). By now, the signals exist. It is not clear to me whether that presumption is correct, and if so whether any hooking-up was done.

Is there a particular problem you're trying to solve?

The documentation contains a warning about a situation I'm trying to avoid. I'd rather understand whether the documentation is up-to-date, and whether it applies to my situation, before deploying my live application, than deploy and then running into some trouble.

[sl0thentr0py]

hey @vanschelven thanks for the feedback. These docs haven't been touched in a while.
I will remove the memory leak section because it just adds to confusion, there are a couple of github issues around if people want historical information on the issue.

re: the memory leaks, I'm not sure if this is an issue anymore as well since we seem to wrap with AsgiMiddleware when we patch channels, seems to me like some historical leftover either way.

[getsantry]

Routing to @getsentry/team-web-sdk-backend for triage, due by Friday, February 10th at 5:00 pm (sfo). ⏲️