Database persistence for writeback trace context#541
Database persistence for writeback trace context#541hweawer wants to merge 1 commit intofeat/tracing-executorfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds database persistence for OpenTelemetry trace context (trace ID, span ID, and trace flags) to writeback tasks. This allows async writeback operations to be linked back to their originating request traces.
Changes:
- Added database migration to add three new text columns (
trace_id,span_id,trace_flags) to thewriteback_tasktable - Updated all SELECT and INSERT queries in the store to include the new trace context columns
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| localdb/migrations/00003_writeback_tracing.go | Adds migration to create three new trace-related columns with empty string defaults and provides rollback migration |
| lib/persistedretry/writeback/store.go | Updates SQL queries (SELECT and INSERT) to include the new trace context columns |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _, err := tx.Exec(` | ||
| CREATE TABLE writeback_task_backup AS SELECT namespace, name, created_at, last_attempt, status, failures, delay FROM writeback_task; | ||
| DROP TABLE writeback_task; | ||
| CREATE TABLE writeback_task ( | ||
| namespace text NOT NULL, | ||
| name text NOT NULL, | ||
| created_at timestamp DEFAULT CURRENT_TIMESTAMP, | ||
| last_attempt timestamp NOT NULL, | ||
| status text NOT NULL, | ||
| failures integer NOT NULL, | ||
| delay integer NOT NULL, | ||
| PRIMARY KEY(namespace, name) | ||
| ); | ||
| INSERT INTO writeback_task SELECT * FROM writeback_task_backup; | ||
| DROP TABLE writeback_task_backup; | ||
| `) |
There was a problem hiding this comment.
The down migration contains multiple SQL statements in a single Exec call, which may fail with the default go-sqlite3 driver configuration. The go-sqlite3 driver requires the connection string parameter "_loc=auto" or needs to be opened with specific flags to enable multi-statement support. Consider splitting this into separate Exec calls for each statement:
- CREATE TABLE writeback_task_backup...
- DROP TABLE writeback_task
- CREATE TABLE writeback_task...
- INSERT INTO writeback_task...
- DROP TABLE writeback_task_backup
This approach is more reliable and consistent with standard SQL execution patterns.
This pull request adds database persistence for OpenTelemetry trace context (trace ID, span ID, and trace flags) to writeback tasks. This allows async writeback operations to be linked back to their originating request traces.