Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions internal/storage/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,15 @@ func AutoMigrateModels(db *gorm.DB, driver string) error {
return fmt.Errorf("failed to migrate database: %w", err)
}

// RAN-21: retire the pre-composite standalone unique index on traces.trace_id.
// AutoMigrate never drops indexes that no longer appear on struct tags, so on
// pre-existing databases the old uniqueIndex would persist and still block
// cross-tenant trace_id reuse. This is idempotent across drivers and a no-op
// on fresh databases.
if err := dropLegacyTraceIDUniqueIndex(db, driver); err != nil {
log.Printf("⚠️ legacy trace_id unique index drop failed: %v", err)
}

// Drop foreign keys that AutoMigrate may have created (MySQL)
if driver == "mysql" {
db.Exec("ALTER TABLE spans DROP FOREIGN KEY fk_traces_spans")
Expand Down
181 changes: 181 additions & 0 deletions internal/storage/migrate_traces.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
package storage

import (
"fmt"
"log"
"strings"

"gorm.io/gorm"
)

// dropLegacyTraceIDUniqueIndex removes any pre-RAN-21 standalone UNIQUE index
// that covers only traces.trace_id. From RAN-21 onward uniqueness is the
// composite idx_traces_tenant_trace_id on (tenant_id, trace_id); a surviving
// standalone unique index would silently block cross-tenant trace_id reuse.
//
// Discovery is structure-based (not name-based) because the legacy name varied
// across drivers and GORM versions. The composite index — which lists two
// columns — is never matched and therefore never dropped.
//
// Fresh databases never contain the legacy index, so this is a safe no-op on
// first boot. Invoked once per AutoMigrateModels call and is idempotent.
func dropLegacyTraceIDUniqueIndex(db *gorm.DB, driver string) error {
if db == nil {
return nil
}
driver = strings.ToLower(driver)
names, err := findLegacyTraceIDUniqueIndexes(db, driver)
if err != nil {
return err
}
for _, name := range names {
if name == "" {
continue
}
if err := dropIndexOnTraces(db, driver, name); err != nil {
return fmt.Errorf("drop legacy trace_id unique index %q: %w", name, err)
}
log.Printf("🧹 Dropped legacy single-column unique index on traces.trace_id: %s", name)
}
return nil
}

// findLegacyTraceIDUniqueIndexes returns every UNIQUE index on the traces table
// whose single indexed column is trace_id. The composite RAN-21 index
// (tenant_id, trace_id) is excluded because it covers two columns.
func findLegacyTraceIDUniqueIndexes(db *gorm.DB, driver string) ([]string, error) {

Check failure on line 46 in internal/storage/migrate_traces.go

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 21 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=RandomCodeSpace_otelcontext&issues=AZ3FYt2x2TusSyGBga3F&open=AZ3FYt2x2TusSyGBga3F&pullRequest=29
switch driver {
case "sqlite", "":
// Enumerate every unique index on traces, then inspect its column list
// via PRAGMA index_info. SQLite auto-creates indexes for UNIQUE table
// constraints with names prefixed "sqlite_autoindex_" — those also
// surface here and are handled identically. Aliased to is_unique
// because SQLite treats "unique" as a reserved keyword even as an
// output alias.
type idxRow struct {
Name string `gorm:"column:name"`
IsUnique int `gorm:"column:is_unique"`
}
var idxs []idxRow
if err := db.Raw(`SELECT name, "unique" AS is_unique FROM pragma_index_list('traces')`).Scan(&idxs).Error; err != nil {
return nil, fmt.Errorf("pragma_index_list(traces): %w", err)
}
type colRow struct {
Name string `gorm:"column:name"`
}
var out []string
for _, ix := range idxs {
if ix.IsUnique != 1 {
continue
}
var cols []colRow
if err := db.Raw(fmt.Sprintf("SELECT name FROM pragma_index_info('%s')", ix.Name)).Scan(&cols).Error; err != nil {
return nil, fmt.Errorf("pragma_index_info(%s): %w", ix.Name, err)
}
if len(cols) == 1 && cols[0].Name == "trace_id" {
out = append(out, ix.Name)
}
}
return out, nil

case "postgres", "postgresql":
// pg_index.indkey is an int2vector of attnums; join against
// pg_attribute to resolve column names. Filter to UNIQUE, non-primary
// indexes on the traces table covering exactly one column = trace_id.
var rows []indexNameRow
const q = `
SELECT c.relname AS name
FROM pg_index i
JOIN pg_class c ON c.oid = i.indexrelid
JOIN pg_class t ON t.oid = i.indrelid
JOIN pg_namespace n ON n.oid = t.relnamespace
WHERE t.relname = 'traces'
AND n.nspname = ANY (current_schemas(false))
AND i.indisunique
AND NOT i.indisprimary
AND i.indnatts = 1
AND (
SELECT attname FROM pg_attribute
WHERE attrelid = t.oid AND attnum = i.indkey[0]
) = 'trace_id'`
if err := db.Raw(q).Scan(&rows).Error; err != nil {
return nil, fmt.Errorf("pg_index lookup: %w", err)
}
return flattenIndexNames(rows), nil

case "mysql":
// information_schema.STATISTICS has one row per (index, seq_in_index).
// Group by index, count columns, and keep indexes where the sole
// column is trace_id and NON_UNIQUE=0.
var rows []indexNameRow
const q = `
SELECT INDEX_NAME AS name
FROM information_schema.STATISTICS
WHERE TABLE_SCHEMA = DATABASE()
AND TABLE_NAME = 'traces'
AND NON_UNIQUE = 0
AND INDEX_NAME <> 'PRIMARY'
GROUP BY INDEX_NAME
HAVING COUNT(*) = 1
AND MAX(COLUMN_NAME) = 'trace_id'`
if err := db.Raw(q).Scan(&rows).Error; err != nil {
return nil, fmt.Errorf("information_schema.STATISTICS lookup: %w", err)
}
return flattenIndexNames(rows), nil

case "sqlserver", "mssql":
// sys.indexes + sys.index_columns: one row per indexed column; keep
// unique, non-primary-key indexes on dbo.traces whose only column is
// trace_id.
var rows []indexNameRow
const q = `
SELECT i.name AS name
FROM sys.indexes i
JOIN sys.objects t ON t.object_id = i.object_id
WHERE t.name = 'traces'
AND i.is_unique = 1
AND i.is_primary_key = 0
AND (
SELECT COUNT(*) FROM sys.index_columns ic
WHERE ic.object_id = i.object_id AND ic.index_id = i.index_id
) = 1
AND EXISTS (
SELECT 1 FROM sys.index_columns ic
JOIN sys.columns c ON c.object_id = ic.object_id AND c.column_id = ic.column_id
WHERE ic.object_id = i.object_id AND ic.index_id = i.index_id AND c.name = 'trace_id'
)`
if err := db.Raw(q).Scan(&rows).Error; err != nil {
return nil, fmt.Errorf("sys.indexes lookup: %w", err)
}
return flattenIndexNames(rows), nil
}
return nil, nil
}

// indexNameRow is a single-column scan target shared by the non-SQLite branches.
// Keeping it package-level (rather than redeclared per-branch) lets
// flattenIndexNames project through a single concrete type.
type indexNameRow struct {
Name string `gorm:"column:name"`
}

func flattenIndexNames(rows []indexNameRow) []string {
out := make([]string, 0, len(rows))
for _, r := range rows {
out = append(out, r.Name)
}
return out
}

// dropIndexOnTraces removes an index by name, per-driver. GORM's Migrator
// handles SQLite/Postgres/MySQL cleanly; for SQL Server we must qualify the
// DROP with the table name.
func dropIndexOnTraces(db *gorm.DB, driver, name string) error {
switch driver {
case "sqlserver", "mssql":
// T-SQL: DROP INDEX name ON table
return db.Exec(fmt.Sprintf("DROP INDEX [%s] ON [traces]", name)).Error
default:
return db.Migrator().DropIndex(&Trace{}, name)
}
}
9 changes: 6 additions & 3 deletions internal/storage/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,14 @@ const DefaultTenantID = "default"
// Index strategy: single-column tenant_id is redundant — every tenant-scoped
// query joins tenant_id with another filter (timestamp, service_name). The
// leftmost column of a composite index satisfies single-column tenant lookups,
// so we only declare composites.
// so we only declare composites. TraceID uniqueness is scoped to (tenant_id,
// trace_id): distinct tenants may legitimately ingest identical trace_ids
// (RAN-21). The old standalone `uniqueIndex` on trace_id is dropped at
// migration time by dropLegacyTraceIDUniqueIndex.
type Trace struct {
ID uint `gorm:"primaryKey" json:"id"`
TenantID string `gorm:"size:64;default:'default';not null;index:idx_traces_tenant_ts,priority:1;index:idx_traces_tenant_service,priority:1" json:"tenant_id"`
TraceID string `gorm:"uniqueIndex;size:32;not null" json:"trace_id"`
TenantID string `gorm:"size:64;default:'default';not null;index:idx_traces_tenant_ts,priority:1;index:idx_traces_tenant_service,priority:1;uniqueIndex:idx_traces_tenant_trace_id,priority:1" json:"tenant_id"`
TraceID string `gorm:"size:32;not null;uniqueIndex:idx_traces_tenant_trace_id,priority:2" json:"trace_id"`
ServiceName string `gorm:"size:255;index:idx_traces_tenant_service,priority:2" json:"service_name"`
Duration int64 `gorm:"index" json:"duration"` // Microseconds
DurationMs float64 `gorm:"-" json:"duration_ms"`
Expand Down
10 changes: 8 additions & 2 deletions internal/storage/trace_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ func (r *Repository) BatchCreateSpans(spans []Span) error {
}

// BatchCreateTraces inserts traces, skipping duplicates.
// Duplicate is defined per the composite uniqueIndex idx_traces_tenant_trace_id
// on (tenant_id, trace_id): a trace_id clash within the same tenant is ignored,
// while the same trace_id under a different tenant inserts cleanly.
func (r *Repository) BatchCreateTraces(traces []Trace) error {
if len(traces) == 0 {
return nil
Expand All @@ -67,6 +70,8 @@ func (r *Repository) BatchCreateTraces(traces []Trace) error {
}

// CreateTrace inserts a new trace, skipping if it already exists.
// Uniqueness is per idx_traces_tenant_trace_id (tenant_id, trace_id), so the
// same trace_id across tenants is allowed.
func (r *Repository) CreateTrace(trace Trace) error {
if strings.ToLower(r.driver) == "mysql" {
return r.db.Clauses(clause.Insert{Modifier: "IGNORE"}).Create(&trace).Error
Expand All @@ -75,8 +80,9 @@ func (r *Repository) CreateTrace(trace Trace) error {
}

// GetTrace returns a trace by ID with its spans and logs, scoped to the tenant on ctx.
// The Preloaded Spans and Logs are additionally filtered so a trace ID collision
// across tenants cannot leak cross-tenant children.
// Trace uniqueness is composite (tenant_id, trace_id), so the same trace_id can
// legitimately exist in multiple tenants; the Preloaded Spans and Logs are
// filtered by tenant_id as defense-in-depth against cross-tenant child leakage.
func (r *Repository) GetTrace(ctx context.Context, traceID string) (*Trace, error) {
tenant := TenantFromContext(ctx)
var trace Trace
Expand Down
Loading
Loading