Skip to content

Database persistence for writeback trace context#541

Open
hweawer wants to merge 1 commit intofeat/tracing-executorfrom
feat/tracing-writeback-storage
Open

Database persistence for writeback trace context#541
hweawer wants to merge 1 commit intofeat/tracing-executorfrom
feat/tracing-writeback-storage

Conversation

@hweawer
Copy link
Collaborator

@hweawer hweawer commented Jan 26, 2026

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.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 the writeback_task table
  • 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.

Comment on lines +47 to +62
_, 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;
`)
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. CREATE TABLE writeback_task_backup...
  2. DROP TABLE writeback_task
  3. CREATE TABLE writeback_task...
  4. INSERT INTO writeback_task...
  5. DROP TABLE writeback_task_backup

This approach is more reliable and consistent with standard SQL execution patterns.

Copilot uses AI. Check for mistakes.
@hweawer hweawer mentioned this pull request Jan 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants