Flow run logs csv download

[pleek91]

Description

Adds a new endpoint /api/flow_runs/{id}/download-logs-csv which downloads a csv of all logs a flow run created. This csv looks like

timestamp,level,flow_run_id,task_run_id,message
2024-07-22 17:58:14.037664+00:00,20,32ded098-2633-414e-90cf-242af9d6d2a8,,"Lorem ipsum http://google.com hello world https://example.com cache_key_fn, type: <class 'function'>, fn: <function task_input_hash at 0x7f165f8fba30>"
2024-07-22 17:58:15.039871+00:00,20,32ded098-2633-414e-90cf-242af9d6d2a8,,"Lorem ipsum http://google.com hello world https://example.com cache_key_fn, type: <class 'function'>, fn: <function task_input_hash at 0x7f165f8fba30>"
2024-07-22 17:58:16.080519+00:00,20,32ded098-2633-414e-90cf-242af9d6d2a8,,Finished in state Completed()

Example

Going to "api/flow_runs/{id}/download-logs-csv downloads a csv file to the browser.

Checklist

• This pull request includes a label categorizing the change e.g. maintenance, fix, feature, enhancement, docs.
• This pull request references any related issue by including "closes <link to issue>"
  • If no issue exists and your change is not a small fix, please create an issue first.
• If this pull request adds new functionality, it includes unit tests that cover the changes
• If this pull request removes docs files, it includes redirect settings in mint.json.
• If this pull request adds functions or classes, it includes helpful docstrings.

[codspeed-hq]

CodSpeed Performance Report

Merging #14703 will not alter performance

_{Comparing logs-csv-endpoint (9ad8d81) with main (85bea69)}

Summary

✅ 5 untouched benchmarks

[cicdw]

Totally understand the motivation here, and your implementation makes a lot of sense if it ran client-side. However, while True: style loops like this have big performance implication (and risk) when run server side, and in addition this could eat up a lot of memory depending on the size of the logs being requested, so I think we're going to need to consider other alternatives.

@chrisguidry or @zangell44 let me know if you disagree with my take here

[chrisguidry]

Totally understand the motivation here, and your implementation makes a lot of sense if it ran client-side. However, while True: style loops like this have big performance implication (and risk) when run server side, and in addition this could eat up a lot of memory depending on the size of the logs being requested, so I think we're going to need to consider other alternatives.

@chrisguidry or @zangell44 let me know if you disagree with my take here

It looks like this is using a StreamingResponse with a generator, so it should be okay from a memory perspective since it should only have a hard references to that 1000 logs at a time (but we should test the heck out of that)

[chrisguidry]

I think we'll be good since you're using the StreamingResponse, but can we get a unit test in for this one?

[cicdw]

Awesome, makes sense - deferring my review to @chrisguidry in that case 🫡

[pleek91]

@cicdw and @chrisguidry thank you! I'll work on some tests. Also @chrisguidry and @zangell44 curious about cloud and if this same solution would work there.

[chrisguidry]

Yep I think streaming would work just fine on Prefect Cloud too, although there are a few more layers of load balancers/proxies involved. When we get there, let's set aside some time to design a load test for it

[chrisguidry]

Looks great, thanks for that extra time on the testing!