Description

shafiktanbir

opened on Oct 26, 2026

Issue body actions

Hey! While looking through the query alerts evaluation service on main (commit c5b123d), I noticed alertSeries.server.ts and healthQuerySeries.server.ts use parseClickhouseDateTime (apps/webapp/app/v3/services/alerts/clickhouseDateTime.ts) to handle ClickHouse DateTime/DateTime64 strings that already include a Z or numeric offset. However, parseEventTime in apps/webapp/app/v3/services/alerts/engine/productionEvaluateBinding.server.ts (line 176) and parseTime in apps/webapp/app/v3/services/alerts/previewQuery.server.ts (line 68) still construct new Date(${value.replace(" ", "T")}Z) directly.

When ClickHouse returns an ISO timestamp that already has a Z or offset suffix, appending "Z" produces an invalid date and parseEventTime returns null, which leaves lastTime as null in runMatch (lines 129-153) so the match cursor doesn't advance. Should both of those call parseClickhouseDateTime too? Happy to open a quick PR if you want!

Activity

1. duongynhi000005-oss commented on Oct 27, 2026

   

   duongynhi000005-oss

   on Oct 27, 2026

   More actions

   Root Cause & Proposed Fix

   1. Root Cause:

      • On current main, rowTime in apps/webapp/app/v3/services/alerts/engine/productionEvaluateBinding.server.ts (lines 170–178) and rowTimeIso in apps/webapp/app/v3/services/alerts/previewQuery.server.ts (lines 66–70) still construct new Date(\${value.replace(" ", "T")}Z`)directly instead of callingparseClickhouseDateTime (apps/webapp/app/v3/services/alerts/clickhouseDateTime.ts`).
      • When ClickHouse returns an ISO timestamp that already includes a Z or numeric offset suffix (e.g. 2026-07-01T12:30:15Z or 2026-07-01T14:30:15+02:00), unconditionally appending "Z" produces an invalid Date (NaN), causing rowTime to return undefined and rowTimeIso to fall back to the raw string.

   2. Proposed Fix:

      • Import and use parseClickhouseDateTime(value) in both rowTime (productionEvaluateBinding.server.ts) and rowTimeIso (previewQuery.server.ts), matching alertSeries.server.ts and healthQuerySeries.server.ts.
      • Extend apps/webapp/app/v3/services/alerts/clickhouseDateTime.test.ts to cover ISO timestamps with Z and numeric timezone offsets alongside space-separated DateTime / DateTime64 strings.

   I have a verified patch and unit tests ready on my fork and can open a PR if assigned.

Issue actions