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

senders: Mitigate issue with GCP logging quota #171

Merged
merged 1 commit into from
Jun 26, 2024

Conversation

italomg
Copy link
Collaborator

@italomg italomg commented Jun 14, 2024

https://cloud.google.com/logging/docs/reference/v2/rest/v2/entries/write
https://cloud.google.com/logging/quotas

WHAT

This PR is doing two things:

  1. Not retrying to send logs that return with HTTP status code 4xx. Instead those logs are simply skipped.
  2. Filtering out any LogEntry that is bigger than 256K

@italomg italomg force-pushed the italo.garcia/mitigate-logentry-quota branch 4 times, most recently from df36f93 to 76c1461 Compare June 19, 2024 10:12
@italomg italomg marked this pull request as ready for review June 19, 2024 10:16
Copy link
Contributor

@dogukancagatay dogukancagatay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, just a few suggestions.

journalpump/senders/google_cloud_logging.py Outdated Show resolved Hide resolved
journalpump/senders/google_cloud_logging.py Outdated Show resolved Hide resolved
journalpump/senders/google_cloud_logging.py Outdated Show resolved Hide resolved
GCP quotas page for the logging v2(https://cloud.google.com/logging/quotas) specifies
that a LogEntry should have at maximum 256KB in size, that is not a hard limit, but
an estimation.
In anycase we should check if any of our LogEntry objects are bigger than the quota
allows and not send those logs. Instead we try to truncate the message to smaller size
and if that isn't enough we send a default log message.
@italomg italomg force-pushed the italo.garcia/mitigate-logentry-quota branch from 76c1461 to 95d94d7 Compare June 21, 2024 13:37
@codecov-commenter
Copy link

codecov-commenter commented Jun 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.47%. Comparing base (a71f5aa) to head (95d94d7).
Report is 5 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #171      +/-   ##
==========================================
+ Coverage   63.32%   63.47%   +0.15%     
==========================================
  Files          19       19              
  Lines        2094     2100       +6     
==========================================
+ Hits         1326     1333       +7     
+ Misses        768      767       -1     
Files Coverage Δ
journalpump/senders/base.py 80.44% <ø> (+0.10%) ⬆️
journalpump/senders/google_cloud_logging.py 91.66% <100.00%> (+1.34%) ⬆️

... and 2 files with indirect coverage changes

Copy link
Contributor

@dogukancagatay dogukancagatay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dogukancagatay dogukancagatay merged commit c62e2ea into master Jun 26, 2024
6 checks passed
@dogukancagatay dogukancagatay deleted the italo.garcia/mitigate-logentry-quota branch June 26, 2024 08:37
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

Successfully merging this pull request may close these issues.

4 participants