fix: tail_lines in stream_logs returns last N events (true tail seman… - #6164
Merged
zhaoqizqwang merged 1 commit intoAug 6, 2026
Merged
Conversation
…tics)
- Add LogStreamer.poll_tail() for fetching last N events:
- Stream mode (SMTJ): backward pagination via get_log_events nextBackwardToken
- Filter mode (SMHP): filter_log_events with startFromHead=False, paginate
until N matches collected (CW bounds pages by scan volume, not result count)
- Multi-stream jobs: merge events across streams by timestamp, return globally last N
- stream_log_loop: when tail_lines is set, call poll_tail() and return immediately
- Refactor _stream_logs_smhp to delegate to LogStreamer + stream_log_loop,
eliminating ~80 lines of duplicated inline polling logic
- Validate start_time >= 2024-01-01 in _tail_filter_mode (CW API restriction)
- Remove dead code: lines_printed counter no longer needed in SMHP forward loop
- Add unit tests for poll_tail (stream mode, filter mode, multi-stream merge,
multi-page pagination, pre-2024 validation)
- Add unit tests for stream_log_loop tail_lines integration
cmahima
approved these changes
Aug 6, 2026
amazeAmazing
marked this pull request as ready for review
August 6, 2026 18:02
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.
Issue #, if available:
N/A
Description of changes:
Issue
stream_logs(tail_lines=N)prints the first N log lines (head behavior) instead of the last N (tail behavior). Users expecttail_linesto work liketail -norkubectl logs --tail— showing the most recent output.Description of changes
Bug fix: true tail semantics for
tail_linesWhen
tail_linesis set,stream_logs()now fetches the last N log events from CloudWatch and returns immediately.SMTJ path (
get_log_events): Uses backward pagination vianextBackwardTokento read from the end of each log stream. For multi-stream jobs (distributed training), merges events across all streams by timestamp and returns the globally last N.SMHP path (
filter_log_events): UsesstartFromHead=Falsewith pagination. CloudWatch bounds pages by bytes scanned (not matches found), so we follownextTokenuntil N matching events are collected.Refactor: eliminate duplicated SMHP inline loop
_stream_logs_smhppreviously had ~80 lines of inline polling logic that duplicated whatLogStreamer+stream_log_loopalready provide. Replaced the inline loop with aLogStreamerinstance in filter mode, delegating tostream_log_loopfor consistent behavior across all paths (SMTJ, SMHP, MTRL).Simplify
_print_eventand remove dead counterPreviously,
_print_eventhad a dual role: print the log line AND check alines_printedcounter to stop attail_lines. Callers usedif _print_event(...): returnto exit the loop when the limit was reached. This was the old "head" behavior (stream forward, stop after N).Now that
tail_linesis handled before the loop viapoll_tail()(which fetches the last N events and returns immediately), the streaming loop only runs whentail_linesis None. The counter and conditional return are dead code — they can never trigger._print_eventis now a simple print helper with no return value.The limit is enforced inside
poll_tail():_tail_stream_mode): passeslimit=Ntoget_log_eventsper stream, then merges across streams and slices to the globally last N withall_results[-n:]._tail_filter_mode): accumulates events from paginatedfilter_log_eventscalls and stops whenlen(results) >= n, then slices toresults[:n]and reverses to chronological order.Validation
_tail_filter_moderaisesValueErrorifstart_timeis before 2024-01-01 — CloudWatch requires this forstartFromHead=Falseonfilter_log_events.Testing
poll_tail(stream mode, filter mode, multi-stream merge, multi-page pagination, pre-2024 validation, stream_log_loop integration)test_log_streamer.pypassFiles changed
sagemaker-train/src/sagemaker/train/common_utils/log_streamer.pypoll_tail(),_tail_stream_mode(),_tail_filter_mode(). Updatestream_log_loopto usepoll_tailwhentail_linesis set.sagemaker-train/src/sagemaker/train/base_trainer.py_stream_logs_smhpinline loop withLogStreamer+stream_log_loop. Remove unused imports (os,ClientError,MultiLogStreamHandler).sagemaker-train/tests/unit/train/test_log_streamer.pyBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.