Skip to content

Commit

Permalink
Python instrumentation featuregate to cli (#2751)
Browse files Browse the repository at this point in the history
* remove python feature gate, use command-line instead

* add chlog

* fix python upgrade

* fix typo at main

* fix chlog
  • Loading branch information
changexd authored Mar 14, 2024
1 parent 1741863 commit 6a0b11e
Show file tree
Hide file tree
Showing 10 changed files with 47 additions and 15 deletions.
16 changes: 16 additions & 0 deletions .chloggen/featuregate_cli_python.yaml
Original file line number Diff line number Diff line change
@@ -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: operator

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: remove featuregate `operator.autoinstrumentation.python`. Use command line flag `--enable-python-instrumentation` instead

# One or more tracking issues related to the change
issues: [2582, 2672]

# (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:
7 changes: 7 additions & 0 deletions internal/config/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ type Config struct {
enableMultiInstrumentation bool
enableApacheHttpdInstrumentation bool
enableDotNetInstrumentation bool
enablePythonInstrumentation bool
autoInstrumentationDotNetImage string
autoInstrumentationGoImage string
autoInstrumentationApacheHttpdImage string
Expand Down Expand Up @@ -82,6 +83,7 @@ func New(opts ...Option) Config {
enableMultiInstrumentation: o.enableMultiInstrumentation,
enableApacheHttpdInstrumentation: o.enableApacheHttpdInstrumentation,
enableDotNetInstrumentation: o.enableDotNetInstrumentation,
enablePythonInstrumentation: o.enablePythonInstrumentation,
targetAllocatorImage: o.targetAllocatorImage,
operatorOpAMPBridgeImage: o.operatorOpAMPBridgeImage,
targetAllocatorConfigMapEntry: o.targetAllocatorConfigMapEntry,
Expand Down Expand Up @@ -132,6 +134,11 @@ func (c *Config) EnableDotNetAutoInstrumentation() bool {
return c.enableDotNetInstrumentation
}

// EnablePythonAutoInstrumentation is true when the operator supports dotnet auto instrumentation.
func (c *Config) EnablePythonAutoInstrumentation() bool {
return c.enablePythonInstrumentation
}

// CollectorConfigMapEntry represents the configuration file name for the collector. Immutable.
func (c *Config) CollectorConfigMapEntry() string {
return c.collectorConfigMapEntry
Expand Down
6 changes: 6 additions & 0 deletions internal/config/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ type options struct {
enableMultiInstrumentation bool
enableApacheHttpdInstrumentation bool
enableDotNetInstrumentation bool
enablePythonInstrumentation bool
targetAllocatorConfigMapEntry string
operatorOpAMPBridgeConfigMapEntry string
targetAllocatorImage string
Expand Down Expand Up @@ -99,6 +100,11 @@ func WithEnableDotNetInstrumentation(s bool) Option {
o.enableDotNetInstrumentation = s
}
}
func WithEnablePythonInstrumentation(s bool) Option {
return func(o *options) {
o.enablePythonInstrumentation = s
}
}
func WithTargetAllocatorConfigMapEntry(s string) Option {
return func(o *options) {
o.targetAllocatorConfigMapEntry = s
Expand Down
4 changes: 4 additions & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ func main() {
enableMultiInstrumentation bool
enableApacheHttpdInstrumentation bool
enableDotNetInstrumentation bool
enablePythonInstrumentation bool
collectorImage string
targetAllocatorImage string
operatorOpAMPBridgeImage string
Expand All @@ -138,6 +139,7 @@ func main() {
pflag.BoolVar(&enableMultiInstrumentation, "enable-multi-instrumentation", false, "Controls whether the operator supports multi instrumentation")
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")
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.")
Expand Down Expand Up @@ -180,6 +182,7 @@ func main() {
"enable-multi-instrumentation", enableMultiInstrumentation,
"enable-apache-httpd-instrumentation", enableApacheHttpdInstrumentation,
"enable-dotnet-instrumentation", enableDotNetInstrumentation,
"enable-python-instrumentation", enablePythonInstrumentation,
)

restConfig := ctrl.GetConfigOrDie()
Expand All @@ -199,6 +202,7 @@ func main() {
config.WithEnableMultiInstrumentation(enableMultiInstrumentation),
config.WithEnableApacheHttpdInstrumentation(enableApacheHttpdInstrumentation),
config.WithEnableDotNetInstrumentation(enableDotNetInstrumentation),
config.WithEnablePythonInstrumentation(enablePythonInstrumentation),
config.WithTargetAllocatorImage(targetAllocatorImage),
config.WithOperatorOpAMPBridgeImage(operatorOpAMPBridgeImage),
config.WithAutoInstrumentationJavaImage(autoInstrumentationJava),
Expand Down
1 change: 1 addition & 0 deletions pkg/constants/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,5 @@ const (

FlagApacheHttpd = "enable-apache-httpd-instrumentation"
FlagDotNet = "enable-dotnet-instrumentation"
FlagPython = "enable-python-instrumentation"
)
6 changes: 0 additions & 6 deletions pkg/featuregate/featuregate.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,6 @@ const (
)

var (
EnablePythonAutoInstrumentationSupport = featuregate.GlobalRegistry().MustRegister(
"operator.autoinstrumentation.python",
featuregate.StageBeta,
featuregate.WithRegisterDescription("controls whether the operator supports Python auto-instrumentation"),
featuregate.WithRegisterFromVersion("v0.76.1"),
)
EnableJavaAutoInstrumentationSupport = featuregate.GlobalRegistry().MustRegister(
"operator.autoinstrumentation.java",
featuregate.StageBeta,
Expand Down
2 changes: 1 addition & 1 deletion pkg/instrumentation/podmutator.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,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.EnablePythonAutoInstrumentationSupport.IsEnabled() || inst == nil {
if pm.config.EnablePythonAutoInstrumentation() || inst == nil {
insts.Python.Instrumentation = inst
} else {
logger.Error(nil, "support for Python auto instrumentation is not enabled")
Expand Down
11 changes: 4 additions & 7 deletions pkg/instrumentation/podmutator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1266,6 +1266,7 @@ func TestMutatePod(t *testing.T) {
},
},
},
config: config.New(config.WithEnablePythonInstrumentation(true)),
},
{
name: "python injection multiple containers, true",
Expand Down Expand Up @@ -1533,6 +1534,7 @@ func TestMutatePod(t *testing.T) {
},
},
},
config: config.New(config.WithEnablePythonInstrumentation(true)),
},
{
name: "python injection feature gate disabled",
Expand Down Expand Up @@ -1619,13 +1621,6 @@ func TestMutatePod(t *testing.T) {
},
},
},
setFeatureGates: func(t *testing.T) {
originalVal := featuregate.EnablePythonAutoInstrumentationSupport.IsEnabled()
require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnablePythonAutoInstrumentationSupport.ID(), false))
t.Cleanup(func() {
require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnablePythonAutoInstrumentationSupport.ID(), originalVal))
})
},
},
{
name: "dotnet injection, true",
Expand Down Expand Up @@ -3844,6 +3839,7 @@ func TestMutatePod(t *testing.T) {
},
config: config.New(
config.WithEnableMultiInstrumentation(true),
config.WithEnablePythonInstrumentation(true),
config.WithEnableDotNetInstrumentation(true),
),
},
Expand Down Expand Up @@ -4506,6 +4502,7 @@ func TestMutatePod(t *testing.T) {
config: config.New(
config.WithEnableMultiInstrumentation(true),
config.WithEnableDotNetInstrumentation(true),
config.WithEnablePythonInstrumentation(true),
),
},
{
Expand Down
7 changes: 6 additions & 1 deletion pkg/instrumentation/upgrade/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ var (
defaultAnnotationToGate = map[string]*featuregate2.Gate{
constants.AnnotationDefaultAutoInstrumentationJava: featuregate.EnableJavaAutoInstrumentationSupport,
constants.AnnotationDefaultAutoInstrumentationNodeJS: featuregate.EnableNodeJSAutoInstrumentationSupport,
constants.AnnotationDefaultAutoInstrumentationPython: featuregate.EnablePythonAutoInstrumentationSupport,
constants.AnnotationDefaultAutoInstrumentationGo: featuregate.EnableGoAutoInstrumentationSupport,
constants.AnnotationDefaultAutoInstrumentationNginx: featuregate.EnableNginxAutoInstrumentationSupport,
}
Expand Down Expand Up @@ -63,6 +62,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.AnnotationDefaultAutoInstrumentationPython: {constants.FlagPython, cfg.EnablePythonAutoInstrumentation()},
}

return &InstrumentationUpgrade{
Expand Down Expand Up @@ -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.AnnotationDefaultAutoInstrumentationPython:
if inst.Spec.Python.Image == autoInst {
upgraded.Spec.Python.Image = u.DefaultAutoInstPython
upgraded.Annotations[annotation] = u.DefaultAutoInstPython
}
}
} else {
u.Logger.Error(nil, "autoinstrumentation not enabled for this language", "flag", config.id)
Expand Down
2 changes: 2 additions & 0 deletions pkg/instrumentation/upgrade/upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ func TestUpgrade(t *testing.T) {
config.WithAutoInstrumentationNginxImage("nginx:1"),
config.WithEnableApacheHttpdInstrumentation(true),
config.WithEnableDotNetInstrumentation(true),
config.WithEnablePythonInstrumentation(true),
),
).Default(context.Background(), inst)
assert.Nil(t, err)
Expand All @@ -103,6 +104,7 @@ func TestUpgrade(t *testing.T) {
config.WithAutoInstrumentationNginxImage("nginx:2"),
config.WithEnableApacheHttpdInstrumentation(true),
config.WithEnableDotNetInstrumentation(true),
config.WithEnablePythonInstrumentation(true),
)
up := NewInstrumentationUpgrade(k8sClient, ctrl.Log.WithName("instrumentation-upgrade"), &record.FakeRecorder{}, cfg)

Expand Down

0 comments on commit 6a0b11e

Please sign in to comment.