Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Truncate beginning of long request bodies instead of the end #939

Open
StevenMassaro opened this issue Jan 8, 2024 · 5 comments
Open

Truncate beginning of long request bodies instead of the end #939

StevenMassaro opened this issue Jan 8, 2024 · 5 comments

Comments

@StevenMassaro
Copy link

I've started making use of the feature to send log data to healthchecks. I like that I get a truncated copy of the logs in my notification provider.

Under most circumstances I can imagine, an error in the script would show up at the end, and typically this error is truncated away.

I'm proposing a change to the truncating logic here, something like this:

    if body and maxlen and len(body) > maxlen:
        body = body[-maxlen:] + "\n[truncated]"

but I wanted to get feedback first before spending time on this change.

This would ensure that the end of the log message is included in the notification, and the beginning of the log message is omitted.

@cuu508
Copy link
Member

cuu508 commented Jan 8, 2024

Hi @StevenMassaro, thanks for the suggestion.

We truncate the request bodies for two main reasons:

  1. to prevent storage from ballooning
  2. to make the handling of each individual ping request quick and efficient

For (1) it would not matter if we keep the head or the tail part of the request body.
For (2) it does matter: suppose the client sends 1GB request body, we process it all and keep the last 100kB. This would be a waste of time, bandwidth and processing power, and would complicate the code (loading the entire request body in memory would be a bad idea, so body = body[-maxlen:] would not work). This is an extreme example but even sending 10MB and keeping last 100kB would not be great.

I suggest doing one of the following:

  • make your log output less verbose so it fits in 100kB
  • do the truncation on the client side. runitor, for example, does this out of the box: it truncates the log output on the client side, and sends the tail not the the head part to the server.

@StevenMassaro
Copy link
Author

Hi @cuu508

I think I must be missing something with regards to your #2.

As the code currently reads to me, it looks like the body is loaded into memory on line 67, then truncated later. Thus, it shouldn't matter whether we retain the head or tail of the body, since it is already loaded into memory.

Let me also clarify my goal here. I am already doing truncation on the client side, as described in the documentation. My problem is that the request body is further truncated when a message is sent via telegram (see code here), and it is omitting the error message, because it is retaining the head, not the tail of the message:

image

@cuu508
Copy link
Member

cuu508 commented Jan 8, 2024

Ah, sorry, I did not read carefully and misunderstood – I was thinking about truncation when ingesting a ping request, but you were talking about truncation when preparing a notification. I'll look into this again.

@StevenMassaro
Copy link
Author

@cuu508 Wondering if you've had a chance to ponder this request

@moraj-turing
Copy link
Contributor

Hey @StevenMassaro I have since created a PR including settings for this behavior

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants
@cuu508 @StevenMassaro @moraj-turing and others