Repository navigation
fix(timeShift): honor alignDST in the render timezone - #587
Merged
Merged
Conversation
emadolsky
approved these changes
Oct 28, 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
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.
What issue is this change attempting to solve?
Why
timeShiftadvertisesalignDST, but ignores it. Weekly traffic comparisons therefore compare different local hours after a daylight-saving transition and can trigger false alerts. Passingtrueas the fourth argument currently changes nothing.The render timezone must govern this comparison: a server can run in UTC while carbonapi is configured for a regional timezone, and a request can supply its own
tz.How does this change solve the problem? Why is this the best approach?
Architecture
The HTTP render handler carries the request timezone in the evaluation context, falling back to configured
tz, then the local timezone. One sharedTimeShiftOffsethelper calculates the correction for both backend fetch planning and expression evaluation. This keeps the fetched historical window and the timestamps used to read it from the metric map identical.Fetch planning now receives absolute request bounds and visits outer time transforms before their arguments. Nested shifts, moving windows, and
timeStacktherefore calculate each historical comparison against the dates at that level. ExistingMetrics()callers retain relative requests without timezone correction.The evaluator/backend boundary and storage protocol stay in place. This implements the advertised graphite-web compatibility option within the existing function, without adding another backend or a separate query path. Offset differences are measured in seconds, including regions with half-hour DST changes.
Before / after
timeShift(source,'7d',true,true)always fetches 168 hours earlier.Europe/Amsterdam, it fetches 169 hours earlier across autumn DST and 167 across spring DST.alignDST; positional arguments and bothalignDST/alignDstnamed spellings work.alignDSTand UTC/fixed-offset zones preserve fixed-duration shifts.Usage
Existing fourth-argument consumers activate the implementation immediately. For a weekly alert baseline, send
tz=Europe/Amsterdamwith:The production validation exercised an existing placement-traffic dashboard expression that already uses this argument. The README includes request and three-week-baseline examples.
As in graphite-web, one correction is applied only when neither range spans a timezone transition. This does not resample missing/repeated local hours point by point. The existing
resetEndlimitation remains.How can we be sure this works as expected?
make test TAGS='-tags cairo,nativegzip': complete race suite.timeStack.169h/337h/505hworkaround at all 60 timestamps, with 60 non-null values. All 60 changed from the incorrect fixed-week comparison; ordinary fixed-shift output matched the running service.Rollout / rollback
The temporary instance was stopped and its build artifacts removed after validation. The running production service retained its original PID and executable and returned HTTP 200 before and after validation. This PR does not deploy the fleet or edit alerts. Consumers opt in with
alignDST=true; disabling that option restores fixed-duration behavior.