Use S3LogsReader with superregion and UTC timezone (#972)

Author: yaroliakh

tests/utils/scribereader_test.py

@@ -40,6 +40,10 @@ def test_read_log_stream_for_action_run_yelp_clog():
     ), mock.patch("tron.config.static_config.build_configuration_watcher", autospec=True,), mock.patch(
         "tron.config.static_config.load_yaml_file",
         autospec=True,
+    ), mock.patch(
+        "tron.utils.scribereader.get_ecosystem", autospec=True, return_value="fake"
+    ), mock.patch(
+        "tron.utils.scribereader.get_superregion", autospec=True, return_value="fake"
     ), mock.patch(
         "tron.utils.scribereader.S3LogsReader", autospec=True
     ) as mock_s3_reader:
@@ -80,6 +84,47 @@ def test_read_log_stream_for_action_run_yelp_clog():
     assert output == ["line 1", "line 2"]
 
 
+@pytest.mark.parametrize(
+    "local_datetime, expected_date",
+    [
+        (
+            datetime.datetime(2024, 2, 29, 23, 59, 59, tzinfo=datetime.timezone(datetime.timedelta(hours=+3))),
+            datetime.date(2024, 2, 29),
+        ),
+        (
+            datetime.datetime(2024, 2, 29, 23, 59, 59, tzinfo=datetime.timezone(datetime.timedelta(hours=-3))),
+            datetime.date(2024, 3, 1),
+        ),
+    ],
+)
+def test_read_log_stream_for_action_run_yelp_clog_tz(local_datetime, expected_date):
+    with mock.patch(
+        "staticconf.read",
+        autospec=True,
+        side_effect=static_conf_patch({"logging.use_s3_reader": True, "logging.max_lines_to_display": 1000}),
+    ), mock.patch("tron.config.static_config.build_configuration_watcher", autospec=True,), mock.patch(
+        "tron.config.static_config.load_yaml_file",
+        autospec=True,
+    ), mock.patch(
+        "tron.utils.scribereader.get_ecosystem", autospec=True, return_value="fake"
+    ), mock.patch(
+        "tron.utils.scribereader.get_superregion", autospec=True, return_value="fake"
+    ), mock.patch(
+        "tron.utils.scribereader.S3LogsReader", autospec=True
+    ) as mock_s3_log_reader:
+
+        read_log_stream_for_action_run(
+            "namespace.job.1234.action",
+            component="stdout",
+            min_date=local_datetime,
+            max_date=local_datetime,
+            paasta_cluster="fake",
+        )
+    mock_s3_log_reader.return_value.get_log_reader.assert_called_once_with(
+        log_name=mock.ANY, min_date=expected_date, max_date=expected_date
+    )
+
+
 def test_read_log_stream_for_action_run_min_date_and_max_date_today():
     # NOTE: these tests don't actually depend on the current time apart from
     # today vs not-today and the args are forwarded to scribereader anyway

tron/utils/scribereader.py

@@ -171,19 +171,27 @@ def read_log_stream_for_action_run(
 
     paasta_logs = PaaSTALogs(component, paasta_cluster, action_run_id)
     stream_name = paasta_logs.stream_name
-
-    today = datetime.date.today()
-    start_date = min_date.date()
     end_date: Optional[datetime.date]
 
     # yelp_clog S3LogsReader is a newer reader that is supposed to replace scribe readers eventually.
     if use_s3_reader:
-        end_date = max_date.date() if max_date else today
+        # S3 reader uses UTC as a standard timezone
+        # if min_date and max_date timezone is missing, astimezone() will assume local timezone and convert it to UTC
+        start_date = min_date.astimezone(datetime.timezone.utc).date()
+        end_date = (
+            max_date.astimezone(datetime.timezone.utc).date()
+            if max_date
+            else datetime.datetime.now().astimezone(datetime.timezone.utc).date()
+        )
 
         log.debug("Using S3LogsReader to retrieve logs")
-        s3_reader = S3LogsReader(ecosystem).get_log_reader(log_name=stream_name, min_date=start_date, max_date=end_date)
+        s3_reader = S3LogsReader(ecosystem, superregion).get_log_reader(
+            log_name=stream_name, min_date=start_date, max_date=end_date
+        )
         paasta_logs.fetch(s3_reader, max_lines)
     else:
+        today = datetime.date.today()
+        start_date = min_date.date()
         end_date = max_date.date() if max_date else None
         use_tailer = today in {start_date, end_date}
         use_reader = start_date != today and end_date is not None

yelp_package/extra_requirements_yelp.txt

@@ -9,6 +9,6 @@ srv-configs==1.3.4  # required by monk
 tenacity==8.2.3 # required by yelp-logging
 thriftpy2==0.4.20   # required by monk
 yelp-cgeom==1.3.1   # required by geogrid
-yelp-clog==7.1.1  # scribereader dependency
+yelp-clog==7.1.2  # scribereader dependency
 yelp-logging==4.17.0  # scribereader dependency
 yelp-meteorite==2.1.1  # used by task-processing to emit metrics, clusterman-metrics dependency