feat(telemetry): extend TelemetryPolicy with logging stanza

cmd/extensions/telemetry-policy/api/v1alpha1/telemetrypolicy_types.go

@@ -29,8 +29,8 @@ import (
 //+kubebuilder:object:root=true
 //+kubebuilder:subresource:status
 
-// TelemetryPolicy enables custom metric labelling for Kuadrant data plane resources
-// through the use of dynamically evaluated CEL expressions.
+// TelemetryPolicy enables custom metric labelling and log field enrichment for
+// Kuadrant data plane resources through the use of dynamically evaluated CEL expressions.
 type TelemetryPolicy struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`
@@ -46,7 +46,12 @@ type TelemetryPolicySpec struct {
 	TargetRef gatewayapiv1alpha2.LocalPolicyTargetReferenceWithSectionName `json:"targetRef"`
 
 	// Metrics holds the telemetry metrics configuration
-	Metrics MetricsSpec `json:"metrics"`
+	// +optional
+	Metrics *MetricsSpec `json:"metrics,omitempty"`
+
+	// Logging holds the telemetry logging configuration
+	// +optional
+	Logging *LoggingSpec `json:"logging,omitempty"`
 }
 
 func (p *TelemetryPolicy) GetName() string {
@@ -77,6 +82,22 @@ type MetricsConfig struct {
 	Labels map[string]string `json:"labels"`
 }
 
+// LoggingSpec defines the configuration for telemetry logging
+type LoggingSpec struct {
+	// Default logging configuration that applies to all requests
+	Default LoggingConfig `json:"default"`
+}
+
+// LoggingConfig defines reusable logging configuration
+type LoggingConfig struct {
+	// Fields to add to auth decision log records, where keys are field names and values are
+	// CEL expressions referencing well-known attributes (e.g. auth.identity.sub).
+	// Only fields whose CEL expressions resolve successfully will be included.
+	// +kubebuilder:validation:MinProperties=1
+	// +kubebuilder:validation:XValidation:rule="self.all(k, !k.contains('.'))",message="field names must not contain periods"
+	Fields map[string]string `json:"fields"`
+}
+
 // TelemetryPolicyStatus defines the observed state of TelemetryPolicy
 type TelemetryPolicyStatus struct {
 	// ObservedGeneration reflects the generation of the most recently observed spec.

cmd/extensions/telemetry-policy/config/crd/bases/extensions.kuadrant.io_telemetrypolicies.yaml

@@ -18,8 +18,8 @@ spec:
     schema:
       openAPIV3Schema:
         description: |-
-          TelemetryPolicy enables custom metric labelling for Kuadrant data plane resources
-          through the use of dynamically evaluated CEL expressions.
+          TelemetryPolicy enables custom metric labelling and log field enrichment for
+          Kuadrant data plane resources through the use of dynamically evaluated CEL expressions.
         properties:
           apiVersion:
             description: |-
@@ -40,6 +40,31 @@ spec:
             type: object
           spec:
             properties:
+              logging:
+                description: Logging holds the telemetry logging configuration
+                properties:
+                  default:
+                    description: Default logging configuration that applies to all
+                      requests
+                    properties:
+                      fields:
+                        additionalProperties:
+                          type: string
+                        description: |-
+                          Fields to add to auth decision log records, where keys are field names and values are
+                          CEL expressions referencing well-known attributes (e.g. auth.identity.sub).
+                          Only fields whose CEL expressions resolve successfully will be included.
+                        minProperties: 1
+                        type: object
+                        x-kubernetes-validations:
+                        - message: field names must not contain periods
+                          rule: self.all(k, !k.contains('.'))
+                    required:
+                    - fields
+                    type: object
+                required:
+                - default
+                type: object
               metrics:
                 description: Metrics holds the telemetry metrics configuration
                 properties:
@@ -108,7 +133,6 @@ spec:
                 - message: Invalid targetRef.kind. The only supported value is 'Gateway'
                   rule: self.kind == 'Gateway'
             required:
-            - metrics
             - targetRef
             type: object
           status:

cmd/extensions/telemetry-policy/internal/controller/telemetrypolicy_reconciler.go

@@ -68,10 +68,21 @@ func (r *TelemetryPolicyReconciler) Reconcile(ctx context.Context, request recon
 }
 
 func (r *TelemetryPolicyReconciler) reconcileSpec(ctx context.Context, pol *v1alpha1.TelemetryPolicy, kuadrantCtx types.KuadrantCtx) (*v1alpha1.TelemetryPolicyStatus, error) {
-	for binding, expression := range pol.Spec.Metrics.Default.Labels {
-		if err := kuadrantCtx.AddDataTo(ctx, pol, types.DomainRequest, types.KuadrantMetricBinding(binding), expression); err != nil {
-			r.Logger.Error(err, "failed to add data to request domain")
-			return calculateErrorStatus(pol, err), err
+	if pol.Spec.Metrics != nil {
+		for binding, expression := range pol.Spec.Metrics.Default.Labels {
+			if err := kuadrantCtx.AddDataTo(ctx, pol, types.DomainRequest, types.KuadrantMetricBinding(binding), expression); err != nil {
+				r.Logger.Error(err, "failed to add data to request domain")
+				return calculateErrorStatus(pol, err), err
+			}
+		}
+	}
+
+	if pol.Spec.Logging != nil {
+		for binding, expression := range pol.Spec.Logging.Default.Fields {
+			if err := kuadrantCtx.AddDataTo(ctx, pol, types.DomainRequest, types.KuadrantLoggingBinding(binding), expression); err != nil {
+				r.Logger.Error(err, "failed to add data to request domain")
+				return calculateErrorStatus(pol, err), err
+			}
 		}
 	}
 

cmd/extensions/telemetry-policy/internal/controller/telemetrypolicy_reconciler_test.go

@@ -0,0 +1,199 @@
+//go:build unit
+
+package controller
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/go-logr/logr"
+	celref "github.com/google/cel-go/common/types/ref"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
+
+	"github.com/kuadrant/kuadrant-operator/cmd/extensions/telemetry-policy/api/v1alpha1"
+	"github.com/kuadrant/kuadrant-operator/pkg/extension/types"
+)
+
+type addDataCall struct {
+	domain     types.Domain
+	binding    string
+	expression string
+}
+
+type mockKuadrantCtx struct {
+	calls   []addDataCall
+	failOn  string // return error when binding matches this value
+	failErr error
+}
+
+func (m *mockKuadrantCtx) AddDataTo(_ context.Context, _ types.Policy, domain types.Domain, binding, expression string) error {
+	m.calls = append(m.calls, addDataCall{domain: domain, binding: binding, expression: expression})
+	if m.failOn != "" && binding == m.failOn {
+		return m.failErr
+	}
+	return nil
+}
+
+func (m *mockKuadrantCtx) Resolve(context.Context, types.Policy, string, bool) (celref.Val, error) {
+	return nil, nil
+}
+func (m *mockKuadrantCtx) ResolvePolicy(context.Context, types.Policy, string, bool) (types.Policy, error) {
+	return nil, nil
+}
+func (m *mockKuadrantCtx) ReconcileObject(context.Context, client.Object, client.Object, types.MutateFn) (client.Object, error) {
+	return nil, nil
+}
+func (m *mockKuadrantCtx) RegisterActionMethod(_ context.Context, _ types.Policy, _ types.ActionMethodConfig) error {
+	return nil
+}
+func (m *mockKuadrantCtx) NewPipeline(types.Policy) types.Pipeline { return nil }
+
+func newTestPolicy(metrics map[string]string, loggingFields map[string]string) *v1alpha1.TelemetryPolicy {
+	pol := &v1alpha1.TelemetryPolicy{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:       "test-policy",
+			Namespace:  "default",
+			Generation: 1,
+		},
+		Spec: v1alpha1.TelemetryPolicySpec{
+			TargetRef: gatewayapiv1alpha2.LocalPolicyTargetReferenceWithSectionName{
+				LocalPolicyTargetReference: gatewayapiv1alpha2.LocalPolicyTargetReference{
+					Group: "gateway.networking.k8s.io",
+					Kind:  "Gateway",
+					Name:  "my-gw",
+				},
+			},
+		},
+	}
+	if metrics != nil {
+		pol.Spec.Metrics = &v1alpha1.MetricsSpec{Default: v1alpha1.MetricsConfig{Labels: metrics}}
+	}
+	if loggingFields != nil {
+		pol.Spec.Logging = &v1alpha1.LoggingSpec{Default: v1alpha1.LoggingConfig{Fields: loggingFields}}
+	}
+	return pol
+}
+
+func TestReconcileSpec_LoggingFieldsOnly(t *testing.T) {
+	mock := &mockKuadrantCtx{}
+	r := &TelemetryPolicyReconciler{
+		ExtensionBase: types.ExtensionBase{Logger: logr.Discard()},
+	}
+	pol := newTestPolicy(nil, map[string]string{
+		"client_identity": "auth.identity.sub",
+		"request_path":    "request.path",
+	})
+
+	status, err := r.reconcileSpec(context.Background(), pol, mock)
+	if err != nil {
+		t.Fatalf("reconcileSpec returned error: %v", err)
+	}
+	if status == nil {
+		t.Fatal("reconcileSpec returned nil status")
+	}
+
+	if len(mock.calls) != 2 {
+		t.Fatalf("expected 2 AddDataTo calls, got %d", len(mock.calls))
+	}
+
+	callMap := make(map[string]addDataCall, len(mock.calls))
+	for _, c := range mock.calls {
+		callMap[c.binding] = c
+	}
+
+	if c, ok := callMap["logging.fields.client_identity"]; !ok {
+		t.Error("missing AddDataTo call for logging.fields.client_identity")
+	} else {
+		if c.expression != "auth.identity.sub" {
+			t.Errorf("client_identity expression = %q, want %q", c.expression, "auth.identity.sub")
+		}
+		if c.domain != types.DomainRequest {
+			t.Errorf("client_identity domain = %v, want DomainRequest", c.domain)
+		}
+	}
+
+	if c, ok := callMap["logging.fields.request_path"]; !ok {
+		t.Error("missing AddDataTo call for logging.fields.request_path")
+	} else {
+		if c.expression != "request.path" {
+			t.Errorf("request_path expression = %q, want %q", c.expression, "request.path")
+		}
+	}
+}
+
+func TestReconcileSpec_MetricsAndLogging(t *testing.T) {
+	mock := &mockKuadrantCtx{}
+	r := &TelemetryPolicyReconciler{
+		ExtensionBase: types.ExtensionBase{Logger: logr.Discard()},
+	}
+	pol := newTestPolicy(
+		map[string]string{"model": "responseBodyJSON('/model')"},
+		map[string]string{"client_identity": "auth.identity.sub"},
+	)
+
+	_, err := r.reconcileSpec(context.Background(), pol, mock)
+	if err != nil {
+		t.Fatalf("reconcileSpec returned error: %v", err)
+	}
+
+	if len(mock.calls) != 2 {
+		t.Fatalf("expected 2 AddDataTo calls, got %d", len(mock.calls))
+	}
+
+	callMap := make(map[string]addDataCall, len(mock.calls))
+	for _, c := range mock.calls {
+		callMap[c.binding] = c
+	}
+
+	if _, ok := callMap["metrics.labels.model"]; !ok {
+		t.Error("missing AddDataTo call for metrics.labels.model")
+	}
+	if _, ok := callMap["logging.fields.client_identity"]; !ok {
+		t.Error("missing AddDataTo call for logging.fields.client_identity")
+	}
+}
+
+func TestReconcileSpec_LoggingFieldError(t *testing.T) {
+	expectedErr := fmt.Errorf("binding failed")
+	mock := &mockKuadrantCtx{
+		failOn:  "logging.fields.bad_field",
+		failErr: expectedErr,
+	}
+	r := &TelemetryPolicyReconciler{
+		ExtensionBase: types.ExtensionBase{Logger: logr.Discard()},
+	}
+	pol := newTestPolicy(nil, map[string]string{
+		"bad_field": "invalid.expression",
+	})
+
+	status, err := r.reconcileSpec(context.Background(), pol, mock)
+	if err != expectedErr {
+		t.Fatalf("expected error %v, got %v", expectedErr, err)
+	}
+	if status == nil {
+		t.Fatal("expected error status, got nil")
+	}
+}
+
+func TestReconcileSpec_EmptySpec(t *testing.T) {
+	mock := &mockKuadrantCtx{}
+	r := &TelemetryPolicyReconciler{
+		ExtensionBase: types.ExtensionBase{Logger: logr.Discard()},
+	}
+	pol := newTestPolicy(nil, nil)
+
+	status, err := r.reconcileSpec(context.Background(), pol, mock)
+	if err != nil {
+		t.Fatalf("reconcileSpec returned error: %v", err)
+	}
+	if status == nil {
+		t.Fatal("reconcileSpec returned nil status")
+	}
+	if len(mock.calls) != 0 {
+		t.Errorf("expected 0 AddDataTo calls for empty spec, got %d", len(mock.calls))
+	}
+}