Clean up and add more tests

Author: Starefossen

Diff for: charts/naiserator/Feature.yaml

@@ -177,6 +177,11 @@ values:
   naiserator.observability.otel.enabled:
     computed:
       template: '{{ne .Kind "onprem"}}'
+  naiserator.observability.auto-instrumentation.enabled:
+    config:
+      type: bool
+    ignoreKind:
+      - onprem
   naiserator.observability.logging.destinations:
     computed:
       # Find all logging_*_default_flow environment variables and use for configuration of available log destinations

Diff for: pkg/resourcecreator/observability/observability.go

@@ -86,7 +86,7 @@ func otelAutoInstrumentAnnotations(source Source, otel config.Otel) map[string]s
 	}
 }
 
-func labelsFromCollectorConfig(collector config.OtelCollector) map[string]string {
+func netpolLabelsFromCollectorConfig(collector config.OtelCollector) map[string]string {
 	labels := collector.Labels
 	labelMap := make(map[string]string, len(labels))
 	for _, label := range labels {
@@ -99,7 +99,29 @@ func labelsFromCollectorConfig(collector config.OtelCollector) map[string]string
 	return labelMap
 }
 
-func tracingNetpol(source Source, otel config.Otel) (*networkingv1.NetworkPolicy, error) {
+func logLabels(obs *nais_io_v1.Observability, cfg config.Logging) (map[string]string, error) {
+	if !obs.Logging.Enabled {
+		return map[string]string{logLabelDefault: "false"}, nil
+	}
+
+	labels := map[string]string{}
+
+	if len(obs.Logging.Destinations) > 0 {
+		labels[logLabelDefault] = "false"
+
+		for _, destination := range obs.Logging.Destinations {
+			if !slices.Contains(cfg.Destinations, destination.ID) {
+				return nil, fmt.Errorf("logging destination %q does not exist in cluster", destination.ID)
+			}
+
+			labels[logLabelPrefix+destination.ID] = "true"
+		}
+	}
+
+	return labels, nil
+}
+
+func otelNetpol(source Source, otel config.Otel) (*networkingv1.NetworkPolicy, error) {
 	name, err := namegen.ShortName(source.GetName()+"-"+"tracing", validation.DNS1035LabelMaxLength)
 	if err != nil {
 		return nil, err
@@ -109,7 +131,7 @@ func tracingNetpol(source Source, otel config.Otel) (*networkingv1.NetworkPolicy
 	objectMeta.Name = name
 
 	protocolTCP := corev1.ProtocolTCP
-	collectorLabels := labelsFromCollectorConfig(otel.Collector)
+	collectorLabels := netpolLabelsFromCollectorConfig(otel.Collector)
 
 	return &networkingv1.NetworkPolicy{
 		ObjectMeta: objectMeta,
@@ -166,10 +188,14 @@ func Create(source Source, ast *resource.Ast, config Config) error {
 
 	if (obs.Tracing != nil && obs.Tracing.Enabled) || (obs.AutoInstrumentation != nil && obs.AutoInstrumentation.Enabled) {
 		if !cfg.Otel.Enabled {
-			return fmt.Errorf("tracing and auto-instrumentation are not supported in this cluster")
+			return fmt.Errorf("opentelemetry is not supported for this cluster")
 		}
 
-		netpol, err := tracingNetpol(source, cfg.Otel)
+		if !cfg.Otel.AutoInstrumentation.Enabled && obs.AutoInstrumentation != nil && obs.AutoInstrumentation.Enabled {
+			return fmt.Errorf("auto-instrumentation is not supported for this cluster")
+		}
+
+		netpol, err := otelNetpol(source, cfg.Otel)
 		if err != nil {
 			return err
 		}
@@ -185,21 +211,13 @@ func Create(source Source, ast *resource.Ast, config Config) error {
 	}
 
 	if obs.Logging != nil {
-		if !obs.Logging.Enabled {
-			ast.Labels[logLabelDefault] = "false"
-			return nil
+		logLabels, err := logLabels(obs, cfg.Logging)
+		if err != nil {
+			return err
 		}
 
-		if len(obs.Logging.Destinations) > 0 {
-			ast.Labels[logLabelDefault] = "false"
-
-			for _, destination := range obs.Logging.Destinations {
-				if !slices.Contains(cfg.Logging.Destinations, destination.ID) {
-					return fmt.Errorf("logging destination %q does not exist in cluster", destination.ID)
-				}
-
-				ast.Labels[logLabelPrefix+destination.ID] = "true"
-			}
+		for k, v := range logLabels {
+			ast.Labels[k] = v
 		}
 	}
 

Diff for: pkg/resourcecreator/observability/observability_test.go

@@ -1,6 +1,7 @@
 package observability
 
 import (
+	"fmt"
 	"testing"
 
 	nais_io_v1 "github.com/nais/liberator/pkg/apis/nais.io/v1"
@@ -141,7 +142,7 @@ func TestLabelsFromCollectorConfig(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			actualLabels := labelsFromCollectorConfig(tc.collector)
+			actualLabels := netpolLabelsFromCollectorConfig(tc.collector)
 			assert.Equal(t, tc.expectedLabels, actualLabels)
 		})
 	}
@@ -225,7 +226,116 @@ func TestTracingNetpol(t *testing.T) {
 		},
 	}
 
-	actualNetworkPolicy, err := tracingNetpol(&app, otel)
+	actualNetworkPolicy, err := otelNetpol(&app, otel)
 	assert.NoError(t, err)
 	assert.Equal(t, expectedNetworkPolicy, actualNetworkPolicy)
 }
+func TestLogLabels(t *testing.T) {
+	tests := []struct {
+		name           string
+		obs            *nais_io_v1.Observability
+		cfg            config.Logging
+		expectedLabels map[string]string
+		expectedError  error
+	}{
+		{
+			name: "Enabled with multiple destinations",
+			obs: &nais_io_v1.Observability{
+				Logging: &nais_io_v1.Logging{
+					Enabled: true,
+					Destinations: []nais_io_v1.LogDestination{
+						{
+							ID: "destination1",
+						},
+						{
+							ID: "destination2",
+						},
+					},
+				},
+			},
+			cfg: config.Logging{
+				Destinations: []string{"destination1", "destination2"},
+			},
+			expectedLabels: map[string]string{
+				"logs.nais.io/flow-default":      "false",
+				"logs.nais.io/flow-destination1": "true",
+				"logs.nais.io/flow-destination2": "true",
+			},
+			expectedError: nil,
+		},
+		{
+			name: "Disabled",
+			obs: &nais_io_v1.Observability{
+				Logging: &nais_io_v1.Logging{
+					Enabled: false,
+				},
+			},
+			cfg: config.Logging{
+				Destinations: []string{"destination1", "destination2"},
+			},
+			expectedLabels: map[string]string{
+				"logs.nais.io/flow-default": "false",
+			},
+			expectedError: nil,
+		},
+		{
+			name: "Invalid destination",
+			obs: &nais_io_v1.Observability{
+				Logging: &nais_io_v1.Logging{
+					Enabled: true,
+					Destinations: []nais_io_v1.LogDestination{
+						{
+							ID: "destination1",
+						},
+					},
+				},
+			},
+			cfg: config.Logging{
+				Destinations: []string{"destination2"},
+			},
+			expectedLabels: nil,
+			expectedError:  fmt.Errorf("logging destination %q does not exist in cluster", "destination1"),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			actualLabels, err := logLabels(tt.obs, tt.cfg)
+
+			assert.Equal(t, tt.expectedError, err)
+			assert.Equal(t, tt.expectedLabels, actualLabels)
+		})
+	}
+}
+
+func TestOtelAutoInstrumentAnnotations(t *testing.T) {
+	app := nais_io_v1alpha1.Application{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "my-app",
+			Namespace: "my-namespace",
+		},
+		Spec: nais_io_v1alpha1.ApplicationSpec{
+			Observability: &nais_io_v1.Observability{
+				AutoInstrumentation: &nais_io_v1.AutoInstrumentation{
+					Enabled: true,
+					Runtime: "java",
+				},
+			},
+		},
+	}
+
+	otel := config.Otel{
+		AutoInstrumentation: config.AutoInstrumentation{
+			AppConfig: "system-namespace/my-config",
+		},
+	}
+
+	expectedAnnotations := map[string]string{
+		"instrumentation.opentelemetry.io/inject-java":     "system-namespace/my-config",
+		"instrumentation.opentelemetry.io/container-names": "my-app",
+	}
+
+	actualAnnotations := otelAutoInstrumentAnnotations(&app, otel)
+
+	assert.Equal(t, expectedAnnotations, actualAnnotations)
+}

Diff for: pkg/resourcecreator/testdata/observability_tracing_enabled_error.yaml

@@ -21,4 +21,4 @@ input:
       tracing:
         enabled: true
 
-error: "tracing and auto-instrumentation are not supported in this cluster"
+error: "opentelemetry is not supported for this cluster"