Take in middleware factory rather than middleware

[cliu587]

PTAL @dguo-coursera, @amory-coursera - I think middlewares should be created per request rather than once on instantiation, as they hold execution state. This along with another change should unbreak the datadog tracing.

[cliu587]

not necessary :) thanks for the 2nd pair of eyes on this, @amory-coursera

[cliu587]

Reviving this - while middlewares are indeed stateless, reusing the same middleware for every request seemed to have resulted in a memory leak due to the thread-local scope data not getting cleaned up.

Here's the heap dump for the newest deploy that resulted in java heap shooting up rapidly: https://imgur.com/a/t5SH0CV (~11GB heap)

Datadog link of deploy to ~40min after deploy: https://app.datadoghq.com/dashboard/a4f-p8g-3mx/service-dash?from_ts=1561553831162&live=true&tile_size=m&to_ts=1561568231162&tpl_var_service=assembler&fullscreen_widget=58099565&fullscreen_section=overview

Here's the heap dump for the last known good deploy (which still has tracing enabled):
https://imgur.com/a/cWpNnzu (~700MB heap)

Datadog link of deploy to ~40min after deploy: https://app.datadoghq.com/dashboard/a4f-p8g-3mx/service-dash?from_ts=1561553831162&live=true&tile_size=m&to_ts=1561568231162&tpl_var_service=assembler&fullscreen_widget=58099565&fullscreen_section=overview

My best guess is because we create a singleton tracing middleware, every thread-local created has a strong reference that's never cleaned up at the end of the request.

EDIT: also added datadog links to correlate

[cliu587]

Gentle ping on this - would like to resolve by end of week if at all possible. Thanks in advance!

[amory-coursera]

This feels like something we could fix in another way, but if this is the path of least resistance I'm ok with the approach.

[cliu587]

This feels like something we could fix in another way, but if this is the path of least resistance I'm ok with the approach.

I think the existence of thread-locals in middlewares along with async execution makes all other solutions very non-obvious. Assuming this fix indeed does the job, I think it is the path of least resistance and the most obviously correct solution.