fix: use timezone-aware UTC for the default cloudwatch end_time - #3438
Open
hsusul wants to merge 1 commit into
Open
fix: use timezone-aware UTC for the default cloudwatch end_time#3438hsusul wants to merge 1 commit into
hsusul wants to merge 1 commit into
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.
Feature or Bugfix
Detail
wr.cloudwatch.start_query(and thereforerun_query/read_logs) computes its defaultend_timewithdatetime.datetime.utcnow(), which returns a naive datetime. The verynext line calls
.timestamp()on it, anddatetime.timestamp()interprets a naive datetimeas local time. The resulting epoch value is therefore shifted by the machine's UTC offset.
The default
start_timeright above it already uses an aware value(
datetime.datetime(1970, 1, 1, tzinfo=datetime.timezone.utc)), so the two boundaries of thesame query window are converted under two different assumptions.
awswrangler/cloudwatch.py(before):Reproduction (no AWS credentials needed)
Current vs corrected behavior
endTimesent to CloudWatch Logs on a UTC+9 hostendTimesent on a UTC hoststart_time=now-5minwith defaultend_timeon a UTC+9 hostInvalidArgumentCombination: start_time must be inferior to end_time.So on any host whose local timezone is ahead of UTC,
wr.cloudwatch.read_logs(...)without anexplicit
end_timesilently drops the most recent offset-worth of log events, and combining arecent
start_timewith the defaultend_timefails outright. On hosts behind UTC the window isextended into the future instead.
Implementation
One line in
awswrangler/cloudwatch.py:This also removes the last use of
datetime.datetime.utcnow()inawswrangler/, which isdeprecated since Python 3.12.
No public API, signature, default, or return type changes. Explicitly supplied
start_time/end_timevalues are converted exactly as before.Regression tests
Added to
tests/unit/test_moto.py(the module theMinimal Testsworkflow runs), plus alocal_timezonefixture that setsTZ/time.tzset()and restores the previous valueafterwards. It skips where
time.tzset()is unavailable (Windows) or where the requested zoneis missing from the timezone database.
test_cloudwatch_start_query_default_end_time_is_utc_now— theendTimesent tologs:StartQueryis the current UTC instant underAsia/TokyoandAmerica/New_York, andthe default
startTimeis still0.test_cloudwatch_start_query_recent_start_time_with_default_end_time— astart_timefiveminutes in the past no longer raises against the default
end_time.test_cloudwatch_start_query_explicit_times_are_preserved— explicit aware boundaries arestill converted straight to epoch milliseconds (no behavior change).
Against the unpatched module these fail as expected: both parametrizations of the first test, and
the
Asia/Tokyocase of the second (theAmerica/New_Yorkcase correctly passes, since atimezone behind UTC moves the default
end_timeinto the future rather than the past).Validation
Run locally on macOS 15.6 / Python 3.13.5,
AWS_DEFAULT_REGION=us-east-1:pytest tests/unit/test_moto.py -k cloudwatchpytest tests/unit/test_moto.py -k cloudwatchpytest -n 4 tests/unit/test_moto.pypytest tests/unit/test_moto.py tests/unit/test_utils.py tests/unit/test_session.py tests/unit/test_metadata.pyruff format --check .ruff check .mypy awswranglerupstream/main(pre-existing, all in unrelated modules)doc8 --max-line-length 120 docs/sourceuv lock --checkuv build --wheelawswrangler-3.17.1-py3-none-any.whlgit diff --checkNot run: the AWS integration tests (
tests/unit/test_cloudwatch.pyand the rest of thecredential-dependent suites), which need a live account and the CDK test infrastructure. The
behavior changed here is the local timestamp conversion, which the added tests cover without
any AWS call.
Relates
utcnowargument instart_query#2193 / cloudwatchstart_queryandrun_queryend_timegoes stale #2185, which moved this default from importtime to call time but kept the naive
utcnow(); this PR fixes the timezone conversion itself.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.