From ecf00ebaa3b0b5b769d4fcb8eb2c06e9c99da76e Mon Sep 17 00:00:00 2001 From: Diego Braga Date: Mon, 29 Jun 2026 22:24:09 +0200 Subject: [PATCH] =?UTF-8?q?otel:=20inject=20traceparent=20on=20cdc?= =?UTF-8?q?=E2=86=92chart-inspector=20outbound=20call?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cdc's outbound GET {server}/resources call to chart-inspector used a bare http.DefaultClient with no otelhttp instrumentation and no traceparent injection, so the chart-inspector server span was never linked to the active reconcile span by HTTP header (it relied solely on the annotation path). Wrap the client transport with otelhttp.NewTransport(http.DefaultTransport) so the request injects the W3C traceparent header (continuing the active reconcile span) and emits a client span, and thread the reconcile ctx through RBACGen.Generate → ChartInspector.Resources via http.NewRequestWithContext so the span/headers actually propagate. Also stop mutating the shared http.DefaultClient global. Default-off: the otelhttp transport is a cheap pass-through when no global tracer provider is registered; the W3C propagator is installed unconditionally by the runtime, so the link is established only when tracing is active. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01FzZJtQ4bEMHuK4CvF6eBuL --- go.mod | 2 ++ internal/chartinspector/chartinspector.go | 23 +++++++++++---- .../chartinspector/chartinspector_test.go | 16 +++++++--- internal/composition/composition.go | 6 ++-- internal/metrics/wrappers.go | 10 +++---- internal/rbacgen/rbacgen.go | 7 +++-- internal/rbacgen/rbacgen_test.go | 29 ++++++++++--------- 7 files changed, 58 insertions(+), 35 deletions(-) diff --git a/go.mod b/go.mod index 5d13160..5099854 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( github.com/krateoplatformops/plumbing v1.7.3 github.com/krateoplatformops/unstructured-runtime v1.1.0 github.com/stretchr/testify v1.11.1 + go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.61.0 go.opentelemetry.io/otel v1.44.0 go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v1.43.0 go.opentelemetry.io/otel/metric v1.44.0 @@ -46,6 +47,7 @@ require ( github.com/evanphx/json-patch/v5 v5.9.11 // indirect github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f // indirect github.com/fatih/color v1.18.0 // indirect + github.com/felixge/httpsnoop v1.0.4 // indirect github.com/fsnotify/fsnotify v1.9.0 // indirect github.com/fxamacker/cbor/v2 v2.9.0 // indirect github.com/go-errors/errors v1.5.1 // indirect diff --git a/internal/chartinspector/chartinspector.go b/internal/chartinspector/chartinspector.go index 48cd942..8b9a23c 100644 --- a/internal/chartinspector/chartinspector.go +++ b/internal/chartinspector/chartinspector.go @@ -1,12 +1,15 @@ package chartinspector import ( + "context" "encoding/json" "fmt" "io" "net/http" "net/url" "time" + + "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" ) type Resource struct { @@ -31,7 +34,7 @@ type Parameters struct { } type ChartInspectorInterface interface { - Resources(Parameters) ([]Resource, error) + Resources(ctx context.Context, params Parameters) ([]Resource, error) } type ChartInspector struct { @@ -42,8 +45,16 @@ type ChartInspector struct { var _ ChartInspectorInterface = &ChartInspector{} func NewChartInspector(server string) ChartInspector { - httpcli := http.DefaultClient - httpcli.Timeout = 60 * time.Second + // Wrap the default transport with otelhttp so the outbound request to + // chart-inspector injects the W3C traceparent header (continuing the active + // reconcile span) and emits a client span. When no global tracer provider / + // propagator is registered this transport is a cheap pass-through, so the + // off-path stays byte-identical; the unstructured-runtime installs the W3C + // propagator unconditionally, so the active reconcile span always propagates. + httpcli := &http.Client{ + Timeout: 60 * time.Second, + Transport: otelhttp.NewTransport(http.DefaultTransport), + } return ChartInspector{server: server, httpClient: httpcli} } @@ -78,7 +89,7 @@ func (c *ChartInspector) Validate(params Parameters) error { return nil } -func (c *ChartInspector) Resources(params Parameters) ([]Resource, error) { +func (c *ChartInspector) Resources(ctx context.Context, params Parameters) ([]Resource, error) { if err := c.Validate(params); err != nil { return nil, fmt.Errorf("validating parameters: %w", err) } @@ -86,7 +97,9 @@ func (c *ChartInspector) Resources(params Parameters) ([]Resource, error) { if err != nil { return nil, fmt.Errorf("joining server url: %w", err) } - req, err := http.NewRequest(http.MethodGet, u, nil) + // Use the reconcile ctx so the otelhttp transport injects the traceparent + // header from the active span, linking the chart-inspector server span. + req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, nil) if err != nil { return nil, fmt.Errorf("creating request: %w", err) } diff --git a/internal/chartinspector/chartinspector_test.go b/internal/chartinspector/chartinspector_test.go index 6dc4bea..3a904b7 100644 --- a/internal/chartinspector/chartinspector_test.go +++ b/internal/chartinspector/chartinspector_test.go @@ -1,10 +1,13 @@ package chartinspector import ( + "context" "net/http" "net/http/httptest" "net/url" "testing" + + "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" ) func TestNewChartInspector(t *testing.T) { @@ -14,8 +17,13 @@ func TestNewChartInspector(t *testing.T) { if inspector.server != server { t.Errorf("expected server %s, got %s", server, inspector.server) } - if inspector.httpClient != http.DefaultClient { - t.Error("expected default http client") + if inspector.httpClient == nil { + t.Error("expected non-nil http client") + } + // The client must carry an otelhttp transport so the outbound request to + // chart-inspector injects the W3C traceparent header from the active span. + if _, ok := inspector.httpClient.Transport.(*otelhttp.Transport); !ok { + t.Errorf("expected otelhttp transport, got %T", inspector.httpClient.Transport) } } @@ -206,7 +214,7 @@ func TestChartInspector_Resources(t *testing.T) { defer server.Close() inspector := NewChartInspector(server.URL) - resources, err := inspector.Resources(tt.params) + resources, err := inspector.Resources(context.Background(), tt.params) if tt.wantErr { if err == nil { @@ -297,7 +305,7 @@ func TestChartInspector_ResourcesQueryParameters(t *testing.T) { CompositionGroup: "custom.group", } - _, err := inspector.Resources(params) + _, err := inspector.Resources(context.Background(), params) if err != nil { t.Errorf("unexpected error: %v", err) } diff --git a/internal/composition/composition.go b/internal/composition/composition.go index 20270c7..abc4032 100644 --- a/internal/composition/composition.go +++ b/internal/composition/composition.go @@ -243,7 +243,7 @@ func (h *handler) Observe(ctx context.Context, mg *unstructured.Unstructured) (c // Get Resources and generate RBAC generated, err := rbgen. WithBaseName(releaseName). - Generate(rbacgen.Parameters{ + Generate(ctx, rbacgen.Parameters{ CompositionName: mg.GetName(), CompositionNamespace: mg.GetNamespace(), CompositionGVR: compositionGVR, @@ -451,7 +451,7 @@ func (h *handler) Create(ctx context.Context, mg *unstructured.Unstructured) err // Get Resources and generate RBAC generated, err := rbgen. WithBaseName(releaseName). - Generate(rbacgen.Parameters{ + Generate(ctx, rbacgen.Parameters{ CompositionName: mg.GetName(), CompositionNamespace: mg.GetNamespace(), CompositionGVR: compositionGVR, @@ -780,7 +780,7 @@ func (h *handler) Delete(ctx context.Context, mg *unstructured.Unstructured) err // Get Resources and generate RBAC generated, err := rbgen. WithBaseName(compositionMeta.GetReleaseName(mg)). - Generate(rbacgen.Parameters{ + Generate(ctx, rbacgen.Parameters{ CompositionName: mg.GetName(), CompositionNamespace: mg.GetNamespace(), CompositionGVR: compositionGVR, diff --git a/internal/metrics/wrappers.go b/internal/metrics/wrappers.go index 0b1a545..5b0d053 100644 --- a/internal/metrics/wrappers.go +++ b/internal/metrics/wrappers.go @@ -22,11 +22,10 @@ type metricChartInspector struct { wrapped chartinspector.ChartInspectorInterface } -func (m *metricChartInspector) Resources(params chartinspector.Parameters) ([]chartinspector.Resource, error) { - ctx := context.Background() // Use background context for metrics +func (m *metricChartInspector) Resources(ctx context.Context, params chartinspector.Parameters) ([]chartinspector.Resource, error) { timer := NewTimer() - resources, err := m.wrapped.Resources(params) + resources, err := m.wrapped.Resources(ctx, params) metrics := GetInstance() if metrics != nil { @@ -54,11 +53,10 @@ func (m *metricRBACGen) WithBaseName(name string) rbacgen.RBACGenInterface { return m } -func (m *metricRBACGen) Generate(params rbacgen.Parameters) (*rbac.RBAC, error) { - ctx := context.Background() // Use background context for metrics +func (m *metricRBACGen) Generate(ctx context.Context, params rbacgen.Parameters) (*rbac.RBAC, error) { timer := NewTimer() - result, err := m.wrapped.Generate(params) + result, err := m.wrapped.Generate(ctx, params) metrics := GetInstance() if metrics != nil { diff --git a/internal/rbacgen/rbacgen.go b/internal/rbacgen/rbacgen.go index 315974d..844394c 100644 --- a/internal/rbacgen/rbacgen.go +++ b/internal/rbacgen/rbacgen.go @@ -1,6 +1,7 @@ package rbacgen import ( + "context" "fmt" corev1 "k8s.io/api/core/v1" @@ -14,7 +15,7 @@ import ( ) type RBACGenInterface interface { - Generate(Parameters) (*rbac.RBAC, error) + Generate(ctx context.Context, params Parameters) (*rbac.RBAC, error) WithBaseName(string) RBACGenInterface } @@ -49,8 +50,8 @@ func (r *RBACGen) WithBaseName(baseName string) RBACGenInterface { return r } -func (r *RBACGen) Generate(params Parameters) (*rbac.RBAC, error) { - resources, err := r.chartInspector.Resources(chartinspector.Parameters{ +func (r *RBACGen) Generate(ctx context.Context, params Parameters) (*rbac.RBAC, error) { + resources, err := r.chartInspector.Resources(ctx, chartinspector.Parameters{ CompositionName: params.CompositionName, CompositionNamespace: params.CompositionNamespace, CompositionGroup: params.CompositionGVR.Group, diff --git a/internal/rbacgen/rbacgen_test.go b/internal/rbacgen/rbacgen_test.go index 411c6e0..544cc50 100644 --- a/internal/rbacgen/rbacgen_test.go +++ b/internal/rbacgen/rbacgen_test.go @@ -1,6 +1,7 @@ package rbacgen import ( + "context" "errors" "testing" @@ -14,8 +15,8 @@ type MockChartInspector struct { mock.Mock } -func (m *MockChartInspector) Resources(params chartinspector.Parameters) ([]chartinspector.Resource, error) { - args := m.Called(params) +func (m *MockChartInspector) Resources(ctx context.Context, params chartinspector.Parameters) ([]chartinspector.Resource, error) { + args := m.Called(ctx, params) if args.Get(0) == nil { return nil, args.Error(1) } @@ -80,9 +81,9 @@ func TestRBACGen_Generate(t *testing.T) { CompositionDefinitionResource: params.CompositionDefintionGVR.Resource, } - mockInspector.On("Resources", expectedParams).Return(mockResources, nil) + mockInspector.On("Resources", mock.Anything, expectedParams).Return(mockResources, nil) - policy, err := rbacGen.Generate(params) + policy, err := rbacGen.Generate(context.Background(), params) assert.NoError(t, err) assert.NotNil(t, policy) @@ -124,9 +125,9 @@ func TestRBACGen_Generate(t *testing.T) { CompositionResource: params.CompositionGVR.Resource, } - mockInspector.On("Resources", expectedParams).Return(mockResources, nil) + mockInspector.On("Resources", mock.Anything, expectedParams).Return(mockResources, nil) - policy, err := rbacGen.Generate(params) + policy, err := rbacGen.Generate(context.Background(), params) assert.NoError(t, err) assert.NotNil(t, policy) @@ -157,9 +158,9 @@ func TestRBACGen_Generate(t *testing.T) { CompositionNamespace: params.CompositionNamespace, } - mockInspector.On("Resources", expectedParams).Return(mockResources, nil) + mockInspector.On("Resources", mock.Anything, expectedParams).Return(mockResources, nil) - policy, err := rbacGen.Generate(params) + policy, err := rbacGen.Generate(context.Background(), params) assert.NoError(t, err) assert.NotNil(t, policy) @@ -188,9 +189,9 @@ func TestRBACGen_Generate(t *testing.T) { CompositionNamespace: params.CompositionNamespace, } - mockInspector.On("Resources", expectedParams).Return(mockResources, nil) + mockInspector.On("Resources", mock.Anything, expectedParams).Return(mockResources, nil) - policy, err := rbacGen.Generate(params) + policy, err := rbacGen.Generate(context.Background(), params) assert.NoError(t, err) assert.NotNil(t, policy) @@ -215,9 +216,9 @@ func TestRBACGen_Generate(t *testing.T) { CompositionNamespace: params.CompositionNamespace, } - mockInspector.On("Resources", expectedParams).Return(nil, errors.New("some error")) + mockInspector.On("Resources", mock.Anything, expectedParams).Return(nil, errors.New("some error")) - policy, err := rbacGen.Generate(params) + policy, err := rbacGen.Generate(context.Background(), params) assert.Error(t, err) assert.Nil(t, policy) @@ -244,9 +245,9 @@ func TestRBACGen_Generate(t *testing.T) { CompositionNamespace: params.CompositionNamespace, } - mockInspector.On("Resources", expectedParams).Return(mockResources, nil) + mockInspector.On("Resources", mock.Anything, expectedParams).Return(mockResources, nil) - policy, err := rbacGen.Generate(params) + policy, err := rbacGen.Generate(context.Background(), params) assert.NoError(t, err) assert.NotNil(t, policy)