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

First retry implementation #1

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

First retry implementation #1

wants to merge 15 commits into from

Conversation

nicklas-dohrn
Copy link
Owner

Description

Please include a summary of the change.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Testing performed?

  • Unit tests
  • Integration tests
  • Acceptance tests

Checklist:

  • This PR is being made against the main branch, or relevant version branch
  • I have made corresponding changes to the documentation
  • I have added testing for my changes

If you have any questions, or want to get attention for a PR or issue please reach out on the #logging-and-metrics channel in the cloudfoundry slack

This addresses all the comments by @ctlong.
It fixes the unneded if else branching for adding to the message
batch which is just not needed anymore.
It fixes the egressMetric to behave similar to the single message
implementation, to not count erroneous logs.
The refactor is mainly reshuffeling

The new timer implementation makes it more clear what the actual logic
is, and might also prevent some unresolvable states.
It now only has two states:
- Running if a batch is not yet full or time triggered
- Not running if there was a batch send either through a time or a size
based trigger
This is a new approach to switch between http and http batching.
It only is different in this regard from the previous attempts,
and only contains refactorings besides this change.
@nicklas-dohrn nicklas-dohrn force-pushed the main branch 10 times, most recently from 5e8242f to b73bec2 Compare December 13, 2024 13:27
@nicklas-dohrn nicklas-dohrn force-pushed the main branch 2 times, most recently from f0d7e6c to fb5fe34 Compare December 29, 2024 11:23
@nicklas-dohrn nicklas-dohrn force-pushed the main branch 3 times, most recently from 1f2a122 to e7e9fba Compare February 24, 2025 15:26
@nicklas-dohrn nicklas-dohrn force-pushed the main branch 2 times, most recently from 5d67c23 to 7a5fab6 Compare February 27, 2025 12:52
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.

1 participant