Skip to content

fix(telemetry): store Athena times as timestamp, normalize MCP bounds - #1070

Merged
ParidelPooya merged 8 commits into
mainfrom
fix/telemetry-athena-timestamp-columns
Oct 23, 2026
Merged

ParidelPooya merged 8 commits into
mainfrom
fix/telemetry-athena-timestamp-columns

Conversation

@ParidelPooya

@ParidelPooya ParidelPooya commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

A deployed test ran the same workflow on the JS, Python, and Java exporters, writing to S3 and Aurora. It found two problems with how record timestamps are compared.

  1. The bundled Athena table declares emittedat, starttime, endtime, and the operation starttime/endtime as string. So Athena compares them as text. Measured on the deployed records: endtime >= '2026-10-23T18:42:46Z' returned none of the 7 records that ended in that second, because . sorts before Z. The same filter on a timestamp column returned all 7.
  2. list_executions interpolated since and until as given. On destinations that compare text (DynamoDB, CloudWatch Logs Insights), a bound without a fraction, or with an offset such as -07:00, returned the wrong records. With an offset the error is hours.

Changes:

  • Athena table: the CDK Glue table (cdk/stack.ts) and the telemetry-core DDL (athena.ts) declare those columns as timestamp. The S3 objects do not change; the OpenX JSON SerDe parses the ISO strings into the typed columns.
  • Athena schema prompt (schema.ts): the columns are listed as timestamp (UTC). A new dialect rule says to compare them with TIMESTAMP literals or timestamp expressions, never strings.
  • MCP list_executions: a new normalizeTimestamp validates each bound, including the calendar (it rejects 2026-02-30, which Date.parse would roll forward to March 2). A bound with no offset is read as UTC. Each dialect then writes its own literal:
    • Typed time columns (Athena timestamp, Aurora and Redshift timestamptz, OpenSearch date) compare instants. The bound keeps every digit the caller gave, because an existing table can hold values finer than a millisecond. Athena gets a TIMESTAMP '…' literal; the others get a quoted UTC string.
    • Text comparisons (DynamoDB, Logs Insights) get the stored format: UTC, 3 fraction digits, Z. A finer bound is rounded so it selects the same records: since up to the next millisecond, until down.
  • Docs: the exporter README has a new "Timestamps" section that states the format contract. The MCP README describes how bounds are read. Both CHANGELOG entries are under Fixed. The Athena entry includes a migration note: a table created earlier keeps string columns until it is recreated or altered.

JS already writes every record timestamp with toISOString(), so the record format does not change here. Python and Java change to the same fixed-width format in sibling PRs:

Testing

  • Deployed check against the 18 real records in S3:
    • I created a temporary Athena table from the new buildCreateTableDdl output, then dropped it.
    • The SQL that buildListExecutionsSql now builds for since: "2026-10-23T11:42:46-07:00" returned the 3 executions in that window, from all three SDKs.
    • date_diff('millisecond', starttime, endtime) equals durationms.
    • Operation starttime/endtime read as timestamps.
    • A string comparison fails with TYPE_MISMATCH, as the prompt says.
  • New timeBounds.test.ts (21 tests): the normalized forms, rejection of invalid dates and injection attempts, text order equal to time order after normalization, and the literal per destination. Updated the two injection.test.ts expectations.
  • Package tests: MCP 427, core 385, exporter 159, vscode 34, desktop 48. All passed.
  • biome ci on the repo: clean.
  • Not run: a CDK deploy of the changed Glue table.

Athena compared the time columns as text, so a filter left out records from the same second. The Glue table and the Athena DDL now declare the time columns as timestamp, and the schema prompt tells queries to use TIMESTAMP literals. The MCP list_executions tool rewrites since and until to the stored UTC format, and uses a TIMESTAMP literal on Athena. The exporter README states the timestamp format.
@ParidelPooya
ParidelPooya temporarily deployed to ai-pr-review-runtime October 23, 2026 19:48 — with GitHub Actions Inactive
@ParidelPooya
ParidelPooya temporarily deployed to ai-pr-review-runtime October 23, 2026 19:49 — with GitHub Actions Inactive
Comment thread packages/aws-durable-execution-sdk-js-telemetry-mcp/src/sqlSafe.ts Outdated
Comment thread packages/aws-durable-execution-sdk-js-telemetry-mcp/src/sqlSafe.ts Outdated
@github-actions

This comment has been minimized.

Date.parse rolls an impossible date forward, so 2026-02-30 queried March 2. The day, hour, minute and second are now checked first. A bound with sub-millisecond digits is rounded up for since and down for until, so the bound selects the same records as before the rewrite.
@ParidelPooya
ParidelPooya temporarily deployed to ai-pr-review-runtime October 23, 2026 20:32 — with GitHub Actions Inactive
Comment thread packages/aws-durable-execution-sdk-js-telemetry-mcp/src/sqlSafe.ts
@github-actions

This comment has been minimized.

Typed time columns compare instants, and an existing Aurora or Redshift table can hold values finer than a millisecond. Rounding a bound there changed which rows it selected. Bounds now keep the caller's digits for Athena, Aurora, Redshift and OpenSearch, and get the stored millisecond width only where the comparison is on text (DynamoDB, Logs Insights).
@ParidelPooya
ParidelPooya temporarily deployed to ai-pr-review-runtime October 23, 2026 20:44 — with GitHub Actions Inactive
Comment thread packages/aws-durable-execution-sdk-js-telemetry-mcp/src/sqlSafe.ts
@github-actions

This comment has been minimized.

DynamoDB and CloudWatch Logs still compare timestamps as text, so records from the Python exporter 0.0.1 preview, which wrote microsecond timestamps, can be ordered incorrectly within one second.
@ParidelPooya
ParidelPooya temporarily deployed to ai-pr-review-runtime October 23, 2026 20:56 — with GitHub Actions Inactive
Comment thread packages/aws-durable-execution-sdk-js-telemetry-mcp/src/timeBounds.test.ts Outdated
@github-actions

This comment has been minimized.

Two assertions omitted the precision argument, so they passed undefined.
@ParidelPooya
ParidelPooya temporarily deployed to ai-pr-review-runtime October 23, 2026 21:02 — with GitHub Actions Inactive
Comment thread packages/aws-durable-execution-sdk-js-telemetry-mcp/src/sqlSafe.ts
@github-actions

This comment has been minimized.

The bound pattern accepted 2026-10-23T18:42.5Z and read the fraction as seconds. ISO-8601 reads it as a fractional minute, 18:42:30. A fraction now needs explicit seconds.
@ParidelPooya
ParidelPooya temporarily deployed to ai-pr-review-runtime October 23, 2026 22:18 — with GitHub Actions Inactive
@ParidelPooya
ParidelPooya temporarily deployed to ai-pr-review-runtime October 23, 2026 22:21 — with GitHub Actions Inactive
@github-actions

This comment has been minimized.

Resolves the CHANGELOG conflict with #1071 by keeping both Fixed entries. The MCP entry now describes the final bound rules.
@ParidelPooya
ParidelPooya temporarily deployed to ai-pr-review-runtime October 23, 2026 22:34 — with GitHub Actions Inactive
@github-actions

Copy link
Copy Markdown

Codex AI review

The prior P2 finding remains at packages/aws-durable-execution-sdk-js-telemetry-mcp/src/sqlSafe.ts:203 (maintainer verdict: out-of-scope). No additional findings. Tests and deployment checks were not executed under the review constraints.

Suppressed repeats

  • arf_v1_gghgogdvp6bq3z5ddo33tyxswv — out-of-scope (maintainer verdict)

Reviewed commit 7abc7c3885501986d6c74dcc4b601195b0a095f2. Workflow run

@ParidelPooya
ParidelPooya merged commit 9840138 into main Oct 23, 2026
28 checks passed
@ParidelPooya
ParidelPooya deleted the fix/telemetry-athena-timestamp-columns branch October 23, 2026 22:53

This branch was previously deployed

1 inactive deployment
ai-pr-review-runtime — 7abc7c38 Deployed Oct 23, 2026 by ParidelPooya via ai-pr-review / Codex review / Generate Codex review #1780
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants