Skip to content

Commit 61daef2

Browse files
swiatekmswiatekm
authored
Remove operator.collector.rewritetargetallocator feature flag (#2851)
1 parent b2c4f3c commit 61daef2

File tree

6 files changed

+47
-84
lines changed

6 files changed

+47
-84
lines changed

‎.chloggen/chore_remove-ta-rewrite-flag.yaml

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
2+
change_type: breaking
3+
4+
# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
5+
component: target allocator
6+
7+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
8+
note: Remove `operator.collector.rewritetargetallocator` feature flag
9+
10+
# One or more tracking issues related to the change
11+
issues: [2796]
12+
13+
# (Optional) One or more lines of additional information to render under the primary note.
14+
# These lines will be padded with 2 spaces and then inserted directly into the document.
15+
# Use pipe (|) for multiline entries.
16+
subtext:

‎apis/v1alpha1/collector_webhook.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ import (
3232
"github.com/open-telemetry/opentelemetry-operator/internal/config"
3333
ta "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator/adapters"
3434
"github.com/open-telemetry/opentelemetry-operator/internal/rbac"
35-
"github.com/open-telemetry/opentelemetry-operator/pkg/featuregate"
3635
)
3736

3837
var (
@@ -365,7 +364,7 @@ func (c CollectorWebhook) validateTargetAllocatorConfig(ctx context.Context, r *
365364
if err != nil {
366365
return nil, fmt.Errorf("the OpenTelemetry Spec Prometheus configuration is incorrect, %w", err)
367366
}
368-
err = ta.ValidatePromConfig(promCfg, r.Spec.TargetAllocator.Enabled, featuregate.EnableTargetAllocatorRewrite.IsEnabled())
367+
err = ta.ValidatePromConfig(promCfg, r.Spec.TargetAllocator.Enabled)
369368
if err != nil {
370369
return nil, fmt.Errorf("the OpenTelemetry Spec Prometheus configuration is incorrect, %w", err)
371370
}

‎internal/manifests/collector/config_replace.go

+8-28
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import (
2525
"github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters"
2626
ta "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator/adapters"
2727
"github.com/open-telemetry/opentelemetry-operator/internal/naming"
28-
"github.com/open-telemetry/opentelemetry-operator/pkg/featuregate"
2928
)
3029

3130
type targetAllocator struct {
@@ -62,43 +61,24 @@ func ReplaceConfig(instance v1beta1.OpenTelemetryCollector) (string, error) {
6261
return "", getCfgPromErr
6362
}
6463

65-
validateCfgPromErr := ta.ValidatePromConfig(promCfgMap, instance.Spec.TargetAllocator.Enabled, featuregate.EnableTargetAllocatorRewrite.IsEnabled())
64+
validateCfgPromErr := ta.ValidatePromConfig(promCfgMap, instance.Spec.TargetAllocator.Enabled)
6665
if validateCfgPromErr != nil {
6766
return "", validateCfgPromErr
6867
}
6968

70-
if featuregate.EnableTargetAllocatorRewrite.IsEnabled() {
71-
// To avoid issues caused by Prometheus validation logic, which fails regex validation when it encounters
72-
// $$ in the prom config, we update the YAML file directly without marshaling and unmarshalling.
73-
updPromCfgMap, getCfgPromErr := ta.AddTAConfigToPromConfig(promCfgMap, naming.TAService(instance.Name))
74-
if getCfgPromErr != nil {
75-
return "", getCfgPromErr
76-
}
77-
78-
// type coercion checks are handled in the AddTAConfigToPromConfig method above
79-
config["receivers"].(map[interface{}]interface{})["prometheus"] = updPromCfgMap
80-
81-
out, updCfgMarshalErr := yaml.Marshal(config)
82-
if updCfgMarshalErr != nil {
83-
return "", updCfgMarshalErr
84-
}
85-
return string(out), nil
86-
}
87-
8869
// To avoid issues caused by Prometheus validation logic, which fails regex validation when it encounters
8970
// $$ in the prom config, we update the YAML file directly without marshaling and unmarshalling.
90-
updPromCfgMap, err := ta.AddHTTPSDConfigToPromConfig(promCfgMap, naming.TAService(instance.Name))
91-
if err != nil {
92-
return "", err
71+
updPromCfgMap, getCfgPromErr := ta.AddTAConfigToPromConfig(promCfgMap, naming.TAService(instance.Name))
72+
if getCfgPromErr != nil {
73+
return "", getCfgPromErr
9374
}
9475

95-
// To avoid issues caused by Prometheus validation logic, which fails regex validation when it encounters
96-
// type coercion checks are handled in the ConfigToPromConfig method above
76+
// type coercion checks are handled in the AddTAConfigToPromConfig method above
9777
config["receivers"].(map[interface{}]interface{})["prometheus"] = updPromCfgMap
9878

99-
out, err := yaml.Marshal(config)
100-
if err != nil {
101-
return "", err
79+
out, updCfgMarshalErr := yaml.Marshal(config)
80+
if updCfgMarshalErr != nil {
81+
return "", updCfgMarshalErr
10282
}
10383

10484
return string(out), nil

‎internal/manifests/targetallocator/adapters/config_to_prom_config.go

+3-12
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
package adapters
1616

1717
import (
18-
"errors"
1918
"fmt"
2019
"net/url"
2120
"regexp"
@@ -297,20 +296,12 @@ func AddTAConfigToPromConfig(prometheus map[interface{}]interface{}, taServiceNa
297296
}
298297

299298
// ValidatePromConfig checks if the prometheus receiver config is valid given other collector-level settings.
300-
func ValidatePromConfig(config map[interface{}]interface{}, targetAllocatorEnabled bool, targetAllocatorRewriteEnabled bool) error {
299+
func ValidatePromConfig(config map[interface{}]interface{}, targetAllocatorEnabled bool) error {
300+
// TODO: Rethink this validation, now that target allocator rewrite is enabled permanently.
301+
301302
_, promConfigExists := config["config"]
302303

303304
if targetAllocatorEnabled {
304-
if targetAllocatorRewriteEnabled { // if rewrite is enabled, we will add a target_allocator section during rewrite
305-
return nil
306-
}
307-
_, targetAllocatorExists := config["target_allocator"]
308-
309-
// otherwise, either the target_allocator or config section needs to be here
310-
if !(promConfigExists || targetAllocatorExists) {
311-
return errors.New("either target allocator or prometheus config needs to be present")
312-
}
313-
314305
return nil
315306
}
316307
// if target allocator isn't enabled, we need a config section

‎internal/manifests/targetallocator/adapters/config_to_prom_config_test.go

+19-33
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
package adapters_test
1616

1717
import (
18-
"errors"
1918
"fmt"
2019
"net/url"
2120
"reflect"
@@ -302,66 +301,53 @@ func TestAddTAConfigToPromConfig(t *testing.T) {
302301

303302
func TestValidatePromConfig(t *testing.T) {
304303
testCases := []struct {
305-
description string
306-
config map[interface{}]interface{}
307-
targetAllocatorEnabled bool
308-
targetAllocatorRewriteEnabled bool
309-
expectedError error
304+
description string
305+
config map[interface{}]interface{}
306+
targetAllocatorEnabled bool
307+
expectedError error
310308
}{
311309
{
312-
description: "target_allocator and rewrite enabled",
313-
config: map[interface{}]interface{}{},
314-
targetAllocatorEnabled: true,
315-
targetAllocatorRewriteEnabled: true,
316-
expectedError: nil,
310+
description: "target_allocator enabled",
311+
config: map[interface{}]interface{}{},
312+
targetAllocatorEnabled: true,
313+
expectedError: nil,
317314
},
318315
{
319316
description: "target_allocator enabled, target_allocator section present",
320317
config: map[interface{}]interface{}{
321318
"target_allocator": map[interface{}]interface{}{},
322319
},
323-
targetAllocatorEnabled: true,
324-
targetAllocatorRewriteEnabled: false,
325-
expectedError: nil,
320+
targetAllocatorEnabled: true,
321+
expectedError: nil,
326322
},
327323
{
328324
description: "target_allocator enabled, config section present",
329325
config: map[interface{}]interface{}{
330326
"config": map[interface{}]interface{}{},
331327
},
332-
targetAllocatorEnabled: true,
333-
targetAllocatorRewriteEnabled: false,
334-
expectedError: nil,
335-
},
336-
{
337-
description: "target_allocator enabled, neither section present",
338-
config: map[interface{}]interface{}{},
339-
targetAllocatorEnabled: true,
340-
targetAllocatorRewriteEnabled: false,
341-
expectedError: errors.New("either target allocator or prometheus config needs to be present"),
328+
targetAllocatorEnabled: true,
329+
expectedError: nil,
342330
},
343331
{
344332
description: "target_allocator disabled, config section present",
345333
config: map[interface{}]interface{}{
346334
"config": map[interface{}]interface{}{},
347335
},
348-
targetAllocatorEnabled: false,
349-
targetAllocatorRewriteEnabled: false,
350-
expectedError: nil,
336+
targetAllocatorEnabled: false,
337+
expectedError: nil,
351338
},
352339
{
353-
description: "target_allocator disabled, config section not present",
354-
config: map[interface{}]interface{}{},
355-
targetAllocatorEnabled: false,
356-
targetAllocatorRewriteEnabled: false,
357-
expectedError: fmt.Errorf("no %s available as part of the configuration", "prometheusConfig"),
340+
description: "target_allocator disabled, config section not present",
341+
config: map[interface{}]interface{}{},
342+
targetAllocatorEnabled: false,
343+
expectedError: fmt.Errorf("no %s available as part of the configuration", "prometheusConfig"),
358344
},
359345
}
360346

361347
for _, testCase := range testCases {
362348
testCase := testCase
363349
t.Run(testCase.description, func(t *testing.T) {
364-
err := ta.ValidatePromConfig(testCase.config, testCase.targetAllocatorEnabled, testCase.targetAllocatorRewriteEnabled)
350+
err := ta.ValidatePromConfig(testCase.config, testCase.targetAllocatorEnabled)
365351
assert.Equal(t, testCase.expectedError, err)
366352
})
367353
}

‎pkg/featuregate/featuregate.go

-9
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,6 @@ var (
4343
featuregate.WithRegisterDescription("controls whether the operator supports Golang auto-instrumentation"),
4444
featuregate.WithRegisterFromVersion("v0.77.0"),
4545
)
46-
// EnableTargetAllocatorRewrite is the feature gate that controls whether the collector's configuration should
47-
// automatically be rewritten when the target allocator is enabled.
48-
EnableTargetAllocatorRewrite = featuregate.GlobalRegistry().MustRegister(
49-
"operator.collector.rewritetargetallocator",
50-
featuregate.StageStable,
51-
featuregate.WithRegisterDescription("controls whether the operator should configure the collector's targetAllocator configuration"),
52-
featuregate.WithRegisterFromVersion("v0.76.1"),
53-
featuregate.WithRegisterToVersion("v0.98.0"),
54-
)
5546

5647
// PrometheusOperatorIsAvailable is the feature gate that enables features associated to the Prometheus Operator.
5748
PrometheusOperatorIsAvailable = featuregate.GlobalRegistry().MustRegister(

0 commit comments

Comments
 (0)