Better "capture_body" flag support

[frnkvieira]

What does this pull request do?

• Fixes "capture_body" flag not being respected causing the request.stream() of Starlette not being able to properly stream (by loading everything into memory first)
• Fixes non-UTF-8 data triggering errors in requests that should be ignored as configured.
• Improves performance of all requests (especially those who send big payloads like files) by not mocking the stream at all in case the capture_body flag is turned off (which is the default). When the flag is turned on requests are also faster because of the faster loop by using a list of bytes instead of bytes concatenation.

I'm still wrapping my head around how this agent works so my apologies in advance if I made any wrong assumptions =)

Related issues

closes #1545

[apmmachine]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Start Time: 2022-08-29T16:43:17.764+0000

• Duration: 27 min 11 sec

Test stats 🧪

Test	Results
Failed	0
Passed	4974
Skipped	3338
Total	8312

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

• /test : Re-trigger the build.

• /test linters : Run the Python linters only.

• /test full : Run the full matrix of tests.

• /test benchmark : Run the APM Agent Python benchmarks tests.

• run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

[beniwohli]

Hi @frnkvieira! Thanks for opening the PR, this looks great! From what I see, we have a pretty similar approach to handling the body in the new generic ASGI middleware (#1528).

Can you point me towards the change that ignores encoding errors? I would have suspected that you added errors="ignore" where we decode the byte string here

apm-agent-python/elasticapm/contrib/starlette/utils.py

Line 132 in 4b2eb86

return body.decode("utf-8")

[apmmachine]

🌐 Coverage report

Name	Metrics % (covered/total)	Diff
Packages	100.0% (66/66)	💚
Files	100.0% (225/225)	💚
Classes	100.0% (225/225)	💚
Lines	89.841% (17439/19411)	👎 -0.078
Conditionals	77.255% (3186/4124)	👎 -0.026

[frnkvieira]

Hi @beniwohli, to be clear, the intent of this pull request is closing #1545, but as a side effect, this fixes non-UTF-8 errors in ignored paths, only ignored ones, thanks to the ignore path checkup being done way earlier (which seems a good choice to me but maybe I'm missing something). (https://github.com/frnkvieira/apm-agent-python/blob/main/elasticapm/contrib/starlette/__init__.py#L134)

Btw, I don't think crashing on non-utf-8 data is a good thing, but I didn't feel confident enough to keep changing stuff =P
Ignore the errors may be a solution, although it would probably create some strange corner cases for people processing the APM data... encode it in base64 or similar may help preserve the original content but would create other problems... which brings me to other questions, maybe add a size limit where the content gets truncated ?... Anyway... thanks for the fast reply =)

[julianogv]

Hi @beniwohli, any idea on when you might merge this PR and publish it to pypi?
I'm currently using a forked version of elastic-apm due to this issue (and some other fixes that are already fixed/merged and not published yet).

[basepi]

@frnkvieira looks like you've turned off "allow maintainer edits" so I opened a PR instead: https://github.com/frnkvieira/apm-agent-python/pull/1

@julianogv apologies for the delay here!

[basepi]

/test

[frnkvieira]

I'm probably missing something because the flag seems to be checked.
Anyway, I just merged the PR on the fork, thanks.

[basepi]

/test

[basepi]

/test linters