Skip to content

Commit 3237181

Browse files
kneeyo1lynnagara
andauthored
fix timezone normalization (#82496)
This fixes the timezone normalization (both offset-naive and offset-aware timestamps). Also added some test cases --------- Co-authored-by: Lyn Nagara <[email protected]>
1 parent 1b162a0 commit 3237181

File tree

2 files changed

+40
-15
lines changed

2 files changed

+40
-15
lines changed

src/sentry/consumers/dlq.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,13 @@ def submit(self, message: Message[KafkaPayload]) -> None:
9898
)
9999

100100
if isinstance(message.value, BrokerValue):
101-
if message.value.timestamp < min_accepted_timestamp:
101+
# Normalize the message timezone to be UTC
102+
if message.value.timestamp.tzinfo is None:
103+
message_timestamp = message.value.timestamp.replace(tzinfo=timezone.utc)
104+
else:
105+
message_timestamp = message.value.timestamp
106+
107+
if message_timestamp < min_accepted_timestamp:
102108
self.offsets_to_forward[message.value.partition] = message.value.next_offset
103109
raise InvalidMessage(
104110
message.value.partition, message.value.offset, reason=RejectReason.STALE.value

tests/sentry/consumers/test_dlq.py

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import time
22
from datetime import datetime, timedelta, timezone
3+
from typing import cast
34
from unittest.mock import Mock
45

56
import msgpack
@@ -12,23 +13,20 @@
1213
from sentry.testutils.pytest.fixtures import django_db_all
1314

1415

15-
def make_message(
16-
payload: bytes, partition: Partition, offset: int, timestamp: datetime | None = None
17-
) -> Message:
16+
def make_message(payload: bytes, partition: Partition, offset: int, timestamp: datetime) -> Message:
1817
return Message(
1918
BrokerValue(
2019
KafkaPayload(None, payload, []),
2120
partition,
2221
offset,
23-
timestamp if timestamp else datetime.now(),
22+
timestamp,
2423
)
2524
)
2625

2726

2827
@pytest.mark.parametrize("stale_threshold_sec", [300])
2928
@django_db_all
30-
def test_dlq_stale_messages(factories, stale_threshold_sec) -> None:
31-
# Tests messages that have gotten stale (default longer than 5 minutes)
29+
def test_dlq_stale_messages_timestamps(factories, stale_threshold_sec) -> None:
3230

3331
organization = factories.create_organization()
3432
project = factories.create_project(organization=organization)
@@ -54,20 +52,41 @@ def test_dlq_stale_messages(factories, stale_threshold_sec) -> None:
5452
)
5553
strategy = factory.create_with_partitions(Mock(), Mock())
5654

57-
for time_diff in range(10, 0, -1):
55+
test_cases = [
56+
{
57+
"timestamp": datetime.now() - timedelta(seconds=stale_threshold_sec - 60),
58+
"should_raise": False,
59+
},
60+
{
61+
"timestamp": datetime.now() - timedelta(seconds=stale_threshold_sec + 60),
62+
"should_raise": True,
63+
},
64+
{
65+
"timestamp": datetime.now(timezone.utc) - timedelta(seconds=stale_threshold_sec + 60),
66+
"should_raise": True,
67+
},
68+
{
69+
"timestamp": datetime.now(timezone.utc) - timedelta(seconds=stale_threshold_sec - 60),
70+
"should_raise": False,
71+
},
72+
]
73+
74+
for idx, case in enumerate(test_cases):
5875
message = make_message(
5976
empty_event_payload,
6077
partition,
61-
offset - time_diff,
62-
timestamp=datetime.now(timezone.utc) - timedelta(minutes=time_diff),
78+
offset + idx,
79+
timestamp=cast(datetime, case["timestamp"]),
6380
)
64-
if time_diff < 5:
65-
strategy.submit(message)
66-
else:
81+
82+
if case["should_raise"]:
6783
with pytest.raises(InvalidMessage) as exc_info:
6884
strategy.submit(message)
6985

7086
assert exc_info.value.partition == partition
71-
assert exc_info.value.offset == offset - time_diff
87+
assert exc_info.value.offset == offset + idx
88+
else:
89+
strategy.submit(message)
7290

73-
assert inner_strategy_mock.submit.call_count == 4
91+
valid_messages = sum(1 for case in test_cases if not case["should_raise"])
92+
assert inner_strategy_mock.submit.call_count == valid_messages

0 commit comments

Comments
 (0)