Repository navigation
fix: log the full timer ID instead of dropping its last character - #119
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The broker's access logger dropped the last character of every timer ID.
TimersParamsStoragepassedmessage_id_ln=-1to FastStream'sget_broker_logger, and FastStream'sExtendedFilterslicesrecord.message_id[:message_id_ln]. A slice of[:-1]cuts the final character, sot-149was logged ast-14andt-3ast-.Found while running the Sentinel failover drills (#117). The drill handled 150 timers,
t-0tot-149, each exactly once, but the "Received" and "Processed" lines showed only 15 distinct IDs, 10 times each:t-,t-1tot-9, andt-10tot-14. Delivery was fine. The log context was correct too, and concurrent consumes do not leak context into each other. Only the formatting was wrong, but it made the logs useless for following a single timer during an incident.The fix passes
sys.maxsize, so IDs are never truncated, and drops the width from%(message_id)sin the format string. The old-1read as "no limit" for both truncation and padding. I kept that intent and did not adopt FastStream's 10-character limit: timer IDs are user-chosen, often UUIDs, and a 10-character prefix would make different timers look alike in the logs.tests/test_log_context.pypublishes 12 timers withmax_concurrent=4and checks that the "Received" and "Processed" records name each timer ID exactly once. It failed before the fix with the truncated IDs.