fix(k8s-logs): honor the requested end time in agent log queries - #229
Merged
Conversation
… returning today's lines
sebasnallar
previously approved these changes
Aug 18, 2026
…to a log page The pagination token was regenerated from the entries of the current page alone, so an instance with no new lines — or whose remaining lines fall past end_time — lost its position. determineSinceTime then fell back to start_time on the next page and the instance re-read the window from the beginning, re-delivering lines the caller had already seen. With more than one instance the pages take turns evicting each other and pagination never reaches the end of the range. GenerateToken now carries the incoming cursors forward. An empty page still yields an empty token: that is how the caller learns there are no more pages, so the cursors are deliberately not carried across it. The ordering, the limit cut and the token move into pagination.Page. The token records the newest entry kept per instance, and that is the cut point only because the entries are sorted ascending and the cut keeps the oldest ones — keeping the three steps in one function documents that coupling and makes it testable outside package main. Linux binaries rebuilt. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The multi-page test walks a window page by page through the production functions — determineSinceTime, getLastReadTime, ProcessLinesFromChannel and pagination.Page — and fakes only the Kubernetes stream, which is the one thing that cannot run here. It asserts that paging terminates, that every line in the window is delivered exactly once and in order, and that a line past end_time is never delivered. One of its instances has no lines inside the window at all: that instance never earns a cursor and is re-read on every page, which is inherent to a timestamp cursor and cheap because the processor stops at its first line past the window. Both tests fail against the previous behavior — the multi-page one never terminates and delivers some lines five times. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The branch had accumulated one entry per commit for what a reader sees as a single fix: the range you select is the range you get. They collapse into one entry that names the symptoms first and the resulting behavior second. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ignacioboud
previously approved these changes
Aug 18, 2026
sebasnallar
previously approved these changes
Aug 18, 2026
…dencies govulncheck reported 40 vulnerabilities in the committed binary — 31 from the standard library, 8 from golang.org/x/net, 1 from golang.org/x/text — of which 17 are reachable from this code: TLS handshake and x509 chain-building issues on the API server connection, net/url parsing, and an infinite loop in x/text. The x/net/html findings that Dependabot flags for this module are present but not reachable, since the binary never parses HTML. The standard library is linked into the binary, so the binary carries the standard library of whatever compiler built it, and the go directive was a floor low enough that a machine ten patch releases behind satisfied it. Raising the directive to the version that fixes these makes the go command fetch a matching toolchain, so a rebuild cannot quietly ship an older standard library again. x/net and x/text move to their fixed versions; x/sys and x/term follow as their requirements. Verified: govulncheck reports no vulnerabilities in source mode and against all three rebuilt binaries, and go vet, go test and the bats suite are clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The loose '1.25' let setup-go keep the Go that ships with the runner image, which is behind the version go.mod now requires, and setup-go pins GOTOOLCHAIN=local so the go command is not allowed to fetch the one it needs: `go vet ./...` failed with "go.mod requires go >= 1.25.13 (running go 1.25.12; GOTOOLCHAIN=local)". Pinning the exact version keeps the checks on the same toolchain the shipped binaries are built with, which is the point of raising the directive in the first place. The shared workflow cannot read this module's go.mod instead — its setup step hardcodes go.mod at the repository root, and this module lives in a subdirectory — so the version lives in two places and has to move in both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The entry had grown into a paragraph about how the fix works. This file is the source for the customer release notes, so it should say what changed for the user and nothing about the mechanism. The bound-validation line is dropped: an error message for an input the console does not let you type is not release-note material. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fedemaleh
reviewed
Aug 18, 2026
fedemaleh
reviewed
Aug 18, 2026
fedemaleh
reviewed
Aug 18, 2026
fedemaleh
reviewed
Aug 18, 2026
Review asked for it: most of these restated the code or narrated a decision, and this is a public repository. What is left is one or two lines each, kept only where a reader would otherwise have to reconstruct something — why the upper bound is applied client-side, why the stream read breaks instead of continuing, what previous is for in the token, why the producer needs cancelling, and in bash the numeric guard before bc and the SECONDS rename. The comment above the bound validation went entirely: it described what would happen without the check while sitting next to the exit(1) that prevents it, which reads as a contradiction. The bats banner comments stay as they are, they match every other test file in the repo. Binaries rebuilt: dropping lines moves the line numbers recorded in the binary. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review asked for it. The version and the file it has to match are visible in the diff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fedemaleh
approved these changes
Aug 18, 2026
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
4 participants
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.
Why
Log queries from agent-backed scopes ignore the end of the requested range, so the console shows the newest lines no matter which window the user picks. Reported in production for several orgs today ("it filters as if it were today", "stuck on the 18th", "if I set
afterit shows me today's logs, if I set the same day it shows nothing").Only orgs on an agent are affected, and this is why: for those scopes the platform does not query the logs itself, it relays the request to the agent, which runs the scripts in this repo. The range is dropped here, not upstream.
build_contextextracts every filter except the upper bound:logtherefore only ever passes--start-time, andkube-logger-gohas no--end-timeflag to pass it to. The query is effectively[start, ∞), and since the Kubernetes API only bounds log reads from below (PodLogOptions.SinceTimehas no counterpart), it returns whatever the live pods currently hold — today's lines.This is not a regression. The initial implementation (June 2025) already extracted only
start_time, the previous bash logger only ever had a--start-timeflag, and the Go rewrite carried the gap forward. The upper bound has never been wired through; it surfaces now as more orgs move onto agent-backed scopes.What changed
kube-logger-go: new--end-time/-eflag andConfig.EndTime. The bound is applied in the processor over streamed lines, using the same lexicographic RFC3339 comparison the existinglastReadTimecheck uses, because the Kubernetes API cannot bound from above.build_context: extractsEND_TIME.log: converts and passes--end-time.Two incidental fixes in the same conversion path:
date/awkstrategy failed, silently querying a different window than the one requested. It now fails with a message instead. This is the one intentional behavior change.SECONDSis a bash special variable whose value is the elapsed time since it was last assigned, so the converted timestamp drifted as the script ran. Renamed.Verification
go build ./...andgo vet ./...clean; two new tests cover the bound being applied and the no-bound case (first Go tests in this package).make build;--end-timeconfirmed present in the rebuilt binary. Darwin binaries are not tracked in git and were not added.build_context+ the conversion exercised end to end on Alpine, matching the agent image:Rollout
These scripts run on customer agents from this repo, so this reaches users as a scope implementation update rather than a platform deploy — worth coordinating with the affected orgs rather than assuming it lands on merge.