From e9952083ce8bffd2a50457378f8ec9da59a1b4cb Mon Sep 17 00:00:00 2001 From: JuanmaBM Date: Wed, 26 Aug 2026 14:01:15 +0200 Subject: [PATCH 1/3] feat: add reconcile-to-request trace correlation via span links Persist W3C traceparent/tracestate on every resource at create/update time, propagate through gRPC ObjectReference, and attach OTel span links on control-plane reconcile spans so Jaeger can navigate from a reconciliation back to the originating API request. API server: - TraceMeta embeddable struct (json:"-", gorm columns) - Proto: traceparent/tracestate on ObjectReference (fields 6-7) - All 7 plugins: model embed, migration, service injection, gRPC presenter - Service layer calls CaptureTraceContext(ctx) before dao.Create/Replace Control plane: - StartReconcileSpan accepts traceparent, creates span link via parseTraceparentLink - All 10 reconciler call sites updated (event-driven pass traceparent, continuous pass "") - 12 unit tests for traceparent parsing Co-Authored-By: Claude Opus 4.6 --- .../pkg/api/grpc/hypershell/v1/common.pb.go | 27 ++- components/api-server/pkg/api/tracemeta.go | 32 ++++ .../plugins/gatewayNetworks/grpc_presenter.go | 12 +- .../plugins/gatewayNetworks/migration.go | 20 +++ .../plugins/gatewayNetworks/model.go | 2 + .../plugins/gatewayNetworks/plugin.go | 1 + .../plugins/gatewayNetworks/service.go | 2 + .../plugins/gatewayReleases/grpc_presenter.go | 12 +- .../plugins/gatewayReleases/migration.go | 20 +++ .../plugins/gatewayReleases/model.go | 2 + .../plugins/gatewayReleases/plugin.go | 1 + .../plugins/gatewayReleases/service.go | 2 + .../plugins/gateways/grpc_presenter.go | 12 +- .../api-server/plugins/gateways/migration.go | 20 +++ .../api-server/plugins/gateways/model.go | 2 + .../api-server/plugins/gateways/plugin.go | 1 + .../api-server/plugins/gateways/service.go | 2 + .../plugins/managedClusters/grpc_presenter.go | 12 +- .../plugins/managedClusters/migration.go | 20 +++ .../plugins/managedClusters/model.go | 2 + .../plugins/managedClusters/plugin.go | 1 + .../plugins/managedClusters/service.go | 2 + .../managedDatabases/grpc_presenter.go | 12 +- .../plugins/managedDatabases/migration.go | 20 +++ .../plugins/managedDatabases/model.go | 2 + .../plugins/managedDatabases/plugin.go | 1 + .../plugins/managedDatabases/service.go | 2 + .../plugins/roleBindings/grpc_presenter.go | 12 +- .../plugins/roleBindings/migration.go | 20 +++ .../api-server/plugins/roleBindings/model.go | 2 + .../api-server/plugins/roleBindings/plugin.go | 1 + .../plugins/roleBindings/service.go | 1 + .../proto/hypershell/v1/common.proto | 2 + .../control-plane/internal/otel/otel_test.go | 56 +++++- .../control-plane/internal/otel/reconcile.go | 69 +++++++- .../internal/reconciler/health.go | 2 +- .../internal/reconciler/namespace.go | 2 +- .../internal/reconciler/reconciler.go | 10 +- .../reconciler/role_binding_reconciler.go | 2 +- .../internal/reconciler/sandboxcount.go | 2 +- specs/index.spec.md | 1 + .../control-plane-observability.spec.md | 4 +- .../reconcile-trace-correlation.spec.md | 162 ++++++++++++++++++ 43 files changed, 545 insertions(+), 47 deletions(-) create mode 100644 components/api-server/pkg/api/tracemeta.go create mode 100644 specs/platform/reconcile-trace-correlation.spec.md diff --git a/components/api-server/pkg/api/grpc/hypershell/v1/common.pb.go b/components/api-server/pkg/api/grpc/hypershell/v1/common.pb.go index f9fe6e8a3..e2b09e105 100644 --- a/components/api-server/pkg/api/grpc/hypershell/v1/common.pb.go +++ b/components/api-server/pkg/api/grpc/hypershell/v1/common.pb.go @@ -81,6 +81,8 @@ type ObjectReference struct { UpdatedAt *timestamppb.Timestamp `protobuf:"bytes,3,opt,name=updated_at,json=updatedAt,proto3" json:"updated_at,omitempty"` Kind string `protobuf:"bytes,4,opt,name=kind,proto3" json:"kind,omitempty"` Href string `protobuf:"bytes,5,opt,name=href,proto3" json:"href,omitempty"` + Traceparent *string `protobuf:"bytes,6,opt,name=traceparent,proto3,oneof" json:"traceparent,omitempty"` + Tracestate *string `protobuf:"bytes,7,opt,name=tracestate,proto3,oneof" json:"tracestate,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -150,6 +152,20 @@ func (x *ObjectReference) GetHref() string { return "" } +func (x *ObjectReference) GetTraceparent() string { + if x != nil && x.Traceparent != nil { + return *x.Traceparent + } + return "" +} + +func (x *ObjectReference) GetTracestate() string { + if x != nil && x.Tracestate != nil { + return *x.Tracestate + } + return "" +} + type ListMeta struct { state protoimpl.MessageState `protogen:"open.v1"` Page int32 `protobuf:"varint,1,opt,name=page,proto3" json:"page,omitempty"` @@ -298,7 +314,7 @@ var File_hypershell_v1_common_proto protoreflect.FileDescriptor const file_hypershell_v1_common_proto_rawDesc = "" + "\n" + - "\x1ahypershell/v1/common.proto\x12\rhypershell.v1\x1a\x1fgoogle/protobuf/timestamp.proto\"\xbf\x01\n" + + "\x1ahypershell/v1/common.proto\x12\rhypershell.v1\x1a\x1fgoogle/protobuf/timestamp.proto\"\xaa\x02\n" + "\x0fObjectReference\x12\x0e\n" + "\x02id\x18\x01 \x01(\tR\x02id\x129\n" + "\n" + @@ -306,7 +322,13 @@ const file_hypershell_v1_common_proto_rawDesc = "" + "\n" + "updated_at\x18\x03 \x01(\v2\x1a.google.protobuf.TimestampR\tupdatedAt\x12\x12\n" + "\x04kind\x18\x04 \x01(\tR\x04kind\x12\x12\n" + - "\x04href\x18\x05 \x01(\tR\x04href\"H\n" + + "\x04href\x18\x05 \x01(\tR\x04href\x12%\n" + + "\vtraceparent\x18\x06 \x01(\tH\x00R\vtraceparent\x88\x01\x01\x12#\n" + + "\n" + + "tracestate\x18\a \x01(\tH\x01R\n" + + "tracestate\x88\x01\x01B\x0e\n" + + "\f_traceparentB\r\n" + + "\v_tracestate\"H\n" + "\bListMeta\x12\x12\n" + "\x04page\x18\x01 \x01(\x05R\x04page\x12\x12\n" + "\x04size\x18\x02 \x01(\x05R\x04size\x12\x14\n" + @@ -360,6 +382,7 @@ func file_hypershell_v1_common_proto_init() { if File_hypershell_v1_common_proto != nil { return } + file_hypershell_v1_common_proto_msgTypes[0].OneofWrappers = []any{} type x struct{} out := protoimpl.TypeBuilder{ File: protoimpl.DescBuilder{ diff --git a/components/api-server/pkg/api/tracemeta.go b/components/api-server/pkg/api/tracemeta.go new file mode 100644 index 000000000..45db63465 --- /dev/null +++ b/components/api-server/pkg/api/tracemeta.go @@ -0,0 +1,32 @@ +package api + +import ( + "context" + "fmt" + + "go.opentelemetry.io/otel/trace" +) + +// TraceMeta embeds alongside api.Meta to persist the originating W3C Trace +// Context on every resource. The json:"-" tag keeps these fields out of REST +// API responses (RTC-05). The columns are nullable so pre-existing rows and +// resources created with telemetry disabled have NULL trace context. +type TraceMeta struct { + Traceparent *string `json:"-" gorm:"column:traceparent"` + Tracestate *string `json:"-" gorm:"column:tracestate"` +} + +// CaptureTraceContext extracts the active span's W3C traceparent and +// tracestate from ctx and stores them. When no valid span is active (OTel +// disabled or no sampled span), the fields are left nil. +func (t *TraceMeta) CaptureTraceContext(ctx context.Context) { + sc := trace.SpanFromContext(ctx).SpanContext() + if !sc.IsValid() { + return + } + tp := fmt.Sprintf("00-%s-%s-%s", sc.TraceID(), sc.SpanID(), sc.TraceFlags()) + t.Traceparent = &tp + if ts := sc.TraceState().String(); ts != "" { + t.Tracestate = &ts + } +} diff --git a/components/api-server/plugins/gatewayNetworks/grpc_presenter.go b/components/api-server/plugins/gatewayNetworks/grpc_presenter.go index de3a3401d..857827384 100644 --- a/components/api-server/plugins/gatewayNetworks/grpc_presenter.go +++ b/components/api-server/plugins/gatewayNetworks/grpc_presenter.go @@ -8,11 +8,13 @@ import ( func gatewayNetworkToProto(d *GatewayNetwork) *pb.GatewayNetwork { return &pb.GatewayNetwork{ Metadata: &pb.ObjectReference{ - Id: d.ID, - CreatedAt: timestamppb.New(d.CreatedAt), - UpdatedAt: timestamppb.New(d.UpdatedAt), - Kind: "GatewayNetwork", - Href: "/api/hypershell/v1/gateway_networks/" + d.ID, + Id: d.ID, + CreatedAt: timestamppb.New(d.CreatedAt), + UpdatedAt: timestamppb.New(d.UpdatedAt), + Kind: "GatewayNetwork", + Href: "/api/hypershell/v1/gateway_networks/" + d.ID, + Traceparent: d.Traceparent, + Tracestate: d.Tracestate, }, Name: d.Name, Topology: d.Topology, diff --git a/components/api-server/plugins/gatewayNetworks/migration.go b/components/api-server/plugins/gatewayNetworks/migration.go index 6bc5fca42..380f42bd4 100644 --- a/components/api-server/plugins/gatewayNetworks/migration.go +++ b/components/api-server/plugins/gatewayNetworks/migration.go @@ -7,6 +7,26 @@ import ( "github.com/openshift-online/rh-trex-ai/pkg/db" ) +func migrationAddTraceContext() *gormigrate.Migration { + return &gormigrate.Migration{ + ID: "2026082500000003", + Migrate: func(tx *gorm.DB) error { + return tx.Exec(` + ALTER TABLE gateway_networks + ADD COLUMN IF NOT EXISTS traceparent TEXT, + ADD COLUMN IF NOT EXISTS tracestate TEXT + `).Error + }, + Rollback: func(tx *gorm.DB) error { + return tx.Exec(` + ALTER TABLE gateway_networks + DROP COLUMN IF EXISTS traceparent, + DROP COLUMN IF EXISTS tracestate + `).Error + }, + } +} + func migration() *gormigrate.Migration { type GatewayNetwork struct { db.Model diff --git a/components/api-server/plugins/gatewayNetworks/model.go b/components/api-server/plugins/gatewayNetworks/model.go index 91c5711f9..8416b3853 100644 --- a/components/api-server/plugins/gatewayNetworks/model.go +++ b/components/api-server/plugins/gatewayNetworks/model.go @@ -1,12 +1,14 @@ package gatewayNetworks import ( + hypershellapi "github.com/openshift-online/hypershell/components/api-server/pkg/api" "github.com/openshift-online/rh-trex-ai/pkg/api" "gorm.io/gorm" ) type GatewayNetwork struct { api.Meta + hypershellapi.TraceMeta Name string `json:"name"` Topology *string `json:"topology"` TunnelMode *string `json:"tunnel_mode"` diff --git a/components/api-server/plugins/gatewayNetworks/plugin.go b/components/api-server/plugins/gatewayNetworks/plugin.go index 623da11ee..f29a67c96 100644 --- a/components/api-server/plugins/gatewayNetworks/plugin.go +++ b/components/api-server/plugins/gatewayNetworks/plugin.go @@ -94,4 +94,5 @@ func init() { db.RegisterMigration(migration()) db.RegisterMigration(migrationDropFleetId()) + db.RegisterMigration(migrationAddTraceContext()) } diff --git a/components/api-server/plugins/gatewayNetworks/service.go b/components/api-server/plugins/gatewayNetworks/service.go index b8a63cb15..3342a6d96 100644 --- a/components/api-server/plugins/gatewayNetworks/service.go +++ b/components/api-server/plugins/gatewayNetworks/service.go @@ -69,6 +69,7 @@ func (s *sqlGatewayNetworkService) Get(ctx context.Context, id string) (*Gateway } func (s *sqlGatewayNetworkService) Create(ctx context.Context, gatewayNetwork *GatewayNetwork) (*GatewayNetwork, *errors.ServiceError) { + gatewayNetwork.CaptureTraceContext(ctx) gatewayNetwork, err := s.gatewayNetworkDao.Create(ctx, gatewayNetwork) if err != nil { return nil, services.HandleCreateError("GatewayNetwork", err) @@ -93,6 +94,7 @@ func (s *sqlGatewayNetworkService) Replace(ctx context.Context, gatewayNetwork * } defer s.lockFactory.Unlock(ctx, lockOwnerID) + gatewayNetwork.CaptureTraceContext(ctx) gatewayNetwork, err = s.gatewayNetworkDao.Replace(ctx, gatewayNetwork) if err != nil { return nil, services.HandleUpdateError("GatewayNetwork", err) diff --git a/components/api-server/plugins/gatewayReleases/grpc_presenter.go b/components/api-server/plugins/gatewayReleases/grpc_presenter.go index c8d2f3de3..affacd816 100644 --- a/components/api-server/plugins/gatewayReleases/grpc_presenter.go +++ b/components/api-server/plugins/gatewayReleases/grpc_presenter.go @@ -8,11 +8,13 @@ import ( func gatewayReleaseToProto(d *GatewayRelease) *pb.GatewayRelease { return &pb.GatewayRelease{ Metadata: &pb.ObjectReference{ - Id: d.ID, - CreatedAt: timestamppb.New(d.CreatedAt), - UpdatedAt: timestamppb.New(d.UpdatedAt), - Kind: "GatewayRelease", - Href: "/api/hypershell/v1/gateway_releases/" + d.ID, + Id: d.ID, + CreatedAt: timestamppb.New(d.CreatedAt), + UpdatedAt: timestamppb.New(d.UpdatedAt), + Kind: "GatewayRelease", + Href: "/api/hypershell/v1/gateway_releases/" + d.ID, + Traceparent: d.Traceparent, + Tracestate: d.Tracestate, }, Name: d.Name, Image: d.Image, diff --git a/components/api-server/plugins/gatewayReleases/migration.go b/components/api-server/plugins/gatewayReleases/migration.go index 7c6a020c7..3f3a8af44 100644 --- a/components/api-server/plugins/gatewayReleases/migration.go +++ b/components/api-server/plugins/gatewayReleases/migration.go @@ -7,6 +7,26 @@ import ( "github.com/openshift-online/rh-trex-ai/pkg/db" ) +func migrationAddTraceContext() *gormigrate.Migration { + return &gormigrate.Migration{ + ID: "2026082500000004", + Migrate: func(tx *gorm.DB) error { + return tx.Exec(` + ALTER TABLE gateway_releases + ADD COLUMN IF NOT EXISTS traceparent TEXT, + ADD COLUMN IF NOT EXISTS tracestate TEXT + `).Error + }, + Rollback: func(tx *gorm.DB) error { + return tx.Exec(` + ALTER TABLE gateway_releases + DROP COLUMN IF EXISTS traceparent, + DROP COLUMN IF EXISTS tracestate + `).Error + }, + } +} + func migration() *gormigrate.Migration { type GatewayRelease struct { db.Model diff --git a/components/api-server/plugins/gatewayReleases/model.go b/components/api-server/plugins/gatewayReleases/model.go index 7cba990fb..b8f3d0235 100644 --- a/components/api-server/plugins/gatewayReleases/model.go +++ b/components/api-server/plugins/gatewayReleases/model.go @@ -1,12 +1,14 @@ package gatewayReleases import ( + hypershellapi "github.com/openshift-online/hypershell/components/api-server/pkg/api" "github.com/openshift-online/rh-trex-ai/pkg/api" "gorm.io/gorm" ) type GatewayRelease struct { api.Meta + hypershellapi.TraceMeta Name string `json:"name"` Image string `json:"image"` RolloutStrategy *string `json:"rollout_strategy"` diff --git a/components/api-server/plugins/gatewayReleases/plugin.go b/components/api-server/plugins/gatewayReleases/plugin.go index 490645298..84fbb4da1 100644 --- a/components/api-server/plugins/gatewayReleases/plugin.go +++ b/components/api-server/plugins/gatewayReleases/plugin.go @@ -94,4 +94,5 @@ func init() { db.RegisterMigration(migration()) db.RegisterMigration(migrationDropFleetId()) + db.RegisterMigration(migrationAddTraceContext()) } diff --git a/components/api-server/plugins/gatewayReleases/service.go b/components/api-server/plugins/gatewayReleases/service.go index ae6459275..993660d7a 100644 --- a/components/api-server/plugins/gatewayReleases/service.go +++ b/components/api-server/plugins/gatewayReleases/service.go @@ -69,6 +69,7 @@ func (s *sqlGatewayReleaseService) Get(ctx context.Context, id string) (*Gateway } func (s *sqlGatewayReleaseService) Create(ctx context.Context, gatewayRelease *GatewayRelease) (*GatewayRelease, *errors.ServiceError) { + gatewayRelease.CaptureTraceContext(ctx) gatewayRelease, err := s.gatewayReleaseDao.Create(ctx, gatewayRelease) if err != nil { return nil, services.HandleCreateError("GatewayRelease", err) @@ -93,6 +94,7 @@ func (s *sqlGatewayReleaseService) Replace(ctx context.Context, gatewayRelease * } defer s.lockFactory.Unlock(ctx, lockOwnerID) + gatewayRelease.CaptureTraceContext(ctx) gatewayRelease, err = s.gatewayReleaseDao.Replace(ctx, gatewayRelease) if err != nil { return nil, services.HandleUpdateError("GatewayRelease", err) diff --git a/components/api-server/plugins/gateways/grpc_presenter.go b/components/api-server/plugins/gateways/grpc_presenter.go index a4b685b45..2bad31f29 100644 --- a/components/api-server/plugins/gateways/grpc_presenter.go +++ b/components/api-server/plugins/gateways/grpc_presenter.go @@ -10,11 +10,13 @@ import ( func gatewayToProto(d *Gateway) *pb.Gateway { gw := &pb.Gateway{ Metadata: &pb.ObjectReference{ - Id: d.ID, - CreatedAt: timestamppb.New(d.CreatedAt), - UpdatedAt: timestamppb.New(d.UpdatedAt), - Kind: "Gateway", - Href: "/api/hypershell/v1/gateways/" + d.ID, + Id: d.ID, + CreatedAt: timestamppb.New(d.CreatedAt), + UpdatedAt: timestamppb.New(d.UpdatedAt), + Kind: "Gateway", + Href: "/api/hypershell/v1/gateways/" + d.ID, + Traceparent: d.Traceparent, + Tracestate: d.Tracestate, }, Name: d.Name, ClusterId: d.ClusterId, diff --git a/components/api-server/plugins/gateways/migration.go b/components/api-server/plugins/gateways/migration.go index 8262615fa..0ef489626 100644 --- a/components/api-server/plugins/gateways/migration.go +++ b/components/api-server/plugins/gateways/migration.go @@ -7,6 +7,26 @@ import ( "github.com/openshift-online/rh-trex-ai/pkg/db" ) +func migrationAddTraceContext() *gormigrate.Migration { + return &gormigrate.Migration{ + ID: "2026082500000002", + Migrate: func(tx *gorm.DB) error { + return tx.Exec(` + ALTER TABLE gateways + ADD COLUMN IF NOT EXISTS traceparent TEXT, + ADD COLUMN IF NOT EXISTS tracestate TEXT + `).Error + }, + Rollback: func(tx *gorm.DB) error { + return tx.Exec(` + ALTER TABLE gateways + DROP COLUMN IF EXISTS traceparent, + DROP COLUMN IF EXISTS tracestate + `).Error + }, + } +} + func migration() *gormigrate.Migration { type Gateway struct { db.Model diff --git a/components/api-server/plugins/gateways/model.go b/components/api-server/plugins/gateways/model.go index 276b78f59..094d10ac2 100644 --- a/components/api-server/plugins/gateways/model.go +++ b/components/api-server/plugins/gateways/model.go @@ -4,6 +4,7 @@ import ( "encoding/hex" "fmt" + hypershellapi "github.com/openshift-online/hypershell/components/api-server/pkg/api" "github.com/openshift-online/rh-trex-ai/pkg/api" "github.com/segmentio/ksuid" "gorm.io/gorm" @@ -13,6 +14,7 @@ const gatewayNamespacePrefix = "openshell-" type Gateway struct { api.Meta + hypershellapi.TraceMeta Name string `json:"name"` ClusterId string `json:"cluster_id"` ReleaseId string `json:"release_id"` diff --git a/components/api-server/plugins/gateways/plugin.go b/components/api-server/plugins/gateways/plugin.go index d3348deed..6ae7e0062 100644 --- a/components/api-server/plugins/gateways/plugin.go +++ b/components/api-server/plugins/gateways/plugin.go @@ -199,4 +199,5 @@ func init() { db.RegisterMigration(migrationDropDatabaseConfig()) db.RegisterMigration(migrationDropFleetId()) db.RegisterMigration(migrationDropFleetsTable()) + db.RegisterMigration(migrationAddTraceContext()) } diff --git a/components/api-server/plugins/gateways/service.go b/components/api-server/plugins/gateways/service.go index d31fb7f01..0230fd938 100644 --- a/components/api-server/plugins/gateways/service.go +++ b/components/api-server/plugins/gateways/service.go @@ -114,6 +114,7 @@ func (s *sqlGatewayService) Create(ctx context.Context, gateway *Gateway) (*Gate return nil, errors.GeneralError("gateway placement did not assign database_id") } + gateway.CaptureTraceContext(ctx) gateway, err := s.gatewayDao.Create(ctx, gateway) if err != nil { return nil, services.HandleCreateError("Gateway", err) @@ -138,6 +139,7 @@ func (s *sqlGatewayService) Replace(ctx context.Context, gateway *Gateway) (*Gat } defer s.lockFactory.Unlock(ctx, lockOwnerID) + gateway.CaptureTraceContext(ctx) gateway, err = s.gatewayDao.Replace(ctx, gateway) if err != nil { return nil, services.HandleUpdateError("Gateway", err) diff --git a/components/api-server/plugins/managedClusters/grpc_presenter.go b/components/api-server/plugins/managedClusters/grpc_presenter.go index 672727d12..c3a2b7d8c 100644 --- a/components/api-server/plugins/managedClusters/grpc_presenter.go +++ b/components/api-server/plugins/managedClusters/grpc_presenter.go @@ -8,11 +8,13 @@ import ( func managedClusterToProto(d *ManagedCluster) *pb.ManagedCluster { return &pb.ManagedCluster{ Metadata: &pb.ObjectReference{ - Id: d.ID, - CreatedAt: timestamppb.New(d.CreatedAt), - UpdatedAt: timestamppb.New(d.UpdatedAt), - Kind: "ManagedCluster", - Href: "/api/hypershell/v1/managed_clusters/" + d.ID, + Id: d.ID, + CreatedAt: timestamppb.New(d.CreatedAt), + UpdatedAt: timestamppb.New(d.UpdatedAt), + Kind: "ManagedCluster", + Href: "/api/hypershell/v1/managed_clusters/" + d.ID, + Traceparent: d.Traceparent, + Tracestate: d.Tracestate, }, Name: d.Name, Provider: d.Provider, diff --git a/components/api-server/plugins/managedClusters/migration.go b/components/api-server/plugins/managedClusters/migration.go index fec58550a..a41417a5d 100644 --- a/components/api-server/plugins/managedClusters/migration.go +++ b/components/api-server/plugins/managedClusters/migration.go @@ -7,6 +7,26 @@ import ( "github.com/openshift-online/rh-trex-ai/pkg/db" ) +func migrationAddTraceContext() *gormigrate.Migration { + return &gormigrate.Migration{ + ID: "2026082500000005", + Migrate: func(tx *gorm.DB) error { + return tx.Exec(` + ALTER TABLE managed_clusters + ADD COLUMN IF NOT EXISTS traceparent TEXT, + ADD COLUMN IF NOT EXISTS tracestate TEXT + `).Error + }, + Rollback: func(tx *gorm.DB) error { + return tx.Exec(` + ALTER TABLE managed_clusters + DROP COLUMN IF EXISTS traceparent, + DROP COLUMN IF EXISTS tracestate + `).Error + }, + } +} + func migration() *gormigrate.Migration { type ManagedCluster struct { db.Model diff --git a/components/api-server/plugins/managedClusters/model.go b/components/api-server/plugins/managedClusters/model.go index ca4e54cf0..cd1e42a17 100644 --- a/components/api-server/plugins/managedClusters/model.go +++ b/components/api-server/plugins/managedClusters/model.go @@ -1,12 +1,14 @@ package managedClusters import ( + hypershellapi "github.com/openshift-online/hypershell/components/api-server/pkg/api" "github.com/openshift-online/rh-trex-ai/pkg/api" "gorm.io/gorm" ) type ManagedCluster struct { api.Meta + hypershellapi.TraceMeta Name string `json:"name"` Provider string `json:"provider"` Region *string `json:"region"` diff --git a/components/api-server/plugins/managedClusters/plugin.go b/components/api-server/plugins/managedClusters/plugin.go index 6ef37b7e7..53eed8ca4 100644 --- a/components/api-server/plugins/managedClusters/plugin.go +++ b/components/api-server/plugins/managedClusters/plugin.go @@ -94,4 +94,5 @@ func init() { db.RegisterMigration(migration()) db.RegisterMigration(migrationDropFleetId()) + db.RegisterMigration(migrationAddTraceContext()) } diff --git a/components/api-server/plugins/managedClusters/service.go b/components/api-server/plugins/managedClusters/service.go index b6a147133..504f7c9fe 100644 --- a/components/api-server/plugins/managedClusters/service.go +++ b/components/api-server/plugins/managedClusters/service.go @@ -69,6 +69,7 @@ func (s *sqlManagedClusterService) Get(ctx context.Context, id string) (*Managed } func (s *sqlManagedClusterService) Create(ctx context.Context, managedCluster *ManagedCluster) (*ManagedCluster, *errors.ServiceError) { + managedCluster.CaptureTraceContext(ctx) managedCluster, err := s.managedClusterDao.Create(ctx, managedCluster) if err != nil { return nil, services.HandleCreateError("ManagedCluster", err) @@ -93,6 +94,7 @@ func (s *sqlManagedClusterService) Replace(ctx context.Context, managedCluster * } defer s.lockFactory.Unlock(ctx, lockOwnerID) + managedCluster.CaptureTraceContext(ctx) managedCluster, err = s.managedClusterDao.Replace(ctx, managedCluster) if err != nil { return nil, services.HandleUpdateError("ManagedCluster", err) diff --git a/components/api-server/plugins/managedDatabases/grpc_presenter.go b/components/api-server/plugins/managedDatabases/grpc_presenter.go index 20b1fa13f..8026773f9 100644 --- a/components/api-server/plugins/managedDatabases/grpc_presenter.go +++ b/components/api-server/plugins/managedDatabases/grpc_presenter.go @@ -8,11 +8,13 @@ import ( func managedDatabaseToProto(d *ManagedDatabase) *pb.ManagedDatabase { return &pb.ManagedDatabase{ Metadata: &pb.ObjectReference{ - Id: d.ID, - CreatedAt: timestamppb.New(d.CreatedAt), - UpdatedAt: timestamppb.New(d.UpdatedAt), - Kind: "ManagedDatabase", - Href: "/api/hypershell/v1/managed_databases/" + d.ID, + Id: d.ID, + CreatedAt: timestamppb.New(d.CreatedAt), + UpdatedAt: timestamppb.New(d.UpdatedAt), + Kind: "ManagedDatabase", + Href: "/api/hypershell/v1/managed_databases/" + d.ID, + Traceparent: d.Traceparent, + Tracestate: d.Tracestate, }, Name: d.Name, Provider: d.Provider, diff --git a/components/api-server/plugins/managedDatabases/migration.go b/components/api-server/plugins/managedDatabases/migration.go index b5e1063ef..487e0c648 100644 --- a/components/api-server/plugins/managedDatabases/migration.go +++ b/components/api-server/plugins/managedDatabases/migration.go @@ -7,6 +7,26 @@ import ( "github.com/openshift-online/rh-trex-ai/pkg/db" ) +func migrationAddTraceContext() *gormigrate.Migration { + return &gormigrate.Migration{ + ID: "2026082500000006", + Migrate: func(tx *gorm.DB) error { + return tx.Exec(` + ALTER TABLE managed_databases + ADD COLUMN IF NOT EXISTS traceparent TEXT, + ADD COLUMN IF NOT EXISTS tracestate TEXT + `).Error + }, + Rollback: func(tx *gorm.DB) error { + return tx.Exec(` + ALTER TABLE managed_databases + DROP COLUMN IF EXISTS traceparent, + DROP COLUMN IF EXISTS tracestate + `).Error + }, + } +} + func migration() *gormigrate.Migration { type ManagedDatabase struct { db.Model diff --git a/components/api-server/plugins/managedDatabases/model.go b/components/api-server/plugins/managedDatabases/model.go index 9e830267d..b4b45eeb0 100644 --- a/components/api-server/plugins/managedDatabases/model.go +++ b/components/api-server/plugins/managedDatabases/model.go @@ -4,6 +4,7 @@ import ( "encoding/hex" "fmt" + hypershellapi "github.com/openshift-online/hypershell/components/api-server/pkg/api" "github.com/openshift-online/rh-trex-ai/pkg/api" "github.com/segmentio/ksuid" "gorm.io/gorm" @@ -13,6 +14,7 @@ const dbNamespacePrefix = "openshell-db-" type ManagedDatabase struct { api.Meta + hypershellapi.TraceMeta Name string `json:"name"` Provider string `json:"provider"` Namespace string `json:"namespace"` diff --git a/components/api-server/plugins/managedDatabases/plugin.go b/components/api-server/plugins/managedDatabases/plugin.go index 1362e04be..28a1c63c2 100644 --- a/components/api-server/plugins/managedDatabases/plugin.go +++ b/components/api-server/plugins/managedDatabases/plugin.go @@ -95,4 +95,5 @@ func init() { db.RegisterMigration(migration()) db.RegisterMigration(migrationAddNamespace()) db.RegisterMigration(migrationDropFleetId()) + db.RegisterMigration(migrationAddTraceContext()) } diff --git a/components/api-server/plugins/managedDatabases/service.go b/components/api-server/plugins/managedDatabases/service.go index 05c31964a..d96aaaed9 100644 --- a/components/api-server/plugins/managedDatabases/service.go +++ b/components/api-server/plugins/managedDatabases/service.go @@ -107,6 +107,7 @@ func (s *sqlManagedDatabaseService) Create(ctx context.Context, managedDatabase return nil, unsupportedProviderError(managedDatabase.Provider) } + managedDatabase.CaptureTraceContext(ctx) managedDatabase, err := s.managedDatabaseDao.Create(ctx, managedDatabase) if err != nil { return nil, services.HandleCreateError("ManagedDatabase", err) @@ -142,6 +143,7 @@ func (s *sqlManagedDatabaseService) Replace(ctx context.Context, managedDatabase return nil, errors.Validation("provider cannot be changed from %q to %q", persisted.Provider, managedDatabase.Provider) } + managedDatabase.CaptureTraceContext(ctx) managedDatabase, err = s.managedDatabaseDao.Replace(ctx, managedDatabase) if err != nil { return nil, services.HandleUpdateError("ManagedDatabase", err) diff --git a/components/api-server/plugins/roleBindings/grpc_presenter.go b/components/api-server/plugins/roleBindings/grpc_presenter.go index b02241c8e..185c959e7 100644 --- a/components/api-server/plugins/roleBindings/grpc_presenter.go +++ b/components/api-server/plugins/roleBindings/grpc_presenter.go @@ -8,11 +8,13 @@ import ( func roleBindingToProto(rb *RoleBinding, roleName string, username string) *pb.RoleBinding { p := &pb.RoleBinding{ Metadata: &pb.ObjectReference{ - Id: rb.ID, - CreatedAt: timestamppb.New(rb.CreatedAt), - UpdatedAt: timestamppb.New(rb.UpdatedAt), - Kind: "RoleBinding", - Href: "/api/hypershell/v1/role_bindings/" + rb.ID, + Id: rb.ID, + CreatedAt: timestamppb.New(rb.CreatedAt), + UpdatedAt: timestamppb.New(rb.UpdatedAt), + Kind: "RoleBinding", + Href: "/api/hypershell/v1/role_bindings/" + rb.ID, + Traceparent: rb.Traceparent, + Tracestate: rb.Tracestate, }, RoleId: rb.RoleID, Scope: rb.Scope, diff --git a/components/api-server/plugins/roleBindings/migration.go b/components/api-server/plugins/roleBindings/migration.go index 72c3a8e0c..7ebe7b441 100644 --- a/components/api-server/plugins/roleBindings/migration.go +++ b/components/api-server/plugins/roleBindings/migration.go @@ -7,6 +7,26 @@ import ( "github.com/openshift-online/rh-trex-ai/pkg/db" ) +func migrationAddTraceContext() *gormigrate.Migration { + return &gormigrate.Migration{ + ID: "2026082500000007", + Migrate: func(tx *gorm.DB) error { + return tx.Exec(` + ALTER TABLE role_bindings + ADD COLUMN IF NOT EXISTS traceparent TEXT, + ADD COLUMN IF NOT EXISTS tracestate TEXT + `).Error + }, + Rollback: func(tx *gorm.DB) error { + return tx.Exec(` + ALTER TABLE role_bindings + DROP COLUMN IF EXISTS traceparent, + DROP COLUMN IF EXISTS tracestate + `).Error + }, + } +} + func migration() *gormigrate.Migration { type RoleBinding struct { db.Model diff --git a/components/api-server/plugins/roleBindings/model.go b/components/api-server/plugins/roleBindings/model.go index 33c7bb06a..e38d50645 100644 --- a/components/api-server/plugins/roleBindings/model.go +++ b/components/api-server/plugins/roleBindings/model.go @@ -1,6 +1,7 @@ package roleBindings import ( + hypershellapi "github.com/openshift-online/hypershell/components/api-server/pkg/api" "github.com/openshift-online/rh-trex-ai/pkg/api" "gorm.io/gorm" ) @@ -12,6 +13,7 @@ const ( type RoleBinding struct { api.Meta + hypershellapi.TraceMeta RoleID string `json:"role_id" gorm:"index"` Scope string `json:"scope"` UserID *string `json:"user_id" gorm:"index"` diff --git a/components/api-server/plugins/roleBindings/plugin.go b/components/api-server/plugins/roleBindings/plugin.go index 458f8467b..487a2a9c7 100644 --- a/components/api-server/plugins/roleBindings/plugin.go +++ b/components/api-server/plugins/roleBindings/plugin.go @@ -96,4 +96,5 @@ func init() { presenters.RegisterKind(&RoleBinding{}, "RoleBinding") db.RegisterMigration(migration()) + db.RegisterMigration(migrationAddTraceContext()) } diff --git a/components/api-server/plugins/roleBindings/service.go b/components/api-server/plugins/roleBindings/service.go index 16a9c9ac5..49e5252eb 100644 --- a/components/api-server/plugins/roleBindings/service.go +++ b/components/api-server/plugins/roleBindings/service.go @@ -194,6 +194,7 @@ func (s *sqlRoleBindingService) Create(ctx context.Context, rb *RoleBinding) (*R return nil, errors.Forbidden("platform:admin can only be assigned via Keycloak") } + rb.CaptureTraceContext(ctx) rb, createErr := s.rbDao.Create(ctx, rb) if createErr != nil { return nil, services.HandleCreateError("RoleBinding", createErr) diff --git a/components/api-server/proto/hypershell/v1/common.proto b/components/api-server/proto/hypershell/v1/common.proto index 040dde69b..83f387faa 100644 --- a/components/api-server/proto/hypershell/v1/common.proto +++ b/components/api-server/proto/hypershell/v1/common.proto @@ -12,6 +12,8 @@ message ObjectReference { google.protobuf.Timestamp updated_at = 3; string kind = 4; string href = 5; + optional string traceparent = 6; + optional string tracestate = 7; } message ListMeta { diff --git a/components/control-plane/internal/otel/otel_test.go b/components/control-plane/internal/otel/otel_test.go index 266fa4d23..006c54329 100644 --- a/components/control-plane/internal/otel/otel_test.go +++ b/components/control-plane/internal/otel/otel_test.go @@ -54,7 +54,7 @@ func TestStartReconcileSpanDisabled(t *testing.T) { defer func() { enabled = prev }() ctx := context.Background() - ctx2, end := StartReconcileSpan(ctx, "Gateway", "reconcile") + ctx2, end := StartReconcileSpan(ctx, "Gateway", "reconcile", "") end(nil) if ctx2 != ctx { @@ -62,6 +62,60 @@ func TestStartReconcileSpanDisabled(t *testing.T) { } } +func TestParseTraceparentLink(t *testing.T) { + tests := []struct { + name string + traceparent string + wantOK bool + wantTraceID string + wantSpanID string + }{ + { + "valid traceparent", + "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01", + true, + "4bf92f3577b34da6a3ce929d0e0e4736", + "00f067aa0ba902b7", + }, + { + "valid unsampled", + "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-00", + true, + "4bf92f3577b34da6a3ce929d0e0e4736", + "00f067aa0ba902b7", + }, + {"empty string", "", false, "", ""}, + {"too few parts", "00-abc-def", false, "", ""}, + {"too many parts", "00-a-b-c-d", false, "", ""}, + {"all-zero trace ID", "00-00000000000000000000000000000000-00f067aa0ba902b7-01", false, "", ""}, + {"all-zero span ID", "00-4bf92f3577b34da6a3ce929d0e0e4736-0000000000000000-01", false, "", ""}, + {"invalid hex in trace ID", "00-ZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ-00f067aa0ba902b7-01", false, "", ""}, + {"short trace ID", "00-4bf92f35-00f067aa0ba902b7-01", false, "", ""}, + {"invalid flags hex", "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-ZZ", false, "", ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + link, ok := parseTraceparentLink(tt.traceparent) + if ok != tt.wantOK { + t.Fatalf("parseTraceparentLink(%q) ok = %v, want %v", tt.traceparent, ok, tt.wantOK) + } + if !ok { + return + } + if got := link.SpanContext.TraceID().String(); got != tt.wantTraceID { + t.Errorf("traceID = %q, want %q", got, tt.wantTraceID) + } + if got := link.SpanContext.SpanID().String(); got != tt.wantSpanID { + t.Errorf("spanID = %q, want %q", got, tt.wantSpanID) + } + if !link.SpanContext.IsRemote() { + t.Error("link should be marked as remote") + } + }) + } +} + func TestStartWatchSpanDisabled(t *testing.T) { prev := enabled enabled = false diff --git a/components/control-plane/internal/otel/reconcile.go b/components/control-plane/internal/otel/reconcile.go index 1f5d9bf75..ea982305f 100644 --- a/components/control-plane/internal/otel/reconcile.go +++ b/components/control-plane/internal/otel/reconcile.go @@ -2,6 +2,8 @@ package otel import ( "context" + "encoding/hex" + "strings" "time" "go.opentelemetry.io/otel" @@ -17,7 +19,11 @@ import ( // must call the returned end function when the reconcile completes, passing // any error. When telemetry is disabled, it returns the original context // and a no-op end function so there is zero overhead (CP-OBS-01). -func StartReconcileSpan(ctx context.Context, kind, eventType string) (context.Context, func(error)) { +// +// When traceparent is non-empty, the span carries a link to the originating +// request trace (RTC-03). A missing or malformed traceparent produces a +// normal root with no link and no error. +func StartReconcileSpan(ctx context.Context, kind, eventType, traceparent string) (context.Context, func(error)) { if !enabled { return ctx, func(error) {} } @@ -25,12 +31,19 @@ func StartReconcileSpan(ctx context.Context, kind, eventType string) (context.Co tracer := otel.Tracer(TracerName) spanName := eventType + " " + kind - ctx, span := tracer.Start(ctx, spanName, + opts := []trace.SpanStartOption{ trace.WithNewRoot(), trace.WithAttributes( attribute.String("resource.kind", kind), attribute.String("event.type", eventType), - )) + ), + } + + if link, ok := parseTraceparentLink(traceparent); ok { + opts = append(opts, trace.WithLinks(link)) + } + + ctx, span := tracer.Start(ctx, spanName, opts...) start := time.Now() return ctx, func(err error) { @@ -45,6 +58,56 @@ func StartReconcileSpan(ctx context.Context, kind, eventType string) (context.Co } } +// parseTraceparentLink parses a W3C traceparent header value and returns a +// span link to the referenced trace/span. Returns false if the value is +// empty, malformed, or contains an invalid trace/span ID. +func parseTraceparentLink(traceparent string) (trace.Link, bool) { + if traceparent == "" { + return trace.Link{}, false + } + + parts := strings.Split(traceparent, "-") + if len(parts) != 4 { + return trace.Link{}, false + } + + traceIDHex := parts[1] + spanIDHex := parts[2] + flagsHex := parts[3] + + traceIDBytes, err := hex.DecodeString(traceIDHex) + if err != nil || len(traceIDBytes) != 16 { + return trace.Link{}, false + } + var traceID trace.TraceID + copy(traceID[:], traceIDBytes) + + spanIDBytes, err := hex.DecodeString(spanIDHex) + if err != nil || len(spanIDBytes) != 8 { + return trace.Link{}, false + } + var spanID trace.SpanID + copy(spanID[:], spanIDBytes) + + if !traceID.IsValid() || !spanID.IsValid() { + return trace.Link{}, false + } + + flagsByte, err := hex.DecodeString(flagsHex) + if err != nil || len(flagsByte) != 1 { + return trace.Link{}, false + } + + sc := trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.TraceFlags(flagsByte[0]), + Remote: true, + }) + + return trace.Link{SpanContext: sc}, true +} + // StartWatchSpan begins a lifecycle span for a single watch stream // connection attempt. When telemetry is disabled it returns the original // context and a no-op end function (CP-OBS-01). diff --git a/components/control-plane/internal/reconciler/health.go b/components/control-plane/internal/reconciler/health.go index 7de61b5cd..5b47da9e7 100644 --- a/components/control-plane/internal/reconciler/health.go +++ b/components/control-plane/internal/reconciler/health.go @@ -165,7 +165,7 @@ func (h *GatewayHealthReconciler) Run(ctx context.Context) error { } func (h *GatewayHealthReconciler) reconcileOnce(ctx context.Context) { - ctx, endSpan := cpotel.StartReconcileSpan(ctx, "gateway-health", "reconcile") + ctx, endSpan := cpotel.StartReconcileSpan(ctx, "gateway-health", "reconcile", "") var tickErr error defer func() { endSpan(tickErr) }() diff --git a/components/control-plane/internal/reconciler/namespace.go b/components/control-plane/internal/reconciler/namespace.go index 9db22e81b..03a4a82e1 100644 --- a/components/control-plane/internal/reconciler/namespace.go +++ b/components/control-plane/internal/reconciler/namespace.go @@ -114,7 +114,7 @@ func (r *NamespaceGCReconciler) Run(ctx context.Context) error { } func (r *NamespaceGCReconciler) reconcileOnce(ctx context.Context) { - ctx, endSpan := cpotel.StartReconcileSpan(ctx, "namespace-gc", "reconcile") + ctx, endSpan := cpotel.StartReconcileSpan(ctx, "namespace-gc", "reconcile", "") var tickErr error defer func() { endSpan(tickErr) }() diff --git a/components/control-plane/internal/reconciler/reconciler.go b/components/control-plane/internal/reconciler/reconciler.go index c7bc5af8a..aa480d963 100644 --- a/components/control-plane/internal/reconciler/reconciler.go +++ b/components/control-plane/internal/reconciler/reconciler.go @@ -63,7 +63,7 @@ func (r *ManagedClusterReconciler) Handle(ctx context.Context, event watcher.Eve r.mu.Unlock() }() - _, endSpan := cpotel.StartReconcileSpan(ctx, "ManagedCluster", event.Type.String()) + _, endSpan := cpotel.StartReconcileSpan(ctx, "ManagedCluster", event.Type.String(), event.Resource.GetMetadata().GetTraceparent()) defer func() { endSpan(nil) }() log.Printf("INFO reconciling ManagedCluster %s (event=%d)", event.ResourceID, event.Type) @@ -148,7 +148,7 @@ func (r *ManagedDatabaseReconciler) Handle(ctx context.Context, event watcher.Ev } func (r *ManagedDatabaseReconciler) handleOne(ctx context.Context, event watcher.Event[*pb.ManagedDatabase]) (reconcileErr error) { - ctx, endSpan := cpotel.StartReconcileSpan(ctx, "ManagedDatabase", event.Type.String()) + ctx, endSpan := cpotel.StartReconcileSpan(ctx, "ManagedDatabase", event.Type.String(), event.Resource.GetMetadata().GetTraceparent()) defer func() { endSpan(reconcileErr) }() if r.clientset == nil || r.dynamicClient == nil { @@ -1151,7 +1151,7 @@ func (r *GatewayReleaseReconciler) Handle(ctx context.Context, event watcher.Eve r.mu.Unlock() }() - _, endSpan := cpotel.StartReconcileSpan(ctx, "GatewayRelease", event.Type.String()) + _, endSpan := cpotel.StartReconcileSpan(ctx, "GatewayRelease", event.Type.String(), event.Resource.GetMetadata().GetTraceparent()) defer func() { endSpan(nil) }() log.Printf("INFO reconciling GatewayRelease %s (event=%d)", event.ResourceID, event.Type) @@ -1258,7 +1258,7 @@ func (r *GatewayReconciler) Handle(ctx context.Context, event watcher.Event[*pb. } suppressGatewayProvisionObservation(event.ResourceID, previousPhase) - ctx, endSpan := cpotel.StartReconcileSpan(ctx, "Gateway", event.Type.String()) + ctx, endSpan := cpotel.StartReconcileSpan(ctx, "Gateway", event.Type.String(), gw.GetMetadata().GetTraceparent()) span := trace.SpanFromContext(ctx) span.SetAttributes(attribute.String("hypershell.resource_id", event.ResourceID)) var reconcileErr error @@ -2164,7 +2164,7 @@ func (r *GatewayNetworkReconciler) Handle(ctx context.Context, event watcher.Eve r.mu.Unlock() }() - _, endSpan := cpotel.StartReconcileSpan(ctx, "GatewayNetwork", event.Type.String()) + _, endSpan := cpotel.StartReconcileSpan(ctx, "GatewayNetwork", event.Type.String(), event.Resource.GetMetadata().GetTraceparent()) defer func() { endSpan(nil) }() log.Printf("INFO reconciling GatewayNetwork %s (event=%d)", event.ResourceID, event.Type) diff --git a/components/control-plane/internal/reconciler/role_binding_reconciler.go b/components/control-plane/internal/reconciler/role_binding_reconciler.go index 9c80f92d4..9746c6451 100644 --- a/components/control-plane/internal/reconciler/role_binding_reconciler.go +++ b/components/control-plane/internal/reconciler/role_binding_reconciler.go @@ -63,7 +63,7 @@ func (r *RoleBindingReconciler) Handle(ctx context.Context, event watcher.Event[ return nil } - ctx, endSpan := cpotel.StartReconcileSpan(ctx, "RoleBinding", event.Type.String()) + ctx, endSpan := cpotel.StartReconcileSpan(ctx, "RoleBinding", event.Type.String(), rb.GetMetadata().GetTraceparent()) var reconcileErr error defer func() { endSpan(reconcileErr) }() diff --git a/components/control-plane/internal/reconciler/sandboxcount.go b/components/control-plane/internal/reconciler/sandboxcount.go index dfb116e62..4e4dba45e 100644 --- a/components/control-plane/internal/reconciler/sandboxcount.go +++ b/components/control-plane/internal/reconciler/sandboxcount.go @@ -262,7 +262,7 @@ func (r *SandboxCountReconciler) lockNamespace(namespace string) func() { // write. Each set RPC is bounded by sandboxCountRPCTimeout so a single hung call // cannot stall the rest of the pass. func (r *SandboxCountReconciler) selfHeal(ctx context.Context, lister corelisters.PodLister) { - ctx, endSpan := cpotel.StartReconcileSpan(ctx, "sandbox-count", "reconcile") + ctx, endSpan := cpotel.StartReconcileSpan(ctx, "sandbox-count", "reconcile", "") var tickErr error defer func() { endSpan(tickErr) }() diff --git a/specs/index.spec.md b/specs/index.spec.md index 05bbd639c..d5c975d5c 100644 --- a/specs/index.spec.md +++ b/specs/index.spec.md @@ -54,6 +54,7 @@ Machine-readable index for autonomous reconciliation (`/reconcile` skill). | `platform/e2e-testing.spec.md` | platform | Infra drivers, e2e test suite, CI workflow, deploy overlays | ALL | local-development, control-plane, openshell-gateway-routing | | `platform/api-server-observability.spec.md` | platform | API OTel SDK bootstrap, HTTP/gRPC server spans, W3C trace continuation, request metrics | API | web-console/tracing, security, local-development, e2e-testing | | `platform/control-plane-observability.spec.md` | platform | CP OTel SDK bootstrap, reconcile spans, gRPC client spans, watch lifecycle, K8s API spans, reconcile metrics | CP | api-server-observability, control-plane, security, local-development | +| `platform/reconcile-trace-correlation.spec.md` | platform | Trace context persistence, span links, reconcile-to-request correlation | API, CP | api-server-observability, control-plane-observability, data-model | | `standards/ui/foundations.spec.md` | standards | UI foundations | WEB | - | | `standards/ui/brand-color.spec.md` | standards | Red Hat brand color | WEB | foundations, accessibility | | `standards/ui/interaction.spec.md` | standards | UI interaction | WEB | foundations | diff --git a/specs/platform/control-plane-observability.spec.md b/specs/platform/control-plane-observability.spec.md index 456f065f6..ad5e7d2bd 100644 --- a/specs/platform/control-plane-observability.spec.md +++ b/specs/platform/control-plane-observability.spec.md @@ -8,7 +8,7 @@ Give the HyperShell control plane distributed tracing and reconcile-level metrics through OpenTelemetry (OTel), so an operator can observe reconcile latency, gRPC watch health, Kubernetes API calls, and failures across the fleet. This specification is the control-plane counterpart to `platform/api-server-observability.spec.md` (HYPERSHELL-26) and `web-console/tracing.spec.md` (HYPERSHELL-27): the API server already produces server spans for inbound HTTP and gRPC requests, and this specification makes the control plane produce spans for the asynchronous reconciliation work that follows. -Correlating a reconcile trace back to the originating user request is intentionally deferred to a follow-up story, because reconciliation is asynchronous: the API writes desired state to PostgreSQL and returns; the control plane observes the change later via a watch stream, possibly after resync, batching, or retries. That correlation is tracked separately. +Correlating a reconcile trace back to the originating user request is defined by `platform/reconcile-trace-correlation.spec.md`, which adds span links from the reconcile root span to the originating request trace. That specification extends this one and `platform/api-server-observability.spec.md`. This specification covers the control plane component only. API server instrumentation is defined by `platform/api-server-observability.spec.md`. Where `standards/security/security.spec.md` imposes a stricter rule on what may appear in telemetry, that rule governs. @@ -275,7 +275,7 @@ When `KIND_JAEGER` is unset, the control plane Deployment SHALL NOT receive a co | Bounded span names by kind, not resource ID | Keeps Jaeger grouping useful and prevents cardinality explosion across large deployments | | Resource ID as a span attribute, not a span name | Enables per-trace debugging without inflating the span-name namespace | | OTLP/gRPC on port 4317 for the control plane | Matches the API server's transport; the development Jaeger exposes 4317 for OTLP/gRPC | -| Reconcile-trace to request-trace correlation deferred | Reconciliation is asynchronous; the correlation mechanism (span links, trace-context persistence) deserves its own story | +| Reconcile-trace to request-trace correlation via span links | Reconciliation is asynchronous; the correlation mechanism (span links, trace-context persistence) is defined in `platform/reconcile-trace-correlation.spec.md` | ## Primary Basis diff --git a/specs/platform/reconcile-trace-correlation.spec.md b/specs/platform/reconcile-trace-correlation.spec.md new file mode 100644 index 000000000..9d427cc36 --- /dev/null +++ b/specs/platform/reconcile-trace-correlation.spec.md @@ -0,0 +1,162 @@ +# Reconcile-to-Request Trace Correlation + +**Status:** Draft +**Applies to:** `components/api-server` resource persistence and gRPC watch messages, `components/control-plane` reconcile spans, and the W3C Trace Context stored on each resource +**Jira:** (to be assigned) + +## Purpose + +When an operator investigates a reconcile failure, they need to trace the causal chain: which user action triggered the change that the control plane is now reconciling? Today the control plane produces reconcile spans (CP-OBS-02) and the API server produces request spans (API-OBS-02/03), but the two traces are disconnected because reconciliation is temporally decoupled from the user request. The API server writes desired state to PostgreSQL and returns; the control plane observes the change later via a gRPC watch stream, possibly after resync, batching, or retries. + +A synchronous parent-child span relationship is therefore incorrect: it would produce a child span that starts long after its parent ended, or one request span with many reconcile children from resyncs. The correct OpenTelemetry model is a **span link** ("caused by"): the reconcile span keeps its own root trace (independently sampled per CP-OBS-02) and carries a link to the originating request trace, so a support engineer can navigate from the reconcile trace to the request trace in Jaeger. + +This specification defines how the originating trace context flows from the API server request, through the database, over the gRPC watch stream, into the control plane reconcile span as a link. It extends `platform/api-server-observability.spec.md` (HYPERSHELL-26) and `platform/control-plane-observability.spec.md` (HYPERSHELL-79). + +## Requirements + +### Requirement: RTC-01 -- Originating Trace Context Persistence + +The API server SHALL capture the W3C Trace Context (`traceparent` header value, and `tracestate` when present) from the inbound request context on every create and update write, and SHALL persist both values on the resource row in PostgreSQL. The trace context SHALL be stored as plain text columns (`traceparent` and `tracestate`) on the shared `api.Meta` base, so every resource type inherits the field without per-plugin schema changes. + +Because the `api.Meta` base struct is defined in the upstream `rh-trex-ai` framework and cannot be modified in-tree, the trace context columns SHALL be added via a local embeddable struct (for example `TraceMeta`) that each resource model embeds alongside `api.Meta`. A single gormigrate migration SHALL add the columns to all resource tables. + +The `traceparent` column SHALL store the W3C Trace Context `traceparent` header value (for example `00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01`). The `tracestate` column SHALL store the W3C `tracestate` header value when present, or be empty when absent. Both columns SHALL be nullable: a resource created before this change, or created when telemetry is disabled, SHALL have NULL trace context and that is a valid state. + +The trace context SHALL be captured from the active span context at the point of persistence (not from the raw HTTP header), so it reflects the actual span that performed the write. On an update, the stored trace context SHALL be overwritten with the new request's context, so the trace context always points to the most recent mutation. + +**Verification:** Create a resource via the API with a valid inbound `traceparent`; confirm the database row stores the `traceparent` and `tracestate` values. Update the resource with a different request trace; confirm the stored values are overwritten. + +#### Scenario: Create persists originating trace context + +- GIVEN the OTel SDK is initialized on the API server +- AND a client sends a POST request carrying a valid `traceparent` header +- WHEN the API server creates the resource +- THEN the resource row in PostgreSQL SHALL contain the `traceparent` value from the request's active span context +- AND `tracestate` SHALL be stored when present in the inbound context + +#### Scenario: Update overwrites trace context + +- GIVEN a resource with a stored `traceparent` from its creation +- WHEN a client sends a PATCH request with a different trace context +- THEN the stored `traceparent` SHALL be overwritten with the new request's active span context +- AND the previous trace context SHALL not be retained + +#### Scenario: No trace context when telemetry is disabled + +- GIVEN the OTel SDK is not initialized (no `OTEL_EXPORTER_OTLP_ENDPOINT`) +- WHEN a client creates a resource +- THEN the `traceparent` and `tracestate` columns SHALL be NULL +- AND the resource SHALL be created normally + +### Requirement: RTC-02 -- Trace Context on gRPC Watch Messages + +The resource protobuf messages carried in gRPC watch responses SHALL include the originating trace context so the control plane can read it without a separate lookup. The `traceparent` and `tracestate` fields SHALL be added to the shared `ObjectReference` message so every resource type's watch response carries them uniformly. + +The fields SHALL be optional strings. When the stored trace context is NULL (resource created before this change or with telemetry disabled), the fields SHALL be empty in the protobuf message. + +**Verification:** Create a resource with an active trace; receive its watch event and confirm the `traceparent` field on the resource's metadata matches the stored value. Create a resource with no trace context and confirm the fields are empty. + +#### Scenario: Watch event carries trace context + +- GIVEN a resource created with a stored `traceparent` +- WHEN the control plane receives a watch event for that resource +- THEN the resource's `ObjectReference` metadata SHALL contain the `traceparent` value +- AND `tracestate` SHALL be present when the resource has a stored value + +#### Scenario: Watch event with no trace context + +- GIVEN a resource created before trace context persistence was added +- WHEN the control plane receives a watch event for that resource +- THEN the `traceparent` and `tracestate` fields on `ObjectReference` SHALL be empty strings +- AND the watch event SHALL be processed normally + +### Requirement: RTC-03 -- Reconcile Span Link to Originating Trace + +The control plane SHALL parse the `traceparent` (and `tracestate` when present) from the watched resource's metadata, extract the trace ID and span ID, and attach them as a span link on the reconcile root span. The span link SHALL use the OpenTelemetry `trace.Link` with the remote span context so Jaeger renders the link as a "caused by" relationship. + +The reconcile span SHALL remain a new trace root (per CP-OBS-02) and SHALL NOT become a child of the originating request span. The span link is a causal reference, not a parent-child relationship. The reconcile span's sampling decision SHALL remain independent of the originating trace's sampling decision. + +When the resource has no stored trace context (NULL `traceparent`), the reconcile span SHALL be a normal root with no link and no error. A missing or malformed `traceparent` SHALL NOT cause a reconcile failure or produce a warning; it SHALL be silently ignored. + +**Verification:** Create a resource with an active trace; trigger reconciliation and confirm the reconcile span in Jaeger carries a link to the originating request trace. Reconcile a resource with no trace context and confirm a normal root span with no link. + +#### Scenario: Reconcile span links to originating request + +- GIVEN a resource with a stored `traceparent` of `00-{traceID}-{spanID}-01` +- WHEN the control plane reconciles the resource +- THEN the reconcile root span SHALL carry a span link to the trace identified by `{traceID}` and `{spanID}` +- AND the reconcile span SHALL remain a new trace root with its own trace ID +- AND Jaeger SHALL render the link as a navigable reference from the reconcile trace to the request trace + +#### Scenario: Reconcile without trace context produces no link + +- GIVEN a resource with no stored `traceparent` (NULL or empty) +- WHEN the control plane reconciles the resource +- THEN the reconcile span SHALL be a normal root with no span link +- AND no error or warning SHALL be logged + +#### Scenario: Malformed traceparent is silently ignored + +- GIVEN a resource with a stored `traceparent` value that does not conform to W3C Trace Context +- WHEN the control plane reconciles the resource +- THEN the reconcile span SHALL be a normal root with no span link +- AND no reconcile error SHALL be raised + +### Requirement: RTC-04 -- End-to-End Trace Navigation in Jaeger + +In the development Jaeger (when `KIND_JAEGER=true`, per CP-OBS-08 and API-OBS-07), a support engineer SHALL be able to navigate from a reconcile trace to the originating user request trace. The Jaeger UI SHALL show the span link on the reconcile span and allow clicking through to the originating trace. The originating trace SHALL include the browser workflow span, the BFF server span, the API server span, and the database span (when all components have OTel enabled). + +This requirement is a verification-only requirement: it does not impose new code beyond RTC-01 through RTC-03, but it validates the end-to-end experience. + +**Verification:** With `KIND_JAEGER=true`, create a gateway through the console; wait for reconciliation; find the reconcile span in Jaeger and confirm it has a link to the browser-to-API trace for the same gateway operation. + +#### Scenario: End-to-end trace navigation + +- GIVEN the local cluster is running with `KIND_JAEGER=true` +- AND the API server, control plane, and web console all have OTel enabled +- WHEN a developer creates a gateway through the console +- AND the control plane reconciles the gateway +- THEN the reconcile trace in Jaeger SHALL show a span link to the originating request trace +- AND clicking the link SHALL navigate to the trace containing the browser, BFF, API server, and database spans + +### Requirement: RTC-05 -- Privacy and Cardinality + +The trace context stored on a resource and carried in watch messages is opaque W3C Trace Context: a `traceparent` string containing version, trace ID, span ID, and trace flags, and an optional `tracestate` string containing vendor-specific key-value pairs. These values SHALL NOT contain sensitive data by construction (they are hex-encoded identifiers and vendor flags). The trace context SHALL NOT be exposed in the REST API responses; it is internal to the observability pipeline. + +The span link on the reconcile span records the linked trace ID and span ID as span attributes. These are bounded hex identifiers and do not affect span-name cardinality or metric label cardinality, consistent with CP-OBS-06. + +#### Scenario: Trace context not exposed in REST responses + +- GIVEN a resource with a stored `traceparent` +- WHEN a client retrieves the resource via the REST API +- THEN the `traceparent` and `tracestate` fields SHALL NOT appear in the JSON response + +#### Scenario: Trace context values do not contain sensitive data + +- GIVEN a resource created with an inbound `traceparent` +- WHEN the stored value is inspected +- THEN it SHALL be a W3C Trace Context string containing only hex-encoded identifiers and flags +- AND it SHALL NOT contain bearer tokens, user identifiers, or any sensitive data + +## Design Decisions + +| Decision | Rationale | +| --- | --- | +| Span link, not parent-child | Reconciliation is asynchronous and may happen long after the request returns. A parent-child relationship would create a span tree where the child outlives the parent, breaking trace semantics. A span link preserves causal reference without implying temporal containment. | +| Local `TraceMeta` embed alongside `api.Meta` | The upstream `api.Meta` is owned by the `rh-trex-ai` framework and cannot be modified in-tree. A local embeddable struct keeps the change self-contained. Each plugin model embeds it, and a single migration adds the columns to all tables. | +| `traceparent` and `tracestate` as separate text columns | Matches the W3C Trace Context header structure. Two columns are simpler than a JSON blob and allow direct extraction without parsing. | +| Fields on `ObjectReference` in protobuf, not on each resource message | `ObjectReference` is the shared metadata message embedded in every resource. Adding the fields there propagates to all watch responses without per-resource proto changes. | +| Capture from active span context, not raw header | The active span context reflects the actual sampled span that performed the write. The raw header might not match if the server started a new root or the header was malformed. | +| Overwrite on update, not append | The most recent mutation is the one the control plane will reconcile. Maintaining a history of trace contexts would complicate the schema and provide limited value. | +| Silent ignore on missing or malformed traceparent | Pre-existing resources and disabled-telemetry deployments must work without errors. The link is best-effort observability, not a correctness requirement. | +| Trace context not exposed in REST API | The trace context is an internal observability concern. Exposing it in the REST API would leak infrastructure details and create an unnecessary contract. | + +## Primary Basis + +- `platform/control-plane-observability.spec.md` (HYPERSHELL-79) -- reconcile spans, `WithNewRoot()`, CP-OBS-02, CP-OBS-06 +- `platform/api-server-observability.spec.md` (HYPERSHELL-26) -- API server spans, trace context extraction, API-OBS-04 +- `platform/data-model.spec.md` -- `api.Meta` base type, resource schema +- `web-console/tracing.spec.md` (HYPERSHELL-27) -- browser and BFF spans +- `standards/security/security.spec.md` -- telemetry privacy +- [W3C Trace Context](https://www.w3.org/TR/trace-context/) -- `traceparent` and `tracestate` format +- [OpenTelemetry Span Links](https://opentelemetry.io/docs/concepts/signals/traces/#span-links) -- causal reference semantics From 93e08d190efb39f223b8bb06ba23b624b072de0b Mon Sep 17 00:00:00 2001 From: JuanmaBM Date: Thu, 27 Aug 2026 09:31:01 +0200 Subject: [PATCH 2/3] fix: add TraceMeta unit tests and proto field documentation Add table-driven tests for CaptureTraceContext covering valid/invalid span contexts, tracestate propagation, unsampled flags, and idempotency. Document traceparent and tracestate fields in the ObjectReference proto message with W3C Trace Context format and emptiness semantics. Co-Authored-By: Claude Opus 4.6 --- .../pkg/api/grpc/hypershell/v1/common.pb.go | 21 ++- .../api-server/pkg/api/tracemeta_test.go | 138 ++++++++++++++++++ .../proto/hypershell/v1/common.proto | 5 + 3 files changed, 156 insertions(+), 8 deletions(-) create mode 100644 components/api-server/pkg/api/tracemeta_test.go diff --git a/components/api-server/pkg/api/grpc/hypershell/v1/common.pb.go b/components/api-server/pkg/api/grpc/hypershell/v1/common.pb.go index e2b09e105..1d012d6bc 100644 --- a/components/api-server/pkg/api/grpc/hypershell/v1/common.pb.go +++ b/components/api-server/pkg/api/grpc/hypershell/v1/common.pb.go @@ -75,14 +75,19 @@ func (EventType) EnumDescriptor() ([]byte, []int) { } type ObjectReference struct { - state protoimpl.MessageState `protogen:"open.v1"` - Id string `protobuf:"bytes,1,opt,name=id,proto3" json:"id,omitempty"` - CreatedAt *timestamppb.Timestamp `protobuf:"bytes,2,opt,name=created_at,json=createdAt,proto3" json:"created_at,omitempty"` - UpdatedAt *timestamppb.Timestamp `protobuf:"bytes,3,opt,name=updated_at,json=updatedAt,proto3" json:"updated_at,omitempty"` - Kind string `protobuf:"bytes,4,opt,name=kind,proto3" json:"kind,omitempty"` - Href string `protobuf:"bytes,5,opt,name=href,proto3" json:"href,omitempty"` - Traceparent *string `protobuf:"bytes,6,opt,name=traceparent,proto3,oneof" json:"traceparent,omitempty"` - Tracestate *string `protobuf:"bytes,7,opt,name=tracestate,proto3,oneof" json:"tracestate,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + Id string `protobuf:"bytes,1,opt,name=id,proto3" json:"id,omitempty"` + CreatedAt *timestamppb.Timestamp `protobuf:"bytes,2,opt,name=created_at,json=createdAt,proto3" json:"created_at,omitempty"` + UpdatedAt *timestamppb.Timestamp `protobuf:"bytes,3,opt,name=updated_at,json=updatedAt,proto3" json:"updated_at,omitempty"` + Kind string `protobuf:"bytes,4,opt,name=kind,proto3" json:"kind,omitempty"` + Href string `protobuf:"bytes,5,opt,name=href,proto3" json:"href,omitempty"` + // W3C Trace Context traceparent of the API request that created or last + // updated this resource (e.g. "00-{traceID}-{spanID}-{flags}"). + // Empty when OpenTelemetry is disabled or the resource predates tracing. + Traceparent *string `protobuf:"bytes,6,opt,name=traceparent,proto3,oneof" json:"traceparent,omitempty"` + // W3C Trace Context tracestate carrying vendor-specific key-value pairs. + // Empty when no vendor state was present on the originating request. + Tracestate *string `protobuf:"bytes,7,opt,name=tracestate,proto3,oneof" json:"tracestate,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } diff --git a/components/api-server/pkg/api/tracemeta_test.go b/components/api-server/pkg/api/tracemeta_test.go new file mode 100644 index 000000000..0da920f9f --- /dev/null +++ b/components/api-server/pkg/api/tracemeta_test.go @@ -0,0 +1,138 @@ +package api + +import ( + "context" + "testing" + + "go.opentelemetry.io/otel/trace" +) + +func TestCaptureTraceContext(t *testing.T) { + traceID, _ := trace.TraceIDFromHex("0af7651916cd43dd8448eb211c80319c") + spanID, _ := trace.SpanIDFromHex("b7ad6b7169203331") + + tests := []struct { + name string + ctx context.Context + wantTraceparent *string + wantTracestate *string + }{ + { + name: "no span in context leaves fields nil", + ctx: context.Background(), + wantTraceparent: nil, + wantTracestate: nil, + }, + { + name: "invalid span context leaves fields nil", + ctx: trace.ContextWithSpanContext(context.Background(), trace.SpanContext{}), + wantTraceparent: nil, + wantTracestate: nil, + }, + { + name: "valid span context sets traceparent", + ctx: trace.ContextWithSpanContext(context.Background(), + trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsSampled, + Remote: true, + }), + ), + wantTraceparent: strPtr("00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01"), + wantTracestate: nil, + }, + { + name: "valid span context with tracestate sets both fields", + ctx: trace.ContextWithSpanContext(context.Background(), + trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsSampled, + TraceState: mustTraceState(t, "vendor=opaque"), + Remote: true, + }), + ), + wantTraceparent: strPtr("00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01"), + wantTracestate: strPtr("vendor=opaque"), + }, + { + name: "unsampled span still captures traceparent with flags 00", + ctx: trace.ContextWithSpanContext(context.Background(), + trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: 0, + Remote: true, + }), + ), + wantTraceparent: strPtr("00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-00"), + wantTracestate: nil, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var tm TraceMeta + tm.CaptureTraceContext(tc.ctx) + + if !strPtrEq(tm.Traceparent, tc.wantTraceparent) { + t.Errorf("Traceparent = %s, want %s", strPtrFmt(tm.Traceparent), strPtrFmt(tc.wantTraceparent)) + } + if !strPtrEq(tm.Tracestate, tc.wantTracestate) { + t.Errorf("Tracestate = %s, want %s", strPtrFmt(tm.Tracestate), strPtrFmt(tc.wantTracestate)) + } + }) + } +} + +func TestCaptureTraceContextIsIdempotent(t *testing.T) { + traceID, _ := trace.TraceIDFromHex("0af7651916cd43dd8448eb211c80319c") + spanID, _ := trace.SpanIDFromHex("b7ad6b7169203331") + + ctx := trace.ContextWithSpanContext(context.Background(), + trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: traceID, + SpanID: spanID, + TraceFlags: trace.FlagsSampled, + Remote: true, + }), + ) + + var tm TraceMeta + tm.CaptureTraceContext(ctx) + first := *tm.Traceparent + + tm.CaptureTraceContext(ctx) + if *tm.Traceparent != first { + t.Errorf("second call changed Traceparent: got %s, want %s", *tm.Traceparent, first) + } +} + +func mustTraceState(t *testing.T, s string) trace.TraceState { + t.Helper() + ts, err := trace.ParseTraceState(s) + if err != nil { + t.Fatalf("ParseTraceState(%q): %v", s, err) + } + return ts +} + +func strPtr(s string) *string { return &s } + +func strPtrEq(a, b *string) bool { + if a == nil && b == nil { + return true + } + if a == nil || b == nil { + return false + } + return *a == *b +} + +func strPtrFmt(s *string) string { + if s == nil { + return "" + } + return *s +} diff --git a/components/api-server/proto/hypershell/v1/common.proto b/components/api-server/proto/hypershell/v1/common.proto index 83f387faa..fca145cf8 100644 --- a/components/api-server/proto/hypershell/v1/common.proto +++ b/components/api-server/proto/hypershell/v1/common.proto @@ -12,7 +12,12 @@ message ObjectReference { google.protobuf.Timestamp updated_at = 3; string kind = 4; string href = 5; + // W3C Trace Context traceparent of the API request that created or last + // updated this resource (e.g. "00-{traceID}-{spanID}-{flags}"). + // Empty when OpenTelemetry is disabled or the resource predates tracing. optional string traceparent = 6; + // W3C Trace Context tracestate carrying vendor-specific key-value pairs. + // Empty when no vendor state was present on the originating request. optional string tracestate = 7; } From c7e61cbac895403e96ebf38e8bbc61a95f6b5d6f Mon Sep 17 00:00:00 2001 From: JuanmaBM Date: Tue, 8 Sep 2026 09:14:49 +0200 Subject: [PATCH 3/3] docs: address review feedback on trace correlation - Reconcile RTC-01 spec text with the shipped per-plugin migration approach (migrationAddTraceContext per plugin, unique IDs) instead of a single migration. - Document that an update with telemetry disabled intentionally overwrites stored trace context to NULL (gorm Save writes nil -> NULL), matching RTC-01 most-recent-mutation semantics; add spec scenario and a code comment on CaptureTraceContext. - Document that server-initiated RoleBinding paths (CreateGatewayOwnerBinding, SyncJWTRoles) intentionally skip CaptureTraceContext and are knowingly link-less. Co-Authored-By: Claude Opus 4.8 --- components/api-server/pkg/api/tracemeta.go | 6 ++++++ .../api-server/plugins/roleBindings/service.go | 9 +++++++++ specs/platform/reconcile-trace-correlation.spec.md | 14 +++++++++++--- 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/components/api-server/pkg/api/tracemeta.go b/components/api-server/pkg/api/tracemeta.go index 45db63465..3cb6d730e 100644 --- a/components/api-server/pkg/api/tracemeta.go +++ b/components/api-server/pkg/api/tracemeta.go @@ -19,6 +19,12 @@ type TraceMeta struct { // CaptureTraceContext extracts the active span's W3C traceparent and // tracestate from ctx and stores them. When no valid span is active (OTel // disabled or no sampled span), the fields are left nil. +// +// On an update path this is deliberate: the DAO persists via gorm Save, which +// writes nil pointers as NULL, so an update performed with telemetry disabled +// overwrites any previously-stored trace context to NULL. That is the intended +// overwrite-on-update semantics (RTC-01) - the columns always describe the most +// recent mutation, and a mutation with no active span has no traceable origin. func (t *TraceMeta) CaptureTraceContext(ctx context.Context) { sc := trace.SpanFromContext(ctx).SpanContext() if !sc.IsValid() { diff --git a/components/api-server/plugins/roleBindings/service.go b/components/api-server/plugins/roleBindings/service.go index 49e5252eb..c22c10b8b 100644 --- a/components/api-server/plugins/roleBindings/service.go +++ b/components/api-server/plugins/roleBindings/service.go @@ -66,6 +66,11 @@ func (s *sqlRoleBindingService) CreateGatewayOwnerBinding(ctx context.Context, u GatewayID: &gatewayID, } + // Intentionally no CaptureTraceContext here: this is a server-initiated + // binding (created as a side effect of gateway provisioning), not a client + // request, so there is no meaningful originating request span to link. The + // trace context stays NULL and the resulting reconcile is knowingly + // link-less (RTC-01 tolerates NULL trace context). _, createErr := s.rbDao.Create(ctx, rb) if createErr != nil { return createErr @@ -125,6 +130,10 @@ func (s *sqlRoleBindingService) SyncJWTRoles(ctx context.Context, userID string, Scope: ScopeGlobal, UserID: &userID, } + // Intentionally no CaptureTraceContext here: JWT role sync is driven by + // the login/token flow, not a client mutation of this resource, so there + // is no originating request span worth linking. NULL trace context is a + // valid state (RTC-01) and these reconciles are knowingly link-less. if _, createErr := s.rbDao.Create(ctx, rb); createErr != nil { return fmt.Errorf("unable to create binding for role %s: %w", roleName, createErr) } diff --git a/specs/platform/reconcile-trace-correlation.spec.md b/specs/platform/reconcile-trace-correlation.spec.md index 9d427cc36..ca3cc2d62 100644 --- a/specs/platform/reconcile-trace-correlation.spec.md +++ b/specs/platform/reconcile-trace-correlation.spec.md @@ -16,13 +16,13 @@ This specification defines how the originating trace context flows from the API ### Requirement: RTC-01 -- Originating Trace Context Persistence -The API server SHALL capture the W3C Trace Context (`traceparent` header value, and `tracestate` when present) from the inbound request context on every create and update write, and SHALL persist both values on the resource row in PostgreSQL. The trace context SHALL be stored as plain text columns (`traceparent` and `tracestate`) on the shared `api.Meta` base, so every resource type inherits the field without per-plugin schema changes. +The API server SHALL capture the W3C Trace Context (`traceparent` header value, and `tracestate` when present) from the inbound request context on every create and update write, and SHALL persist both values on the resource row in PostgreSQL. The trace context SHALL be stored as plain text columns (`traceparent` and `tracestate`) via a shared embeddable struct, so every resource type inherits the same fields with a uniform column layout rather than a bespoke per-plugin field definition. -Because the `api.Meta` base struct is defined in the upstream `rh-trex-ai` framework and cannot be modified in-tree, the trace context columns SHALL be added via a local embeddable struct (for example `TraceMeta`) that each resource model embeds alongside `api.Meta`. A single gormigrate migration SHALL add the columns to all resource tables. +Because the `api.Meta` base struct is defined in the upstream `rh-trex-ai` framework and cannot be modified in-tree, the trace context columns SHALL be added via a local embeddable struct (for example `TraceMeta`) that each resource model embeds alongside `api.Meta`, so every resource type inherits the fields without a per-plugin struct definition. Each resource plugin SHALL register its own gormigrate migration (`migrationAddTraceContext`) that adds the columns to that plugin's table, keeping schema ownership with the plugin that owns the resource. The migration IDs SHALL be unique across plugins so there is no gormigrate collision. The `traceparent` column SHALL store the W3C Trace Context `traceparent` header value (for example `00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01`). The `tracestate` column SHALL store the W3C `tracestate` header value when present, or be empty when absent. Both columns SHALL be nullable: a resource created before this change, or created when telemetry is disabled, SHALL have NULL trace context and that is a valid state. -The trace context SHALL be captured from the active span context at the point of persistence (not from the raw HTTP header), so it reflects the actual span that performed the write. On an update, the stored trace context SHALL be overwritten with the new request's context, so the trace context always points to the most recent mutation. +The trace context SHALL be captured from the active span context at the point of persistence (not from the raw HTTP header), so it reflects the actual span that performed the write. On an update, the stored trace context SHALL be overwritten with the new request's context, so the trace context always points to the most recent mutation. When the updating request has no active span (telemetry disabled), the overwrite SHALL clear the stored trace context to NULL rather than preserve a stale value: the columns must always describe the most recent mutation, and that mutation had no traceable origin. **Verification:** Create a resource via the API with a valid inbound `traceparent`; confirm the database row stores the `traceparent` and `tracestate` values. Update the resource with a different request trace; confirm the stored values are overwritten. @@ -48,6 +48,14 @@ The trace context SHALL be captured from the active span context at the point of - THEN the `traceparent` and `tracestate` columns SHALL be NULL - AND the resource SHALL be created normally +#### Scenario: Update with telemetry disabled clears stale trace context + +- GIVEN a resource with a stored `traceparent` from an earlier traced mutation +- AND the OTel SDK is not initialized on the request that performs the update +- WHEN a client updates the resource +- THEN the stored `traceparent` and `tracestate` SHALL be overwritten with NULL +- AND the previous trace context SHALL not be retained, because it no longer describes the most recent mutation + ### Requirement: RTC-02 -- Trace Context on gRPC Watch Messages The resource protobuf messages carried in gRPC watch responses SHALL include the originating trace context so the control plane can read it without a separate lookup. The `traceparent` and `tracestate` fields SHALL be added to the shared `ObjectReference` message so every resource type's watch response carries them uniformly.