diff --git a/e2e/testcases/override_log_level_test.go b/e2e/testcases/override_log_level_test.go index 6d5bbfaa3b..01c4d86a76 100644 --- a/e2e/testcases/override_log_level_test.go +++ b/e2e/testcases/override_log_level_test.go @@ -93,14 +93,14 @@ func TestOverrideRootSyncLogLevel(t *testing.T) { } // apply override to all containers and validate - nt.MustMergePatch(rootSyncV1, `{"spec": {"override": {"logLevels": [{"containerName": "reconciler", "logLevel": 5}, {"containerName": "git-sync", "logLevel": 7}, {"containerName": "otel-agent", "logLevel": -1}, {"containerName": "hydration-controller", "logLevel": 9}]}}}`) + nt.MustMergePatch(rootSyncV1, `{"spec": {"override": {"logLevels": [{"containerName": "reconciler", "logLevel": 5}, {"containerName": "git-sync", "logLevel": 7}, {"containerName": "otel-agent", "logLevel": 0}, {"containerName": "hydration-controller", "logLevel": 9}]}}}`) err = nt.Watcher.WatchObject(kinds.Deployment(), rootReconcilerName.Name, rootReconcilerName.Namespace, []testpredicates.Predicate{ testpredicates.DeploymentContainerArgsContains(reconcilermanager.Reconciler, "-v=5"), testpredicates.DeploymentContainerArgsContains(reconcilermanager.GitSync, "-v=7"), - testpredicates.DeploymentContainerArgsContains(metrics.OtelAgentName, "--set=service.telemetry.logs.level=debug"), + testpredicates.DeploymentContainerArgsContains(metrics.OtelAgentName, "--set=service.telemetry.logs.level=fatal"), testpredicates.DeploymentContainerArgsContains(reconcilermanager.HydrationController, "-v=9"), }, ) @@ -129,7 +129,7 @@ func TestOverrideRootSyncLogLevel(t *testing.T) { // try invalid log level value maxError := "logLevel in body should be less than or equal to 10" - minError := "logLevel in body should be greater than or equal to -1" + minError := "logLevel in body should be greater than or equal to 0" err = nt.KubeClient.MergePatch(rootSyncV1, `{"spec": {"override": {"logLevels": [{"containerName": "reconciler", "logLevel": 13}]}}}`) if !strings.Contains(err.Error(), maxError) { @@ -216,7 +216,7 @@ func TestOverrideRepoSyncLogLevel(t *testing.T) { }, { ContainerName: "otel-agent", - LogLevel: 2, + LogLevel: 8, }, { ContainerName: "hydration-controller", @@ -235,7 +235,7 @@ func TestOverrideRepoSyncLogLevel(t *testing.T) { testpredicates.DeploymentContainerArgsContains(reconcilermanager.Reconciler, "-v=7"), testpredicates.DeploymentContainerArgsContains(reconcilermanager.GitSync, "-v=9"), testpredicates.DeploymentContainerArgsContains(reconcilermanager.HydrationController, "-v=5"), - testpredicates.DeploymentContainerArgsContains(metrics.OtelAgentName, "--set=service.telemetry.logs.level=error"), + testpredicates.DeploymentContainerArgsContains(metrics.OtelAgentName, "--set=service.telemetry.logs.level=debug"), }, ) if err != nil { diff --git a/manifests/reposync-crd.yaml b/manifests/reposync-crd.yaml index 6513e019da..07b69b15d6 100644 --- a/manifests/reposync-crd.yaml +++ b/manifests/reposync-crd.yaml @@ -354,12 +354,14 @@ spec: pattern: ^(reconciler|git-sync|hydration-controller|oci-sync|helm-sync|gcenode-askpass-sidecar|otel-agent)$ type: string logLevel: - description: logLevel specifies the log level override value - for the container. The default value for git-sync container - is 5, while all other containers will default to 0. Allowed - values are from -1 to 10 + description: logLevel specifies the verbosity level of the + logging for a specific container. The "git-sync" and "otel-agent" + containers default to 5, while all other containers default + to 0. Increasing the value of logLevel increases the verbosity + of the logs. Lower severity messages are logged at higher + verbosity. Allowed values are from 0 to 10. maximum: 10 - minimum: -1 + minimum: 0 type: integer required: - containerName @@ -1403,12 +1405,14 @@ spec: pattern: ^(reconciler|git-sync|hydration-controller|oci-sync|helm-sync|gcenode-askpass-sidecar|otel-agent)$ type: string logLevel: - description: logLevel specifies the log level override value - for the container. The default value for git-sync container - is 5, while all other containers will default to 0. Allowed - values are from -1 to 10 + description: logLevel specifies the verbosity level of the + logging for a specific container. The "git-sync" and "otel-agent" + containers default to 5, while all other containers default + to 0. Increasing the value of logLevel increases the verbosity + of the logs. Lower severity messages are logged at higher + verbosity. Allowed values are from 0 to 10. maximum: 10 - minimum: -1 + minimum: 0 type: integer required: - containerName diff --git a/manifests/rootsync-crd.yaml b/manifests/rootsync-crd.yaml index 7c79ca78d8..855e9595a1 100644 --- a/manifests/rootsync-crd.yaml +++ b/manifests/rootsync-crd.yaml @@ -364,12 +364,14 @@ spec: pattern: ^(reconciler|git-sync|hydration-controller|oci-sync|helm-sync|gcenode-askpass-sidecar|otel-agent)$ type: string logLevel: - description: logLevel specifies the log level override value - for the container. The default value for git-sync container - is 5, while all other containers will default to 0. Allowed - values are from -1 to 10 + description: logLevel specifies the verbosity level of the + logging for a specific container. The "git-sync" and "otel-agent" + containers default to 5, while all other containers default + to 0. Increasing the value of logLevel increases the verbosity + of the logs. Lower severity messages are logged at higher + verbosity. Allowed values are from 0 to 10. maximum: 10 - minimum: -1 + minimum: 0 type: integer required: - containerName @@ -1468,12 +1470,14 @@ spec: pattern: ^(reconciler|git-sync|hydration-controller|oci-sync|helm-sync|gcenode-askpass-sidecar|otel-agent)$ type: string logLevel: - description: logLevel specifies the log level override value - for the container. The default value for git-sync container - is 5, while all other containers will default to 0. Allowed - values are from -1 to 10 + description: logLevel specifies the verbosity level of the + logging for a specific container. The "git-sync" and "otel-agent" + containers default to 5, while all other containers default + to 0. Increasing the value of logLevel increases the verbosity + of the logs. Lower severity messages are logged at higher + verbosity. Allowed values are from 0 to 10. maximum: 10 - minimum: -1 + minimum: 0 type: integer required: - containerName diff --git a/pkg/api/configsync/v1alpha1/resource_override.go b/pkg/api/configsync/v1alpha1/resource_override.go index d17ca9a24e..d5b6cec6a3 100644 --- a/pkg/api/configsync/v1alpha1/resource_override.go +++ b/pkg/api/configsync/v1alpha1/resource_override.go @@ -160,10 +160,12 @@ type ContainerLogLevelOverride struct { // +kubebuilder:validation:Pattern=^(reconciler|git-sync|hydration-controller|oci-sync|helm-sync|gcenode-askpass-sidecar|otel-agent)$ ContainerName string `json:"containerName"` - // logLevel specifies the log level override value for the container. - // The default value for git-sync container is 5, while all other containers will default to 0. - // Allowed values are from -1 to 10 - // +kubebuilder:validation:Minimum=-1 + // logLevel specifies the verbosity level of the logging for a specific container. + // The "git-sync" and "otel-agent" containers default to 5, while all other containers default to 0. + // Increasing the value of logLevel increases the verbosity of the logs. + // Lower severity messages are logged at higher verbosity. + // Allowed values are from 0 to 10. + // +kubebuilder:validation:Minimum=0 // +kubebuilder:validation:Maximum=10 // +kubebuilder:validation:Required LogLevel int `json:"logLevel"` diff --git a/pkg/api/configsync/v1beta1/resource_override.go b/pkg/api/configsync/v1beta1/resource_override.go index c4275a1e3b..e762b87915 100644 --- a/pkg/api/configsync/v1beta1/resource_override.go +++ b/pkg/api/configsync/v1beta1/resource_override.go @@ -160,10 +160,12 @@ type ContainerLogLevelOverride struct { // +kubebuilder:validation:Pattern=^(reconciler|git-sync|hydration-controller|oci-sync|helm-sync|gcenode-askpass-sidecar|otel-agent)$ ContainerName string `json:"containerName"` - // logLevel specifies the log level override value for the container. - // The default value for git-sync container is 5, while all other containers will default to 0. - // Allowed values are from -1 to 10 - // +kubebuilder:validation:Minimum=-1 + // logLevel specifies the verbosity level of the logging for a specific container. + // The "git-sync" and "otel-agent" containers default to 5, while all other containers default to 0. + // Increasing the value of logLevel increases the verbosity of the logs. + // Lower severity messages are logged at higher verbosity. + // Allowed values are from 0 to 10. + // +kubebuilder:validation:Minimum=0 // +kubebuilder:validation:Maximum=10 // +kubebuilder:validation:Required LogLevel int `json:"logLevel"` diff --git a/pkg/reconcilermanager/controllers/reconciler_container_log_level.go b/pkg/reconcilermanager/controllers/reconciler_container_log_level.go index cbe20372f5..92257e7add 100644 --- a/pkg/reconcilermanager/controllers/reconciler_container_log_level.go +++ b/pkg/reconcilermanager/controllers/reconciler_container_log_level.go @@ -27,7 +27,7 @@ import ( // ReconcilerContainerLogLevelDefaults are the default log level to use for the // reconciler deployment containers. -// All containers default value are 0 except git-sync which default value is 5 +// All containers default value are 0 except git-sync/otel-agent which default value is 5 func ReconcilerContainerLogLevelDefaults() map[string]v1beta1.ContainerLogLevelOverride { return map[string]v1beta1.ContainerLogLevelOverride{ reconcilermanager.Reconciler: { @@ -56,7 +56,7 @@ func ReconcilerContainerLogLevelDefaults() map[string]v1beta1.ContainerLogLevelO }, metrics.OtelAgentName: { ContainerName: metrics.OtelAgentName, - LogLevel: 0, + LogLevel: 5, // otel-agent default is 5, this maps to zap level "INFO" }, } } @@ -107,7 +107,9 @@ func mutateContainerLogLevel(c *corev1.Container, override []v1beta1.ContainerLo switch c.Name { case metrics.OtelAgentName: // otel-agent surfaces the log level configuration differently. - zapLevel := zapcore.Level(logLevel.LogLevel) + // Our log levels range from 0-10, whereas zap ranges from -1 (debug) to 5 (fatal). + // We reverse the order for consistent behavior with our other log levels. + zapLevel := zapcore.Level(max(int(zapcore.FatalLevel)-logLevel.LogLevel, int(zapcore.DebugLevel))) // unmarshal and marshal to validate that the zap level is valid if _, err := zapcore.ParseLevel(zapLevel.String()); err != nil { return err diff --git a/pkg/reconcilermanager/controllers/reconciler_container_log_level_test.go b/pkg/reconcilermanager/controllers/reconciler_container_log_level_test.go new file mode 100644 index 0000000000..7114686dd2 --- /dev/null +++ b/pkg/reconcilermanager/controllers/reconciler_container_log_level_test.go @@ -0,0 +1,94 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package controllers + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + "kpt.dev/configsync/pkg/api/configsync/v1beta1" + "kpt.dev/configsync/pkg/metrics" +) + +func TestMutateContainerLogLevelOtelAgent(t *testing.T) { + testCases := map[string]struct { + logLevel int + expectedLevel string + }{ + "otel-agent with 0 log level": { + logLevel: 0, + expectedLevel: "fatal", + }, + "otel-agent with 1 log level": { + logLevel: 1, + expectedLevel: "panic", + }, + "otel-agent with 2 log level": { + logLevel: 2, + expectedLevel: "dpanic", + }, + "otel-agent with 3 log level": { + logLevel: 3, + expectedLevel: "error", + }, + "otel-agent with 4 log level": { + logLevel: 4, + expectedLevel: "warn", + }, + "otel-agent with 5 log level": { + logLevel: 5, + expectedLevel: "info", + }, + "otel-agent with 6 log level": { + logLevel: 6, + expectedLevel: "debug", + }, + "otel-agent with 7 log level": { + logLevel: 7, + expectedLevel: "debug", + }, + "otel-agent with 8 log level": { + logLevel: 8, + expectedLevel: "debug", + }, + "otel-agent with 9 log level": { + logLevel: 9, + expectedLevel: "debug", + }, + "otel-agent with 10 log level": { + logLevel: 10, + expectedLevel: "debug", + }, + } + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + container := &corev1.Container{ + Name: metrics.OtelAgentName, + } + override := []v1beta1.ContainerLogLevelOverride{ + { + ContainerName: metrics.OtelAgentName, + LogLevel: tc.logLevel, + }, + } + err := mutateContainerLogLevel(container, override) + assert.NoError(t, err) + expectedArgs := []string{fmt.Sprintf("--set=service.telemetry.logs.level=%s", tc.expectedLevel)} + assert.Equal(t, expectedArgs, container.Args) + }) + } +}