Skip to content

Commit

Permalink
feat: change java feature gate to cli flag (#2831)
Browse files Browse the repository at this point in the history
* feat: change java feature gate to cli flag

* fix test

* set java to default enable

* set enableJavaInstrumentation by default true

* add call to config contructor in the java tests
  • Loading branch information
dexter0195 authored Apr 16, 2024
1 parent bba5059 commit b2b0037
Show file tree
Hide file tree
Showing 10 changed files with 47 additions and 20 deletions.
16 changes: 16 additions & 0 deletions .chloggen/autoinstrumentation-java-cli-flag.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: change java instrumentation feature gate operator.autoinstrumentation.java into command line flag --enable-java-instrumentation

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

# (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:
8 changes: 8 additions & 0 deletions internal/config/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ type Config struct {
enableDotNetInstrumentation bool
enableNginxInstrumentation bool
enablePythonInstrumentation bool
enableJavaInstrumentation bool
autoInstrumentationDotNetImage string
autoInstrumentationGoImage string
autoInstrumentationApacheHttpdImage string
Expand All @@ -74,6 +75,7 @@ func New(opts ...Option) Config {
operatorOpAMPBridgeConfigMapEntry: defaultOperatorOpAMPBridgeConfigMapEntry,
logger: logf.Log.WithName("config"),
version: version.Get(),
enableJavaInstrumentation: true,
}
for _, opt := range opts {
opt(&o)
Expand All @@ -89,6 +91,7 @@ func New(opts ...Option) Config {
enableDotNetInstrumentation: o.enableDotNetInstrumentation,
enableNginxInstrumentation: o.enableNginxInstrumentation,
enablePythonInstrumentation: o.enablePythonInstrumentation,
enableJavaInstrumentation: o.enableJavaInstrumentation,
targetAllocatorImage: o.targetAllocatorImage,
operatorOpAMPBridgeImage: o.operatorOpAMPBridgeImage,
targetAllocatorConfigMapEntry: o.targetAllocatorConfigMapEntry,
Expand Down Expand Up @@ -151,6 +154,11 @@ func (c *Config) EnableNginxAutoInstrumentation() bool {
return c.enableNginxInstrumentation
}

// EnableJavaAutoInstrumentation is true when the operator supports nginx auto instrumentation.
func (c *Config) EnableJavaAutoInstrumentation() bool {
return c.enableJavaInstrumentation
}

// EnablePythonAutoInstrumentation is true when the operator supports dotnet auto instrumentation.
func (c *Config) EnablePythonAutoInstrumentation() bool {
return c.enablePythonInstrumentation
Expand Down
6 changes: 6 additions & 0 deletions internal/config/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ type options struct {
enableDotNetInstrumentation bool
enableNginxInstrumentation bool
enablePythonInstrumentation bool
enableJavaInstrumentation bool
targetAllocatorConfigMapEntry string
operatorOpAMPBridgeConfigMapEntry string
targetAllocatorImage string
Expand Down Expand Up @@ -108,6 +109,11 @@ func WithEnableNginxInstrumentation(s bool) Option {
o.enableNginxInstrumentation = s
}
}
func WithEnableJavaInstrumentation(s bool) Option {
return func(o *options) {
o.enableJavaInstrumentation = s
}
}
func WithEnablePythonInstrumentation(s bool) Option {
return func(o *options) {
o.enablePythonInstrumentation = s
Expand Down
4 changes: 4 additions & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ func main() {
enableDotNetInstrumentation bool
enablePythonInstrumentation bool
enableNginxInstrumentation bool
enableJavaInstrumentation bool
collectorImage string
targetAllocatorImage string
operatorOpAMPBridgeImage string
Expand Down Expand Up @@ -142,6 +143,7 @@ func main() {
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")
pflag.BoolVar(&enableJavaInstrumentation, constants.FlagJava, true, "Controls whether the operator supports java 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 @@ -186,6 +188,7 @@ func main() {
"enable-dotnet-instrumentation", enableDotNetInstrumentation,
"enable-python-instrumentation", enablePythonInstrumentation,
"enable-nginx-instrumentation", enableNginxInstrumentation,
"enable-java-instrumentation", enableJavaInstrumentation,
)

restConfig := ctrl.GetConfigOrDie()
Expand All @@ -207,6 +210,7 @@ func main() {
config.WithEnableDotNetInstrumentation(enableDotNetInstrumentation),
config.WithEnableNginxInstrumentation(enableNginxInstrumentation),
config.WithEnablePythonInstrumentation(enablePythonInstrumentation),
config.WithEnableJavaInstrumentation(enableJavaInstrumentation),
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 @@ -41,4 +41,5 @@ const (
FlagDotNet = "enable-dotnet-instrumentation"
FlagPython = "enable-python-instrumentation"
FlagNginx = "enable-nginx-instrumentation"
FlagJava = "enable-java-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 (
EnableJavaAutoInstrumentationSupport = featuregate.GlobalRegistry().MustRegister(
"operator.autoinstrumentation.java",
featuregate.StageBeta,
featuregate.WithRegisterDescription("controls whether the operator supports Java auto-instrumentation"),
featuregate.WithRegisterFromVersion("v0.76.1"),
)
EnableNodeJSAutoInstrumentationSupport = featuregate.GlobalRegistry().MustRegister(
"operator.autoinstrumentation.nodejs",
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 @@ -229,7 +229,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.EnableJavaAutoInstrumentationSupport.IsEnabled() || inst == nil {
if pm.config.EnableJavaAutoInstrumentation() || inst == nil {
insts.Java.Instrumentation = inst
} else {
logger.Error(nil, "support for Java auto instrumentation is not enabled")
Expand Down
10 changes: 3 additions & 7 deletions pkg/instrumentation/podmutator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ func TestMutatePod(t *testing.T) {
},
},
},
config: config.New(),
},
{
name: "javaagent injection multiple containers, true",
Expand Down Expand Up @@ -540,6 +541,7 @@ func TestMutatePod(t *testing.T) {
},
},
},
config: config.New(),
},
{
name: "javaagent injection feature gate disabled",
Expand Down Expand Up @@ -629,13 +631,7 @@ func TestMutatePod(t *testing.T) {
},
},
},
setFeatureGates: func(t *testing.T) {
originalVal := featuregate.EnableJavaAutoInstrumentationSupport.IsEnabled()
require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableJavaAutoInstrumentationSupport.ID(), false))
t.Cleanup(func() {
require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableJavaAutoInstrumentationSupport.ID(), originalVal))
})
},
config: config.New(config.WithEnableJavaInstrumentation(false)),
},
{
name: "nodejs injection, true",
Expand Down
12 changes: 6 additions & 6 deletions pkg/instrumentation/upgrade/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import (

var (
defaultAnnotationToGate = map[string]*featuregate2.Gate{
constants.AnnotationDefaultAutoInstrumentationJava: featuregate.EnableJavaAutoInstrumentationSupport,
constants.AnnotationDefaultAutoInstrumentationNodeJS: featuregate.EnableNodeJSAutoInstrumentationSupport,
constants.AnnotationDefaultAutoInstrumentationGo: featuregate.EnableGoAutoInstrumentationSupport,
}
Expand Down Expand Up @@ -63,6 +62,7 @@ func NewInstrumentationUpgrade(client client.Client, logger logr.Logger, recorde
constants.AnnotationDefaultAutoInstrumentationDotNet: {constants.FlagDotNet, cfg.EnableDotNetAutoInstrumentation()},
constants.AnnotationDefaultAutoInstrumentationNginx: {constants.FlagNginx, cfg.EnableNginxAutoInstrumentation()},
constants.AnnotationDefaultAutoInstrumentationPython: {constants.FlagPython, cfg.EnablePythonAutoInstrumentation()},
constants.AnnotationDefaultAutoInstrumentationJava: {constants.FlagJava, cfg.EnableJavaAutoInstrumentation()},
}

return &InstrumentationUpgrade{
Expand Down Expand Up @@ -142,6 +142,11 @@ func (u *InstrumentationUpgrade) upgrade(_ context.Context, inst v1alpha1.Instru
upgraded.Spec.Python.Image = u.DefaultAutoInstPython
upgraded.Annotations[annotation] = u.DefaultAutoInstPython
}
case constants.AnnotationDefaultAutoInstrumentationJava:
if inst.Spec.Java.Image == autoInst {
upgraded.Spec.Java.Image = u.DefaultAutoInstJava
upgraded.Annotations[annotation] = u.DefaultAutoInstJava
}
}
} else {
u.Logger.V(4).Info("autoinstrumentation not enabled for this language", "flag", instCfg.id)
Expand All @@ -154,11 +159,6 @@ func (u *InstrumentationUpgrade) upgrade(_ context.Context, inst v1alpha1.Instru
if autoInst != "" {
if gate.IsEnabled() {
switch annotation {
case constants.AnnotationDefaultAutoInstrumentationJava:
if inst.Spec.Java.Image == autoInst {
upgraded.Spec.Java.Image = u.DefaultAutoInstJava
upgraded.Annotations[annotation] = u.DefaultAutoInstJava
}
case constants.AnnotationDefaultAutoInstrumentationNodeJS:
if inst.Spec.NodeJS.Image == autoInst {
upgraded.Spec.NodeJS.Image = u.DefaultAutoInstNodeJS
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 @@ -76,6 +76,7 @@ func TestUpgrade(t *testing.T) {
config.WithEnableDotNetInstrumentation(true),
config.WithEnableNginxInstrumentation(true),
config.WithEnablePythonInstrumentation(true),
config.WithEnableJavaInstrumentation(true),
),
).Default(context.Background(), inst)
assert.Nil(t, err)
Expand All @@ -101,6 +102,7 @@ func TestUpgrade(t *testing.T) {
config.WithEnableDotNetInstrumentation(true),
config.WithEnableNginxInstrumentation(true),
config.WithEnablePythonInstrumentation(true),
config.WithEnableJavaInstrumentation(true),
)
up := NewInstrumentationUpgrade(k8sClient, ctrl.Log.WithName("instrumentation-upgrade"), &record.FakeRecorder{}, cfg)

Expand Down

0 comments on commit b2b0037

Please sign in to comment.