timing-asgi icon indicating copy to clipboard operation
timing-asgi copied to clipboard

Expected behavior with BackgroundTask

Open adriangb opened this issue 2 years ago • 4 comments

What is the expected behavior when dealing with background tasks? Should the time it takes to execute them be included or excluded from the timings?

adriangb avatar Sep 24 '22 05:09 adriangb

Sorry for the late reply @adriangb but this is currently the expected, but obviously not terribly helpful, behaviour.

I have made some plans which I partially explain in the now-close-to-3-year-old issue here: https://github.com/steinnes/timing-asgi/issues/15

I've got some vacation coming up, where I hope to find the time to complete the refactor to emit two timings, one for the "response", which then does not include the background task, and another for the "request" which includes the background task time.

I am open to suggestions for naming these things better, but the diagram in the link above should explain rather well what I have in mind.

steinnes avatar Nov 09 '22 17:11 steinnes

Well I'm just curious if it's a wanted or unwanted feature. We could change how background tasks work in Starlette or at least 3rd party libraries, but I want to understand how this would impact things like this middleware. In other words, if background tasks ran outside of the ASGI request / response and thus it was not possible to time them with an ASGI middleware, would that be... a good thing?

adriangb avatar Nov 09 '22 17:11 adriangb

I did not expect them to be included when I wrote this, although I had every opportunity to read the source and be aware of it, it did come as a surprise. So to me it was an unwanted feature, but I obviously have not been hard pressed to do anything about it, just look at the age of the issue pointing it out 😁

If you go ahead and change it, I suppose it would be nice to have a way to either wrap their calls or hook into when they complete, as I wouldn't be surprised some users would prefer the functionality to stay the same, which is why I had plans to be able to emit timings for both time-until-response and total time of the request.

steinnes avatar Nov 09 '22 19:11 steinnes

Noted, I think it should be pretty easy to provide a telemetry hook for background tasks! To be concrete, I'm thinking about something like this: https://github.com/adriangb/asgi-background

adriangb avatar Nov 09 '22 19:11 adriangb