From dab898f6bb45d654fb138eb6c4860e15ee5eb59b Mon Sep 17 00:00:00 2001 From: jiangjiang <86391540+googs1025@users.noreply.github.com> Date: Sun, 24 Mar 2024 03:20:49 +0800 Subject: [PATCH 1/5] test: use utilruntime package wrap AddToScheme method (#2782) --- controllers/suite_test.go | 27 +++++-------------- .../podmutation/webhookhandler_suite_test.go | 11 +++----- pkg/collector/upgrade/suite_test.go | 12 +++------ .../instrumentation_suite_test.go | 6 ++--- .../upgrade/upgrade_suite_test.go | 6 ++--- 5 files changed, 17 insertions(+), 45 deletions(-) diff --git a/controllers/suite_test.go b/controllers/suite_test.go index f15fe3e975..ff17ae73e2 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -36,6 +36,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/uuid" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/kubernetes" @@ -109,26 +110,12 @@ func TestMain(m *testing.M) { fmt.Printf("failed to start testEnv: %v", err) os.Exit(1) } - if err = monitoringv1.AddToScheme(testScheme); err != nil { - fmt.Printf("failed to register scheme: %v", err) - os.Exit(1) - } - if err = networkingv1.AddToScheme(testScheme); err != nil { - fmt.Printf("failed to register scheme: %v", err) - os.Exit(1) - } - if err = routev1.AddToScheme(testScheme); err != nil { - fmt.Printf("failed to register scheme: %v", err) - os.Exit(1) - } - if err = v1alpha1.AddToScheme(testScheme); err != nil { - fmt.Printf("failed to register scheme: %v", err) - os.Exit(1) - } - if err = v1beta1.AddToScheme(testScheme); err != nil { - fmt.Printf("failed to register scheme: %v", err) - os.Exit(1) - } + + utilruntime.Must(monitoringv1.AddToScheme(testScheme)) + utilruntime.Must(networkingv1.AddToScheme(testScheme)) + utilruntime.Must(routev1.AddToScheme(testScheme)) + utilruntime.Must(v1alpha1.AddToScheme(testScheme)) + utilruntime.Must(v1beta1.AddToScheme(testScheme)) testEnv = &envtest.Environment{ CRDDirectoryPaths: []string{filepath.Join("..", "config", "crd", "bases")}, diff --git a/internal/webhook/podmutation/webhookhandler_suite_test.go b/internal/webhook/podmutation/webhookhandler_suite_test.go index 335b571381..84f5d56757 100644 --- a/internal/webhook/podmutation/webhookhandler_suite_test.go +++ b/internal/webhook/podmutation/webhookhandler_suite_test.go @@ -26,6 +26,7 @@ import ( "time" "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" @@ -57,14 +58,8 @@ var ( func TestMain(m *testing.M) { ctx, cancel = context.WithCancel(context.TODO()) defer cancel() - if err = v1alpha1.AddToScheme(testScheme); err != nil { - fmt.Printf("failed to register scheme: %v", err) - os.Exit(1) - } - if err = v1beta1.AddToScheme(testScheme); err != nil { - fmt.Printf("failed to register scheme: %v", err) - os.Exit(1) - } + utilruntime.Must(v1alpha1.AddToScheme(testScheme)) + utilruntime.Must(v1beta1.AddToScheme(testScheme)) testEnv = &envtest.Environment{ CRDDirectoryPaths: []string{filepath.Join("..", "..", "..", "config", "crd", "bases")}, diff --git a/pkg/collector/upgrade/suite_test.go b/pkg/collector/upgrade/suite_test.go index 84076790e6..5026914a96 100644 --- a/pkg/collector/upgrade/suite_test.go +++ b/pkg/collector/upgrade/suite_test.go @@ -26,6 +26,7 @@ import ( "time" "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" @@ -57,15 +58,8 @@ var ( func TestMain(m *testing.M) { ctx, cancel = context.WithCancel(context.TODO()) defer cancel() - - if err = v1alpha1.AddToScheme(testScheme); err != nil { - fmt.Printf("failed to register scheme: %v", err) - os.Exit(1) - } - if err = v1beta1.AddToScheme(testScheme); err != nil { - fmt.Printf("failed to register scheme: %v", err) - os.Exit(1) - } + utilruntime.Must(v1alpha1.AddToScheme(testScheme)) + utilruntime.Must(v1beta1.AddToScheme(testScheme)) testEnv = &envtest.Environment{ CRDDirectoryPaths: []string{filepath.Join("..", "..", "..", "config", "crd", "bases")}, diff --git a/pkg/instrumentation/instrumentation_suite_test.go b/pkg/instrumentation/instrumentation_suite_test.go index 4877f7355a..d65f21b0dd 100644 --- a/pkg/instrumentation/instrumentation_suite_test.go +++ b/pkg/instrumentation/instrumentation_suite_test.go @@ -20,6 +20,7 @@ import ( "path/filepath" "testing" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" @@ -47,10 +48,7 @@ func TestMain(m *testing.M) { os.Exit(1) } - if err = v1alpha1.AddToScheme(testScheme); err != nil { - fmt.Printf("failed to register scheme: %v", err) - os.Exit(1) - } + utilruntime.Must(v1alpha1.AddToScheme(testScheme)) k8sClient, err = client.New(cfg, client.Options{Scheme: testScheme}) if err != nil { diff --git a/pkg/instrumentation/upgrade/upgrade_suite_test.go b/pkg/instrumentation/upgrade/upgrade_suite_test.go index 11f62ea128..5a17d5ac98 100644 --- a/pkg/instrumentation/upgrade/upgrade_suite_test.go +++ b/pkg/instrumentation/upgrade/upgrade_suite_test.go @@ -20,6 +20,7 @@ import ( "path/filepath" "testing" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" "sigs.k8s.io/controller-runtime/pkg/client" @@ -47,10 +48,7 @@ func TestMain(m *testing.M) { os.Exit(1) } - if err = v1alpha1.AddToScheme(testScheme); err != nil { - fmt.Printf("failed to register scheme: %v", err) - os.Exit(1) - } + utilruntime.Must(v1alpha1.AddToScheme(testScheme)) k8sClient, err = client.New(cfg, client.Options{Scheme: testScheme}) if err != nil { From d1f89a2ad019ec5f8250dc24353c8847b628a6c3 Mon Sep 17 00:00:00 2001 From: Israel Blancas Date: Fri, 29 Mar 2024 12:43:02 +0100 Subject: [PATCH 2/5] Fix RBAC creation resources for k8sattributes (#2789) --- .../2788-fix-k8sattributes-rbac-creation.yaml | 16 +++ .../collector/adapters/config_to_ports.go | 2 +- .../collector/adapters/config_to_rbac.go | 2 +- .../processor/processor_k8sattributes.go | 16 +-- .../processor/processor_k8sattributes_test.go | 110 ++++++++++++++++++ 5 files changed, 136 insertions(+), 10 deletions(-) create mode 100755 .chloggen/2788-fix-k8sattributes-rbac-creation.yaml create mode 100644 internal/manifests/collector/parser/processor/processor_k8sattributes_test.go diff --git a/.chloggen/2788-fix-k8sattributes-rbac-creation.yaml b/.chloggen/2788-fix-k8sattributes-rbac-creation.yaml new file mode 100755 index 0000000000..01288c7d22 --- /dev/null +++ b/.chloggen/2788-fix-k8sattributes-rbac-creation.yaml @@ -0,0 +1,16 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: bug_fix + +# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action) +component: collector + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: "Fix the RBAC resources creation when the processor was only enabled" + +# One or more tracking issues related to the change +issues: [2788] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: diff --git a/internal/manifests/collector/adapters/config_to_ports.go b/internal/manifests/collector/adapters/config_to_ports.go index f0945389f8..5f2a88be9c 100644 --- a/internal/manifests/collector/adapters/config_to_ports.go +++ b/internal/manifests/collector/adapters/config_to_ports.go @@ -100,7 +100,7 @@ func ConfigToComponentPorts(logger logr.Logger, cType ComponentType, config map[ } if err != nil { - logger.V(2).Info("no parser found for '%s'", cmptName) + logger.V(2).Info("no parser found for", "component", cmptName) continue } diff --git a/internal/manifests/collector/adapters/config_to_rbac.go b/internal/manifests/collector/adapters/config_to_rbac.go index 340e11eb41..55fd2d9bef 100644 --- a/internal/manifests/collector/adapters/config_to_rbac.go +++ b/internal/manifests/collector/adapters/config_to_rbac.go @@ -52,7 +52,7 @@ func ConfigToRBAC(logger logr.Logger, config map[interface{}]interface{}) []rbac processorName := key.(string) processorParser, err := processor.For(logger, processorName, processorCfg) if err != nil { - logger.V(2).Info("no parser found for '%s'", processorName) + logger.V(2).Info("no parser found for", "processor", processorName) continue } diff --git a/internal/manifests/collector/parser/processor/processor_k8sattributes.go b/internal/manifests/collector/parser/processor/processor_k8sattributes.go index 25b30434af..3fcbfb0911 100644 --- a/internal/manifests/collector/parser/processor/processor_k8sattributes.go +++ b/internal/manifests/collector/parser/processor/processor_k8sattributes.go @@ -52,6 +52,14 @@ func (o *K8sAttributesParser) ParserName() string { func (o *K8sAttributesParser) GetRBACRules() []rbacv1.PolicyRule { var prs []rbacv1.PolicyRule + // This one needs to be added always + policy := rbacv1.PolicyRule{ + APIGroups: []string{""}, + Resources: []string{"pods", "namespaces"}, + Verbs: []string{"get", "watch", "list"}, + } + prs = append(prs, policy) + extractCfg, ok := o.config["extract"] if !ok { return prs @@ -67,14 +75,6 @@ func (o *K8sAttributesParser) GetRBACRules() []rbacv1.PolicyRule { return prs } - // This one needs to be added always - policy := rbacv1.PolicyRule{ - APIGroups: []string{""}, - Resources: []string{"pods", "namespaces"}, - Verbs: []string{"get", "watch", "list"}, - } - prs = append(prs, policy) - for _, m := range metadata { metadataField := fmt.Sprint(m) if metadataField == "k8s.deployment.uid" || metadataField == "k8s.deployment.name" { diff --git a/internal/manifests/collector/parser/processor/processor_k8sattributes_test.go b/internal/manifests/collector/parser/processor/processor_k8sattributes_test.go new file mode 100644 index 0000000000..5b5c044bca --- /dev/null +++ b/internal/manifests/collector/parser/processor/processor_k8sattributes_test.go @@ -0,0 +1,110 @@ +// Copyright The OpenTelemetry Authors +// +// 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 processor + +import ( + "testing" + + "github.com/stretchr/testify/assert" + rbacv1 "k8s.io/api/rbac/v1" + logf "sigs.k8s.io/controller-runtime/pkg/log" +) + +var logger = logf.Log.WithName("unit-tests") + +func TestK8sAttributesRBAC(t *testing.T) { + + tests := []struct { + name string + config map[interface{}]interface{} + expectedRules []rbacv1.PolicyRule + }{ + { + name: "no extra parameters", + config: nil, + expectedRules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"pods", "namespaces"}, + Verbs: []string{"get", "watch", "list"}, + }, + }, + }, + { + name: "extract k8s.deployment.uid", + config: map[interface{}]interface{}{ + "extract": map[interface{}]interface{}{ + "metadata": []interface{}{ + "k8s.deployment.uid", + }, + }, + }, + expectedRules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"pods", "namespaces"}, + Verbs: []string{"get", "watch", "list"}, + }, + { + APIGroups: []string{"apps"}, + Resources: []string{"replicasets"}, + Verbs: []string{"get", "watch", "list"}, + }, + { + APIGroups: []string{"extensions"}, + Resources: []string{"replicasets"}, + Verbs: []string{"get", "watch", "list"}, + }, + }, + }, + { + name: "extract k8s.deployment.name", + config: map[interface{}]interface{}{ + "extract": map[interface{}]interface{}{ + "metadata": []interface{}{ + "k8s.deployment.name", + }, + }, + }, + expectedRules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"pods", "namespaces"}, + Verbs: []string{"get", "watch", "list"}, + }, + { + APIGroups: []string{"apps"}, + Resources: []string{"replicasets"}, + Verbs: []string{"get", "watch", "list"}, + }, + { + APIGroups: []string{"extensions"}, + Resources: []string{"replicasets"}, + Verbs: []string{"get", "watch", "list"}, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := NewK8sAttributesParser(logger, "test", tt.config) + rules := p.GetRBACRules() + assert.Equal(t, tt.expectedRules, rules) + }) + + } + +} From 880441cc8137621fd81dc4a5ed57741ab03a5722 Mon Sep 17 00:00:00 2001 From: OpenTelemetry Bot <107717825+opentelemetrybot@users.noreply.github.com> Date: Fri, 29 Mar 2024 12:47:55 +0100 Subject: [PATCH 3/5] Update the OpenTelemetry Java agent version to 2.2.0 (#2762) --- autoinstrumentation/java/version.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/autoinstrumentation/java/version.txt b/autoinstrumentation/java/version.txt index 7ec1d6db40..ccbccc3dc6 100644 --- a/autoinstrumentation/java/version.txt +++ b/autoinstrumentation/java/version.txt @@ -1 +1 @@ -2.1.0 +2.2.0 From 60d37e18ce8ad8b5eb0e0a0da7dbe2fe19fec534 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20=C5=9Awi=C4=85tek?= Date: Fri, 29 Mar 2024 14:00:01 +0100 Subject: [PATCH 4/5] Promote operator.collector.rewritetargetallocator feature flag to stable (#2797) --- .chloggen/promote-rewrite-featuregate.yaml | 16 +++ .../collector/config_replace_test.go | 61 +--------- .../manifests/collector/configmap_test.go | 109 ------------------ pkg/featuregate/featuregate.go | 3 +- 4 files changed, 19 insertions(+), 170 deletions(-) create mode 100755 .chloggen/promote-rewrite-featuregate.yaml diff --git a/.chloggen/promote-rewrite-featuregate.yaml b/.chloggen/promote-rewrite-featuregate.yaml new file mode 100755 index 0000000000..de8462d35e --- /dev/null +++ b/.chloggen/promote-rewrite-featuregate.yaml @@ -0,0 +1,16 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: breaking + +# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action) +component: target allocator + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Promote `operator.collector.rewritetargetallocator` feature flag to stable + +# One or more tracking issues related to the change +issues: [2796] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: diff --git a/internal/manifests/collector/config_replace_test.go b/internal/manifests/collector/config_replace_test.go index 8f31af3107..2a97f8a008 100644 --- a/internal/manifests/collector/config_replace_test.go +++ b/internal/manifests/collector/config_replace_test.go @@ -18,57 +18,17 @@ import ( "os" "testing" - "github.com/prometheus/prometheus/discovery/http" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - colfeaturegate "go.opentelemetry.io/collector/featuregate" "gopkg.in/yaml.v2" ta "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator/adapters" - "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" ) func TestPrometheusParser(t *testing.T) { param, err := newParams("test/test-img", "testdata/http_sd_config_test.yaml") assert.NoError(t, err) - t.Run("should update config with http_sd_config", func(t *testing.T) { - err := colfeaturegate.GlobalRegistry().Set(featuregate.EnableTargetAllocatorRewrite.ID(), false) - require.NoError(t, err) - t.Cleanup(func() { - _ = colfeaturegate.GlobalRegistry().Set(featuregate.EnableTargetAllocatorRewrite.ID(), true) - }) - actualConfig, err := ReplaceConfig(param.OtelCol) - assert.NoError(t, err) - - // prepare - var cfg Config - promCfgMap, err := ta.ConfigToPromConfig(actualConfig) - assert.NoError(t, err) - - promCfg, err := yaml.Marshal(promCfgMap) - assert.NoError(t, err) - - err = yaml.UnmarshalStrict(promCfg, &cfg) - assert.NoError(t, err) - - // test - expectedMap := map[string]bool{ - "prometheus": false, - "service-x": false, - } - for _, scrapeConfig := range cfg.PromConfig.ScrapeConfigs { - assert.Len(t, scrapeConfig.ServiceDiscoveryConfigs, 1) - assert.Equal(t, scrapeConfig.ServiceDiscoveryConfigs[0].Name(), "http") - assert.Equal(t, scrapeConfig.ServiceDiscoveryConfigs[0].(*http.SDConfig).URL, "http://test-targetallocator:80/jobs/"+scrapeConfig.JobName+"/targets?collector_id=$POD_NAME") - expectedMap[scrapeConfig.JobName] = true - } - for k := range expectedMap { - assert.True(t, expectedMap[k], k) - } - assert.True(t, cfg.TargetAllocConfig == nil) - }) - t.Run("should update config with targetAllocator block if block not present", func(t *testing.T) { // Set up the test scenario param.OtelCol.Spec.TargetAllocator.Enabled = true @@ -171,26 +131,7 @@ func TestReplaceConfig(t *testing.T) { assert.YAMLEq(t, expectedConfig, actualConfig) }) - t.Run("should rewrite scrape configs with SD config when TargetAllocator is enabled and feature flag is not set", func(t *testing.T) { - err := colfeaturegate.GlobalRegistry().Set(featuregate.EnableTargetAllocatorRewrite.ID(), false) - require.NoError(t, err) - t.Cleanup(func() { - _ = colfeaturegate.GlobalRegistry().Set(featuregate.EnableTargetAllocatorRewrite.ID(), true) - }) - - param.OtelCol.Spec.TargetAllocator.Enabled = true - - expectedConfigBytes, err := os.ReadFile("testdata/relabel_config_expected_with_sd_config.yaml") - assert.NoError(t, err) - expectedConfig := string(expectedConfigBytes) - - actualConfig, err := ReplaceConfig(param.OtelCol) - assert.NoError(t, err) - - assert.YAMLEq(t, expectedConfig, actualConfig) - }) - - t.Run("should remove scrape configs if TargetAllocator is enabled and feature flag is set", func(t *testing.T) { + t.Run("should remove scrape configs if TargetAllocator is enabled", func(t *testing.T) { param.OtelCol.Spec.TargetAllocator.Enabled = true expectedConfigBytes, err := os.ReadFile("testdata/config_expected_targetallocator.yaml") diff --git a/internal/manifests/collector/configmap_test.go b/internal/manifests/collector/configmap_test.go index 525a5e80be..b850084c53 100644 --- a/internal/manifests/collector/configmap_test.go +++ b/internal/manifests/collector/configmap_test.go @@ -17,11 +17,7 @@ package collector import ( "testing" - colfeaturegate "go.opentelemetry.io/collector/featuregate" - "github.com/stretchr/testify/assert" - - "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" ) func TestDesiredConfigMap(t *testing.T) { @@ -73,111 +69,6 @@ service: } }) - t.Run("should return expected collector config map with http_sd_config if rewrite flag disabled", func(t *testing.T) { - err := colfeaturegate.GlobalRegistry().Set(featuregate.EnableTargetAllocatorRewrite.ID(), false) - assert.NoError(t, err) - t.Cleanup(func() { - _ = colfeaturegate.GlobalRegistry().Set(featuregate.EnableTargetAllocatorRewrite.ID(), true) - }) - expectedLables["app.kubernetes.io/component"] = "opentelemetry-collector" - expectedLables["app.kubernetes.io/name"] = "test-collector" - - expectedData := map[string]string{ - "collector.yaml": `exporters: - debug: -receivers: - jaeger: - protocols: - grpc: null - prometheus: - config: - scrape_configs: - - http_sd_configs: - - url: http://test-targetallocator:80/jobs/otel-collector/targets?collector_id=$POD_NAME - job_name: otel-collector - scrape_interval: 10s -service: - pipelines: - metrics: - exporters: - - debug - processors: [] - receivers: - - prometheus - - jaeger -`, - } - - param := deploymentParams() - param.OtelCol.Spec.TargetAllocator.Enabled = true - actual, err := ConfigMap(param) - - assert.NoError(t, err) - assert.Equal(t, "test-collector", actual.GetName()) - assert.Equal(t, expectedLables, actual.GetLabels()) - assert.Equal(t, len(expectedData), len(actual.Data)) - for k, expected := range expectedData { - assert.YAMLEq(t, expected, actual.Data[k]) - } - - }) - - t.Run("should return expected escaped collector config map with http_sd_config if rewrite flag disabled", func(t *testing.T) { - err := colfeaturegate.GlobalRegistry().Set(featuregate.EnableTargetAllocatorRewrite.ID(), false) - assert.NoError(t, err) - t.Cleanup(func() { - _ = colfeaturegate.GlobalRegistry().Set(featuregate.EnableTargetAllocatorRewrite.ID(), true) - }) - - expectedLables["app.kubernetes.io/component"] = "opentelemetry-collector" - expectedLables["app.kubernetes.io/name"] = "test-collector" - expectedLables["app.kubernetes.io/version"] = "latest" - - expectedData := map[string]string{ - "collector.yaml": `exporters: - debug: -receivers: - prometheus: - config: - scrape_configs: - - http_sd_configs: - - url: http://test-targetallocator:80/jobs/serviceMonitor%2Ftest%2Ftest%2F0/targets?collector_id=$POD_NAME - job_name: serviceMonitor/test/test/0 - target_allocator: - collector_id: ${POD_NAME} - endpoint: http://test-targetallocator:80 - http_sd_config: - refresh_interval: 60s - interval: 30s -service: - pipelines: - metrics: - exporters: - - debug - processors: [] - receivers: - - prometheus -`, - } - - param, err := newParams("test/test-img", "testdata/http_sd_config_servicemonitor_test_ta_set.yaml") - assert.NoError(t, err) - param.OtelCol.Spec.TargetAllocator.Enabled = true - actual, err := ConfigMap(param) - - assert.NoError(t, err) - assert.Equal(t, "test-collector", actual.Name) - assert.Equal(t, expectedLables, actual.Labels) - assert.Equal(t, len(expectedData), len(actual.Data)) - for k, expected := range expectedData { - assert.YAMLEq(t, expected, actual.Data[k]) - } - - // Reset the value - expectedLables["app.kubernetes.io/version"] = "0.47.0" - - }) - t.Run("should return expected escaped collector config map with target_allocator config block", func(t *testing.T) { expectedLables["app.kubernetes.io/component"] = "opentelemetry-collector" expectedLables["app.kubernetes.io/name"] = "test-collector" diff --git a/pkg/featuregate/featuregate.go b/pkg/featuregate/featuregate.go index 793c19ab1e..ca3ecde484 100644 --- a/pkg/featuregate/featuregate.go +++ b/pkg/featuregate/featuregate.go @@ -53,9 +53,10 @@ var ( // automatically be rewritten when the target allocator is enabled. EnableTargetAllocatorRewrite = featuregate.GlobalRegistry().MustRegister( "operator.collector.rewritetargetallocator", - featuregate.StageBeta, + featuregate.StageStable, featuregate.WithRegisterDescription("controls whether the operator should configure the collector's targetAllocator configuration"), featuregate.WithRegisterFromVersion("v0.76.1"), + featuregate.WithRegisterToVersion("v0.98.0"), ) // PrometheusOperatorIsAvailable is the feature gate that enables features associated to the Prometheus Operator. From d4b24f3b9d59e31534f1722f78ac745875a3374a Mon Sep 17 00:00:00 2001 From: Alessio Trivisonno Date: Mon, 1 Apr 2024 16:38:49 +0200 Subject: [PATCH 5/5] Change nginx instrumentation into command line (#2777) * feat: change nginx instrumentation to cli flag * set nginx instrumentation to default false * update config/manager * enable nginx in config/manager * update chloggen --- .../autoinstrumentation-nginx-cli-flag.yaml | 16 ++++++++++++++++ ...telemetry-operator.clusterserviceversion.yaml | 5 +++-- config/manager/manager.yaml | 3 ++- internal/config/main.go | 7 +++++++ internal/config/options.go | 6 ++++++ main.go | 4 ++++ pkg/constants/env.go | 1 + pkg/featuregate/featuregate.go | 6 ------ pkg/instrumentation/podmutator.go | 2 +- pkg/instrumentation/podmutator_test.go | 16 ++-------------- pkg/instrumentation/upgrade/upgrade.go | 12 ++++++------ pkg/instrumentation/upgrade/upgrade_test.go | 8 ++------ .../opentelemetry-operator-v0.86.0.yaml | 2 +- 13 files changed, 51 insertions(+), 37 deletions(-) create mode 100644 .chloggen/autoinstrumentation-nginx-cli-flag.yaml diff --git a/.chloggen/autoinstrumentation-nginx-cli-flag.yaml b/.chloggen/autoinstrumentation-nginx-cli-flag.yaml new file mode 100644 index 0000000000..5f925117fa --- /dev/null +++ b/.chloggen/autoinstrumentation-nginx-cli-flag.yaml @@ -0,0 +1,16 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: breaking + +# The name of the component, or a single word describing the area of concern, (e.g. operator, target allocator, github action) +component: operator + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: change nginx instrumentation feature gate operator.autoinstrumentation.nginx into command line flag --enable-nginx-instrumentation + +# One or more tracking issues related to the change +issues: [2582, 2676] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: diff --git a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml index eaa0bc7aa2..a10d4637f3 100644 --- a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml +++ b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml @@ -99,7 +99,7 @@ metadata: categories: Logging & Tracing,Monitoring certified: "false" containerImage: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator - createdAt: "2024-03-11T13:32:19Z" + createdAt: "2024-03-25T08:38:05Z" description: Provides the OpenTelemetry components, including the Collector operators.operatorframework.io/builder: operator-sdk-v1.29.0 operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 @@ -487,7 +487,8 @@ spec: - --enable-leader-election - --zap-log-level=info - --zap-time-encoding=rfc3339nano - - --feature-gates=+operator.autoinstrumentation.go,+operator.autoinstrumentation.nginx + - --feature-gates=+operator.autoinstrumentation.go + - --enable-nginx-instrumentation=true image: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator:0.96.0 livenessProbe: httpGet: diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 5598f0bd81..76eb71ac95 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -32,7 +32,8 @@ spec: - "--enable-leader-election" - "--zap-log-level=info" - "--zap-time-encoding=rfc3339nano" - - "--feature-gates=+operator.autoinstrumentation.go,+operator.autoinstrumentation.nginx" + - "--feature-gates=+operator.autoinstrumentation.go" + - "--enable-nginx-instrumentation=true" image: controller name: manager livenessProbe: diff --git a/internal/config/main.go b/internal/config/main.go index c8e24e8560..61e1512695 100644 --- a/internal/config/main.go +++ b/internal/config/main.go @@ -46,6 +46,7 @@ type Config struct { enableMultiInstrumentation bool enableApacheHttpdInstrumentation bool enableDotNetInstrumentation bool + enableNginxInstrumentation bool enablePythonInstrumentation bool autoInstrumentationDotNetImage string autoInstrumentationGoImage string @@ -83,6 +84,7 @@ func New(opts ...Option) Config { enableMultiInstrumentation: o.enableMultiInstrumentation, enableApacheHttpdInstrumentation: o.enableApacheHttpdInstrumentation, enableDotNetInstrumentation: o.enableDotNetInstrumentation, + enableNginxInstrumentation: o.enableNginxInstrumentation, enablePythonInstrumentation: o.enablePythonInstrumentation, targetAllocatorImage: o.targetAllocatorImage, operatorOpAMPBridgeImage: o.operatorOpAMPBridgeImage, @@ -134,6 +136,11 @@ func (c *Config) EnableDotNetAutoInstrumentation() bool { return c.enableDotNetInstrumentation } +// EnableNginxAutoInstrumentation is true when the operator supports nginx auto instrumentation. +func (c *Config) EnableNginxAutoInstrumentation() bool { + return c.enableNginxInstrumentation +} + // EnablePythonAutoInstrumentation is true when the operator supports dotnet auto instrumentation. func (c *Config) EnablePythonAutoInstrumentation() bool { return c.enablePythonInstrumentation diff --git a/internal/config/options.go b/internal/config/options.go index ab74059f04..9d150dd247 100644 --- a/internal/config/options.go +++ b/internal/config/options.go @@ -45,6 +45,7 @@ type options struct { enableMultiInstrumentation bool enableApacheHttpdInstrumentation bool enableDotNetInstrumentation bool + enableNginxInstrumentation bool enablePythonInstrumentation bool targetAllocatorConfigMapEntry string operatorOpAMPBridgeConfigMapEntry string @@ -100,6 +101,11 @@ func WithEnableDotNetInstrumentation(s bool) Option { o.enableDotNetInstrumentation = s } } +func WithEnableNginxInstrumentation(s bool) Option { + return func(o *options) { + o.enableNginxInstrumentation = s + } +} func WithEnablePythonInstrumentation(s bool) Option { return func(o *options) { o.enablePythonInstrumentation = s diff --git a/main.go b/main.go index 8042aaeed5..5e57e669d4 100644 --- a/main.go +++ b/main.go @@ -113,6 +113,7 @@ func main() { enableApacheHttpdInstrumentation bool enableDotNetInstrumentation bool enablePythonInstrumentation bool + enableNginxInstrumentation bool collectorImage string targetAllocatorImage string operatorOpAMPBridgeImage string @@ -140,6 +141,7 @@ func main() { pflag.BoolVar(&enableApacheHttpdInstrumentation, constants.FlagApacheHttpd, true, "Controls whether the operator supports Apache HTTPD auto-instrumentation") pflag.BoolVar(&enableDotNetInstrumentation, constants.FlagDotNet, true, "Controls whether the operator supports dotnet auto-instrumentation") pflag.BoolVar(&enablePythonInstrumentation, constants.FlagPython, true, "Controls whether the operator supports python auto-instrumentation") + pflag.BoolVar(&enableNginxInstrumentation, constants.FlagNginx, false, "Controls whether the operator supports nginx auto-instrumentation") stringFlagOrEnv(&collectorImage, "collector-image", "RELATED_IMAGE_COLLECTOR", fmt.Sprintf("ghcr.io/open-telemetry/opentelemetry-collector-releases/opentelemetry-collector:%s", v.OpenTelemetryCollector), "The default OpenTelemetry collector image. This image is used when no image is specified in the CustomResource.") stringFlagOrEnv(&targetAllocatorImage, "target-allocator-image", "RELATED_IMAGE_TARGET_ALLOCATOR", fmt.Sprintf("ghcr.io/open-telemetry/opentelemetry-operator/target-allocator:%s", v.TargetAllocator), "The default OpenTelemetry target allocator image. This image is used when no image is specified in the CustomResource.") stringFlagOrEnv(&operatorOpAMPBridgeImage, "operator-opamp-bridge-image", "RELATED_IMAGE_OPERATOR_OPAMP_BRIDGE", fmt.Sprintf("ghcr.io/open-telemetry/opentelemetry-operator/operator-opamp-bridge:%s", v.OperatorOpAMPBridge), "The default OpenTelemetry Operator OpAMP Bridge image. This image is used when no image is specified in the CustomResource.") @@ -183,6 +185,7 @@ func main() { "enable-apache-httpd-instrumentation", enableApacheHttpdInstrumentation, "enable-dotnet-instrumentation", enableDotNetInstrumentation, "enable-python-instrumentation", enablePythonInstrumentation, + "enable-nginx-instrumentation", enableNginxInstrumentation, ) restConfig := ctrl.GetConfigOrDie() @@ -202,6 +205,7 @@ func main() { config.WithEnableMultiInstrumentation(enableMultiInstrumentation), config.WithEnableApacheHttpdInstrumentation(enableApacheHttpdInstrumentation), config.WithEnableDotNetInstrumentation(enableDotNetInstrumentation), + config.WithEnableNginxInstrumentation(enableNginxInstrumentation), config.WithEnablePythonInstrumentation(enablePythonInstrumentation), config.WithTargetAllocatorImage(targetAllocatorImage), config.WithOperatorOpAMPBridgeImage(operatorOpAMPBridgeImage), diff --git a/pkg/constants/env.go b/pkg/constants/env.go index 12391fde7e..01180842a4 100644 --- a/pkg/constants/env.go +++ b/pkg/constants/env.go @@ -38,4 +38,5 @@ const ( FlagApacheHttpd = "enable-apache-httpd-instrumentation" FlagDotNet = "enable-dotnet-instrumentation" FlagPython = "enable-python-instrumentation" + FlagNginx = "enable-nginx-instrumentation" ) diff --git a/pkg/featuregate/featuregate.go b/pkg/featuregate/featuregate.go index ca3ecde484..a20d589b35 100644 --- a/pkg/featuregate/featuregate.go +++ b/pkg/featuregate/featuregate.go @@ -43,12 +43,6 @@ var ( featuregate.WithRegisterDescription("controls whether the operator supports Golang auto-instrumentation"), featuregate.WithRegisterFromVersion("v0.77.0"), ) - EnableNginxAutoInstrumentationSupport = featuregate.GlobalRegistry().MustRegister( - "operator.autoinstrumentation.nginx", - featuregate.StageAlpha, - featuregate.WithRegisterDescription("controls whether the operator supports Nginx auto-instrumentation"), - featuregate.WithRegisterFromVersion("v0.86.0"), - ) // EnableTargetAllocatorRewrite is the feature gate that controls whether the collector's configuration should // automatically be rewritten when the target allocator is enabled. EnableTargetAllocatorRewrite = featuregate.GlobalRegistry().MustRegister( diff --git a/pkg/instrumentation/podmutator.go b/pkg/instrumentation/podmutator.go index 0a4fbde291..8cdd9cfa81 100644 --- a/pkg/instrumentation/podmutator.go +++ b/pkg/instrumentation/podmutator.go @@ -302,7 +302,7 @@ func (pm *instPodMutator) Mutate(ctx context.Context, ns corev1.Namespace, pod c logger.Error(err, "failed to select an OpenTelemetry Instrumentation instance for this pod") return pod, err } - if featuregate.EnableNginxAutoInstrumentationSupport.IsEnabled() || inst == nil { + if pm.config.EnableNginxAutoInstrumentation() || inst == nil { insts.Nginx.Instrumentation = inst } else { logger.Error(nil, "support for Nginx auto instrumentation is not enabled") diff --git a/pkg/instrumentation/podmutator_test.go b/pkg/instrumentation/podmutator_test.go index 7343db7525..436cba28db 100644 --- a/pkg/instrumentation/podmutator_test.go +++ b/pkg/instrumentation/podmutator_test.go @@ -2972,13 +2972,7 @@ func TestMutatePod(t *testing.T) { }, }, }, - setFeatureGates: func(t *testing.T) { - originalVal := featuregate.EnableNginxAutoInstrumentationSupport.IsEnabled() - require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableNginxAutoInstrumentationSupport.ID(), true)) - t.Cleanup(func() { - require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableNginxAutoInstrumentationSupport.ID(), originalVal)) - }) - }, + config: config.New(config.WithEnableNginxInstrumentation(true)), }, { name: "nginx injection feature gate disabled", @@ -3059,13 +3053,7 @@ func TestMutatePod(t *testing.T) { }, }, }, - setFeatureGates: func(t *testing.T) { - originalVal := featuregate.EnableNginxAutoInstrumentationSupport.IsEnabled() - require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableNginxAutoInstrumentationSupport.ID(), false)) - t.Cleanup(func() { - require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableNginxAutoInstrumentationSupport.ID(), originalVal)) - }) - }, + config: config.New(config.WithEnableDotNetInstrumentation(false)), }, { diff --git a/pkg/instrumentation/upgrade/upgrade.go b/pkg/instrumentation/upgrade/upgrade.go index ae0cd1789c..30922711e6 100644 --- a/pkg/instrumentation/upgrade/upgrade.go +++ b/pkg/instrumentation/upgrade/upgrade.go @@ -35,7 +35,6 @@ var ( constants.AnnotationDefaultAutoInstrumentationJava: featuregate.EnableJavaAutoInstrumentationSupport, constants.AnnotationDefaultAutoInstrumentationNodeJS: featuregate.EnableNodeJSAutoInstrumentationSupport, constants.AnnotationDefaultAutoInstrumentationGo: featuregate.EnableGoAutoInstrumentationSupport, - constants.AnnotationDefaultAutoInstrumentationNginx: featuregate.EnableNginxAutoInstrumentationSupport, } ) @@ -62,6 +61,7 @@ func NewInstrumentationUpgrade(client client.Client, logger logr.Logger, recorde defaultAnnotationToConfig := map[string]autoInstConfig{ constants.AnnotationDefaultAutoInstrumentationApacheHttpd: {constants.FlagApacheHttpd, cfg.EnableApacheHttpdAutoInstrumentation()}, constants.AnnotationDefaultAutoInstrumentationDotNet: {constants.FlagDotNet, cfg.EnableDotNetAutoInstrumentation()}, + constants.AnnotationDefaultAutoInstrumentationNginx: {constants.FlagNginx, cfg.EnableNginxAutoInstrumentation()}, constants.AnnotationDefaultAutoInstrumentationPython: {constants.FlagPython, cfg.EnablePythonAutoInstrumentation()}, } @@ -132,6 +132,11 @@ func (u *InstrumentationUpgrade) upgrade(_ context.Context, inst v1alpha1.Instru upgraded.Spec.DotNet.Image = u.DefaultAutoInstDotNet upgraded.Annotations[annotation] = u.DefaultAutoInstDotNet } + case constants.AnnotationDefaultAutoInstrumentationNginx: + if inst.Spec.Nginx.Image == autoInst { + upgraded.Spec.Nginx.Image = u.DefaultAutoInstNginx + upgraded.Annotations[annotation] = u.DefaultAutoInstNginx + } case constants.AnnotationDefaultAutoInstrumentationPython: if inst.Spec.Python.Image == autoInst { upgraded.Spec.Python.Image = u.DefaultAutoInstPython @@ -170,11 +175,6 @@ func (u *InstrumentationUpgrade) upgrade(_ context.Context, inst v1alpha1.Instru upgraded.Spec.Go.Image = u.DefaultAutoInstGo upgraded.Annotations[annotation] = u.DefaultAutoInstGo } - case constants.AnnotationDefaultAutoInstrumentationNginx: - if inst.Spec.Nginx.Image == autoInst { - upgraded.Spec.Nginx.Image = u.DefaultAutoInstNginx - upgraded.Annotations[annotation] = u.DefaultAutoInstNginx - } } } else { u.Logger.Error(nil, "autoinstrumentation not enabled for this language", "flag", gate.ID()) diff --git a/pkg/instrumentation/upgrade/upgrade_test.go b/pkg/instrumentation/upgrade/upgrade_test.go index 546c7d3e40..900360b631 100644 --- a/pkg/instrumentation/upgrade/upgrade_test.go +++ b/pkg/instrumentation/upgrade/upgrade_test.go @@ -42,12 +42,6 @@ func TestUpgrade(t *testing.T) { require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableGoAutoInstrumentationSupport.ID(), originalVal)) }) - originalVal = featuregate.EnableNginxAutoInstrumentationSupport.IsEnabled() - require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableNginxAutoInstrumentationSupport.ID(), true)) - t.Cleanup(func() { - require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableNginxAutoInstrumentationSupport.ID(), originalVal)) - }) - nsName := strings.ToLower(t.Name()) err := k8sClient.Create(context.Background(), &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ @@ -80,6 +74,7 @@ func TestUpgrade(t *testing.T) { config.WithAutoInstrumentationNginxImage("nginx:1"), config.WithEnableApacheHttpdInstrumentation(true), config.WithEnableDotNetInstrumentation(true), + config.WithEnableNginxInstrumentation(true), config.WithEnablePythonInstrumentation(true), ), ).Default(context.Background(), inst) @@ -104,6 +99,7 @@ func TestUpgrade(t *testing.T) { config.WithAutoInstrumentationNginxImage("nginx:2"), config.WithEnableApacheHttpdInstrumentation(true), config.WithEnableDotNetInstrumentation(true), + config.WithEnableNginxInstrumentation(true), config.WithEnablePythonInstrumentation(true), ) up := NewInstrumentationUpgrade(k8sClient, ctrl.Log.WithName("instrumentation-upgrade"), &record.FakeRecorder{}, cfg) diff --git a/tests/e2e-upgrade/upgrade-test/opentelemetry-operator-v0.86.0.yaml b/tests/e2e-upgrade/upgrade-test/opentelemetry-operator-v0.86.0.yaml index d83aec3df2..12afe3893b 100644 --- a/tests/e2e-upgrade/upgrade-test/opentelemetry-operator-v0.86.0.yaml +++ b/tests/e2e-upgrade/upgrade-test/opentelemetry-operator-v0.86.0.yaml @@ -8317,7 +8317,7 @@ spec: - --enable-leader-election - --zap-log-level=info - --zap-time-encoding=rfc3339nano - - --feature-gates=+operator.autoinstrumentation.go,+operator.autoinstrumentation.nginx + - --feature-gates=+operator.autoinstrumentation.go image: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator:0.86.0 livenessProbe: httpGet: