diff --git a/src/AGENTS.md b/src/AGENTS.md index 9a5ad3fd7087d3..a3d380e1265c54 100644 --- a/src/AGENTS.md +++ b/src/AGENTS.md @@ -295,6 +295,76 @@ class MultiProducer: return KafkaProducer(build_kafka_configuration(default_config=config)) ``` +### DateTime and Timezone Handling + +**CRITICAL**: All datetime objects in Sentry MUST be timezone-aware and use UTC. + +```python +# CORRECT: Always use UTC explicitly +from datetime import UTC, datetime + +now = datetime.now(UTC) +specific_time = datetime(2024, 1, 1, 12, 0, 0, tzinfo=UTC) + +# CORRECT: Use Django's timezone utilities +from django.utils import timezone + +now = timezone.now() # Returns aware datetime in UTC + +# WRONG: Never create naive datetimes +now = datetime.now() # NO! Creates timezone-naive datetime +specific_time = datetime(2024, 1, 1, 12, 0, 0) # NO! Missing tzinfo +``` + +**Why This Matters:** + +- Sentry is a distributed system serving users worldwide +- Database is configured with `TIME_ZONE = "UTC"` +- Naive datetimes cause subtle bugs in timezone conversions +- Django's `USE_TZ = True` requires timezone-aware datetimes + +**Current Enforcement:** + +UTC datetime enforcement is implemented through multiple layers: + +1. **Django Runtime Warnings**: `USE_TZ = True` in Django settings provides runtime warnings for naive datetimes +2. **Flake8 Rule (S015)**: Custom flake8 rule detects: + - `datetime.now()` without arguments → must use `datetime.now(UTC)` + - `datetime.utcnow()` (deprecated pattern) → must use `datetime.now(UTC)` +3. **Pre-commit Hooks**: The flake8 hook runs automatically on commit +4. **Helper Utilities**: `src/sentry/utils/dates.py` provides `ensure_aware()` to convert naive to aware datetimes + +**Handling Existing Code:** + +The codebase has many uses of `datetime.now()` and `datetime.utcnow()`. These will trigger S015 warnings when modified. To fix: + +```python +# Before +from datetime import datetime +now = datetime.now() # S015 error + +# After +from datetime import UTC, datetime +now = datetime.now(UTC) # ✅ Correct + +# Or use Django's timezone utility +from django.utils import timezone +now = timezone.now() # ✅ Also correct +``` + +**Exception Cases:** + +If you genuinely need local timezone (rare cases like user-facing scheduling), add: + +1. A detailed comment explaining the business requirement +2. A `# noqa: S015` comment to bypass the check + +```python +# Business requirement: Schedule report for user's local time (8am in their timezone) +# User explicitly selected timezone in their preferences +local_time = datetime.now() # noqa: S015 +``` + ## Architecture Rules ### Silo Mode diff --git a/tests/AGENTS.md b/tests/AGENTS.md index 7199b5f7452e7c..919e28ae4f6418 100644 --- a/tests/AGENTS.md +++ b/tests/AGENTS.md @@ -86,6 +86,58 @@ For example, a diff that uses `pytest` instead of `unittest` would look like: + EffectiveGrantStatus.from_cache(None) ``` +## Testing with Time + +**CRITICAL**: All datetime objects in Sentry MUST be timezone-aware and use UTC. + +```python +from sentry.testutils.helpers.datetime import before_now, freeze_time + +# Get a datetime relative to now +past_time = before_now(days=7) # 7 days ago in UTC + +# Freeze time for testing +@freeze_time("2024-01-01 12:00:00") +def test_something(): + assert datetime.now(UTC).year == 2024 +``` + +**Why Use Test Helpers:** + +- `before_now()` ensures all test datetimes are UTC-aware and relative to now +- `freeze_time()` (from `time_machine` library) allows deterministic time-based testing +- Both helpers prevent timezone-related test flakiness across different environments + +**Common Time-Based Test Patterns:** + +```python +from datetime import UTC, datetime +from sentry.testutils.helpers.datetime import before_now, freeze_time + +# Pattern 1: Testing time-sensitive features +def test_expired_data(): + expired_event = self.create_event( + data={"timestamp": before_now(days=30).timestamp()} + ) + assert expired_event.is_expired() + +# Pattern 2: Testing time progression +@freeze_time("2024-01-01 10:00:00") +def test_scheduled_task(): + task = self.create_task(scheduled_at=datetime.now(UTC)) + assert task.should_run() + +# Pattern 3: Testing date ranges +def test_date_filtering(): + start = before_now(days=7) + end = before_now(days=1) + events = Event.objects.filter( + datetime__gte=start, + datetime__lte=end + ) + assert events.count() > 0 +``` + ## File Location Map ### Tests diff --git a/tests/tools/test_flake8_plugin.py b/tests/tools/test_flake8_plugin.py index d6aaf48f40ab08..51108feb952a62 100644 --- a/tests/tools/test_flake8_plugin.py +++ b/tests/tools/test_flake8_plugin.py @@ -240,3 +240,56 @@ def test(monkeypatch) -> None: pass """ expected = ["t.py:1:9: S014 Use `unittest.mock` instead"] assert _run(src) == expected + + +def test_S015() -> None: + # Test datetime.now() without arguments + src_bad = """\ +from datetime import datetime + +def get_time(): + return datetime.now() # bad: no UTC +""" + expected = [ + "t.py:4:11: S015 Use `datetime.now(UTC)` or `timezone.now()` for timezone-aware datetimes" + ] + assert _run(src_bad) == expected + + # Test datetime.utcnow() - deprecated pattern + src_utcnow = """\ +from datetime import datetime + +def get_time(): + return datetime.utcnow() # bad: deprecated +""" + expected = [ + "t.py:4:11: S015 Use `datetime.now(UTC)` or `timezone.now()` for timezone-aware datetimes" + ] + assert _run(src_utcnow) == expected + + # Test datetime.now(UTC) - should pass + src_good = """\ +from datetime import UTC, datetime + +def get_time(): + return datetime.now(UTC) # good: has UTC +""" + assert _run(src_good) == [] + + # Test timezone.now() - should pass + src_timezone = """\ +from django.utils import timezone + +def get_time(): + return timezone.now() # good: timezone-aware +""" + assert _run(src_timezone) == [] + + # Test datetime.now(tz=UTC) - should pass + src_kwarg = """\ +from datetime import UTC, datetime + +def get_time(): + return datetime.now(tz=UTC) # good: has UTC as kwarg +""" + assert _run(src_kwarg) == [] diff --git a/tools/flake8_plugin.py b/tools/flake8_plugin.py index 245192fe3dfe18..f308ce8d5e8f0f 100644 --- a/tools/flake8_plugin.py +++ b/tools/flake8_plugin.py @@ -40,6 +40,8 @@ S014_msg = "S014 Use `unittest.mock` instead" +S015_msg = "S015 Use `datetime.now(UTC)` or `timezone.now()` for timezone-aware datetimes" + class SentryVisitor(ast.NodeVisitor): def __init__(self, filename: str) -> None: @@ -158,6 +160,18 @@ def visit_Call(self, node: ast.Call) -> None: if keyword.arg == "SENTRY_OPTIONS": self.errors.append((keyword.lineno, keyword.col_offset, S011_msg)) + # S015: Check for datetime.now() or datetime.utcnow() without UTC + if isinstance(node.func, ast.Attribute): + # Check if it's datetime.now() or datetime.utcnow() + if ( + node.func.attr in ("now", "utcnow") + and isinstance(node.func.value, ast.Name) + and node.func.value.id == "datetime" + ): + # datetime.now() without arguments or datetime.utcnow() + if node.func.attr == "utcnow" or len(node.args) == 0 and len(node.keywords) == 0: + self.errors.append((node.lineno, node.col_offset, S015_msg)) + self.generic_visit(node)