Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NodeJS instrumentation featuregates into cli #2834

Merged
16 changes: 16 additions & 0 deletions .chloggen/change-instrumentation-feature-gates.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.nodejs`. Use command line flag `--enable-nodejs-instrumentation` instead

# One or more tracking issues related to the change
issues: [2674]

# (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:
4 changes: 2 additions & 2 deletions .github/workflows/e2e.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ jobs:
- e2e-metadata-filters
include:
- group: e2e-instrumentation
setup: "add-operator-arg OPERATOR_ARG=--enable-go-instrumentation prepare-e2e"
setup: "add-instrumentation-params prepare-e2e"
- group: e2e-multi-instrumentation
setup: "add-operator-arg OPERATOR_ARG=--enable-multi-instrumentation prepare-e2e"
setup: "add-multi-instrumentation-params prepare-e2e"
- group: e2e-metadata-filters
setup: "add-operator-arg OPERATOR_ARG='--annotations-filter=*filter.out --labels=*filter.out' prepare-e2e"

Expand Down
8 changes: 8 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,14 @@ add-operator-arg: manifests kustomize
add-image-targetallocator:
@$(MAKE) add-operator-arg OPERATOR_ARG=--target-allocator-image=$(TARGETALLOCATOR_IMG)

.PHONY: add-instrumentation-params
add-instrumentation-params:
@$(MAKE) add-operator-arg OPERATOR_ARG=--enable-go-instrumentation=true

.PHONY: add-multi-instrumentation-params
add-multi-instrumentation-params:
@$(MAKE) add-operator-arg OPERATOR_ARG=--enable-multi-instrumentation

.PHONY: add-image-opampbridge
add-image-opampbridge:
@$(MAKE) add-operator-arg OPERATOR_ARG=--operator-opamp-bridge-image=$(OPERATOROPAMPBRIDGE_IMG)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,6 @@ spec:
- --enable-leader-election
- --zap-log-level=info
- --zap-time-encoding=rfc3339nano
- --feature-gates=+operator.autoinstrumentation.go
- --enable-nginx-instrumentation=true
image: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator:0.98.0
livenessProbe:
Expand Down
1 change: 0 additions & 1 deletion config/manager/manager.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ spec:
- "--enable-leader-election"
- "--zap-log-level=info"
- "--zap-time-encoding=rfc3339nano"
- "--feature-gates=+operator.autoinstrumentation.go"
- "--enable-nginx-instrumentation=true"
image: controller
name: manager
Expand Down
7 changes: 7 additions & 0 deletions internal/config/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ type Config struct {
enableGoInstrumentation bool
enableNginxInstrumentation bool
enablePythonInstrumentation bool
enableNodeJSInstrumentation bool
enableJavaInstrumentation bool
autoInstrumentationDotNetImage string
autoInstrumentationGoImage string
Expand Down Expand Up @@ -93,6 +94,7 @@ func New(opts ...Option) Config {
enableGoInstrumentation: o.enableGoInstrumentation,
enableNginxInstrumentation: o.enableNginxInstrumentation,
enablePythonInstrumentation: o.enablePythonInstrumentation,
enableNodeJSInstrumentation: o.enableNodeJSInstrumentation,
enableJavaInstrumentation: o.enableJavaInstrumentation,
targetAllocatorImage: o.targetAllocatorImage,
operatorOpAMPBridgeImage: o.operatorOpAMPBridgeImage,
Expand Down Expand Up @@ -171,6 +173,11 @@ func (c *Config) EnablePythonAutoInstrumentation() bool {
return c.enablePythonInstrumentation
}

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

// 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 @@ -49,6 +49,7 @@ type options struct {
enableGoInstrumentation bool
enableNginxInstrumentation bool
enablePythonInstrumentation bool
enableNodeJSInstrumentation bool
enableJavaInstrumentation bool
targetAllocatorConfigMapEntry string
operatorOpAMPBridgeConfigMapEntry string
Expand Down Expand Up @@ -125,6 +126,11 @@ func WithEnablePythonInstrumentation(s bool) Option {
o.enablePythonInstrumentation = s
}
}
func WithEnableNodeJSInstrumentation(s bool) Option {
return func(o *options) {
o.enableNodeJSInstrumentation = 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 @@ -115,6 +115,7 @@ func main() {
enableGoInstrumentation bool
enablePythonInstrumentation bool
enableNginxInstrumentation bool
enableNodeJSInstrumentation bool
enableJavaInstrumentation bool
collectorImage string
targetAllocatorImage string
Expand Down Expand Up @@ -145,6 +146,7 @@ func main() {
pflag.BoolVar(&enableGoInstrumentation, constants.FlagGo, false, "Controls whether the operator supports Go 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(&enableNodeJSInstrumentation, constants.FlagNodeJS, true, "Controls whether the operator supports nodejs 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.")
Expand Down Expand Up @@ -191,6 +193,7 @@ func main() {
"enable-go-instrumentation", enableGoInstrumentation,
"enable-python-instrumentation", enablePythonInstrumentation,
"enable-nginx-instrumentation", enableNginxInstrumentation,
"enable-nodejs-instrumentation", enableNodeJSInstrumentation,
"enable-java-instrumentation", enableJavaInstrumentation,
)

Expand All @@ -214,6 +217,7 @@ func main() {
config.WithEnableGoInstrumentation(enableGoInstrumentation),
config.WithEnableNginxInstrumentation(enableNginxInstrumentation),
config.WithEnablePythonInstrumentation(enablePythonInstrumentation),
config.WithEnableNodeJSInstrumentation(enableNodeJSInstrumentation),
config.WithEnableJavaInstrumentation(enableJavaInstrumentation),
config.WithTargetAllocatorImage(targetAllocatorImage),
config.WithOperatorOpAMPBridgeImage(operatorOpAMPBridgeImage),
Expand Down
1 change: 1 addition & 0 deletions pkg/constants/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,6 @@ const (
FlagGo = "enable-go-instrumentation"
FlagPython = "enable-python-instrumentation"
FlagNginx = "enable-nginx-instrumentation"
FlagNodeJS = "enable-nodejs-instrumentation"
FlagJava = "enable-java-instrumentation"
)
19 changes: 0 additions & 19 deletions pkg/featuregate/featuregate.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,25 +25,6 @@ const (
)

var (
EnableNodeJSAutoInstrumentationSupport = featuregate.GlobalRegistry().MustRegister(
"operator.autoinstrumentation.nodejs",
featuregate.StageBeta,
featuregate.WithRegisterDescription("controls whether the operator supports NodeJS auto-instrumentation"),
featuregate.WithRegisterFromVersion("v0.76.1"),
)
EnableNginxAutoInstrumentationSupport = featuregate.GlobalRegistry().MustRegister(
"operator.autoinstrumentation.nginx",
featuregate.StageAlpha,
featuregate.WithRegisterDescription("controls whether the operator supports Nginx auto-instrumentation"),
featuregate.WithRegisterFromVersion("v0.86.0"),
)
EnableGoAutoInstrumentationSupport = featuregate.GlobalRegistry().MustRegister(
"operator.autoinstrumentation.go",
featuregate.StageAlpha,
featuregate.WithRegisterDescription("controls whether the operator supports Golang auto-instrumentation"),
featuregate.WithRegisterFromVersion("v0.77.0"),
)

// PrometheusOperatorIsAvailable is the feature gate that enables features associated to the Prometheus Operator.
PrometheusOperatorIsAvailable = featuregate.GlobalRegistry().MustRegister(
"operator.observability.prometheus",
Expand Down
3 changes: 1 addition & 2 deletions pkg/instrumentation/podmutator.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import (
"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1"
"github.com/open-telemetry/opentelemetry-operator/internal/config"
"github.com/open-telemetry/opentelemetry-operator/internal/webhook/podmutation"
"github.com/open-telemetry/opentelemetry-operator/pkg/featuregate"
)

var (
Expand Down Expand Up @@ -241,7 +240,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.EnableNodeJSAutoInstrumentationSupport.IsEnabled() || inst == nil {
if pm.config.EnableNodeJSAutoInstrumentation() || inst == nil {
insts.NodeJS.Instrumentation = inst
} else {
logger.Error(nil, "support for NodeJS auto instrumentation is not enabled")
Expand Down
18 changes: 7 additions & 11 deletions pkg/instrumentation/podmutator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,12 @@ import (
"github.com/go-logr/logr"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
colfeaturegate "go.opentelemetry.io/collector/featuregate"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/tools/record"

"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1"
"github.com/open-telemetry/opentelemetry-operator/internal/config"
"github.com/open-telemetry/opentelemetry-operator/pkg/featuregate"
)

func TestMutatePod(t *testing.T) {
Expand Down Expand Up @@ -815,6 +813,7 @@ func TestMutatePod(t *testing.T) {
},
},
},
config: config.New(config.WithEnableNodeJSInstrumentation(true)),
},
{
name: "nodejs injection multiple containers, true",
Expand Down Expand Up @@ -1086,6 +1085,7 @@ func TestMutatePod(t *testing.T) {
},
},
},
config: config.New(config.WithEnableNodeJSInstrumentation(true)),
},
{
name: "nodejs injection feature gate disabled",
Expand Down Expand Up @@ -1168,13 +1168,6 @@ func TestMutatePod(t *testing.T) {
},
},
},
setFeatureGates: func(t *testing.T) {
originalVal := featuregate.EnableNodeJSAutoInstrumentationSupport.IsEnabled()
require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableNodeJSAutoInstrumentationSupport.ID(), false))
t.Cleanup(func() {
require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableNodeJSAutoInstrumentationSupport.ID(), originalVal))
})
},
},
{
name: "python injection, true",
Expand Down Expand Up @@ -4201,6 +4194,7 @@ func TestMutatePod(t *testing.T) {
config.WithEnableMultiInstrumentation(true),
config.WithEnablePythonInstrumentation(true),
config.WithEnableDotNetInstrumentation(true),
config.WithEnableNodeJSInstrumentation(true),
),
},
{
Expand Down Expand Up @@ -4991,6 +4985,7 @@ func TestMutatePod(t *testing.T) {
config.WithEnableMultiInstrumentation(true),
config.WithEnableDotNetInstrumentation(true),
config.WithEnablePythonInstrumentation(true),
config.WithEnableNodeJSInstrumentation(true),
),
},
{
Expand Down Expand Up @@ -5145,7 +5140,7 @@ func TestMutatePod(t *testing.T) {
},
},
},
config: config.New(config.WithEnableMultiInstrumentation(false)),
config: config.New(config.WithEnableMultiInstrumentation(false), config.WithEnableJavaInstrumentation(false)),
},
{
name: "multi instrumentation feature gate enabled, multiple instrumentation annotations set, no containers",
Expand Down Expand Up @@ -5289,7 +5284,7 @@ func TestMutatePod(t *testing.T) {
},
},
},
config: config.New(config.WithEnableMultiInstrumentation(true)),
config: config.New(config.WithEnableMultiInstrumentation(true), config.WithEnableJavaInstrumentation(false)),
},
{
name: "multi instrumentation feature gate enabled, single instrumentation annotation set, no containers",
Expand Down Expand Up @@ -5590,6 +5585,7 @@ func TestMutatePod(t *testing.T) {
config: config.New(
config.WithEnableMultiInstrumentation(true),
config.WithEnableDotNetInstrumentation(false),
config.WithEnableNodeJSInstrumentation(false),
),
},
}
Expand Down
41 changes: 7 additions & 34 deletions pkg/instrumentation/upgrade/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,12 @@ import (
"reflect"

"github.com/go-logr/logr"
featuregate2 "go.opentelemetry.io/collector/featuregate"
"k8s.io/client-go/tools/record"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1"
"github.com/open-telemetry/opentelemetry-operator/internal/config"
"github.com/open-telemetry/opentelemetry-operator/pkg/constants"
"github.com/open-telemetry/opentelemetry-operator/pkg/featuregate"
)

var (
defaultAnnotationToGate = map[string]*featuregate2.Gate{
constants.AnnotationDefaultAutoInstrumentationNodeJS: featuregate.EnableNodeJSAutoInstrumentationSupport,
}
)

type autoInstConfig struct {
Expand Down Expand Up @@ -62,6 +54,7 @@ func NewInstrumentationUpgrade(client client.Client, logger logr.Logger, recorde
constants.AnnotationDefaultAutoInstrumentationGo: {constants.FlagGo, cfg.EnableGoAutoInstrumentation()},
constants.AnnotationDefaultAutoInstrumentationNginx: {constants.FlagNginx, cfg.EnableNginxAutoInstrumentation()},
constants.AnnotationDefaultAutoInstrumentationPython: {constants.FlagPython, cfg.EnablePythonAutoInstrumentation()},
constants.AnnotationDefaultAutoInstrumentationNodeJS: {constants.FlagNodeJS, cfg.EnableNodeJSAutoInstrumentation()},
constants.AnnotationDefaultAutoInstrumentationJava: {constants.FlagJava, cfg.EnableJavaAutoInstrumentation()},
}

Expand Down Expand Up @@ -147,43 +140,23 @@ func (u *InstrumentationUpgrade) upgrade(_ context.Context, inst v1alpha1.Instru
upgraded.Spec.Python.Image = u.DefaultAutoInstPython
upgraded.Annotations[annotation] = u.DefaultAutoInstPython
}
case constants.AnnotationDefaultAutoInstrumentationNodeJS:
if inst.Spec.NodeJS.Image == autoInst {
upgraded.Spec.NodeJS.Image = u.DefaultAutoInstNodeJS
upgraded.Annotations[annotation] = u.DefaultAutoInstNodeJS
}
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)
}
}
}

for annotation, gate := range defaultAnnotationToGate {
autoInst := upgraded.Annotations[annotation]
if autoInst != "" {
if gate.IsEnabled() {
switch annotation {
case constants.AnnotationDefaultAutoInstrumentationNodeJS:
if inst.Spec.NodeJS.Image == autoInst {
upgraded.Spec.NodeJS.Image = u.DefaultAutoInstNodeJS
upgraded.Annotations[annotation] = u.DefaultAutoInstNodeJS
}
case constants.AnnotationDefaultAutoInstrumentationPython:
if inst.Spec.Python.Image == autoInst {
upgraded.Spec.Python.Image = u.DefaultAutoInstPython
upgraded.Annotations[annotation] = u.DefaultAutoInstPython
}
case constants.AnnotationDefaultAutoInstrumentationGo:
if inst.Spec.Go.Image == autoInst {
upgraded.Spec.Go.Image = u.DefaultAutoInstGo
upgraded.Annotations[annotation] = u.DefaultAutoInstGo
}
}
} else {
u.Logger.V(4).Info("autoinstrumentation not enabled for this language", "flag", gate.ID())
}
}
}
return upgraded
}
2 changes: 2 additions & 0 deletions pkg/instrumentation/upgrade/upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ func TestUpgrade(t *testing.T) {
config.WithEnableGoInstrumentation(true),
config.WithEnableNginxInstrumentation(true),
config.WithEnablePythonInstrumentation(true),
config.WithEnableNodeJSInstrumentation(true),
config.WithEnableJavaInstrumentation(true),
),
).Default(context.Background(), inst)
Expand Down Expand Up @@ -96,6 +97,7 @@ func TestUpgrade(t *testing.T) {
config.WithEnableGoInstrumentation(true),
config.WithEnableNginxInstrumentation(true),
config.WithEnablePythonInstrumentation(true),
config.WithEnableNodeJSInstrumentation(true),
config.WithEnableJavaInstrumentation(true),
)
up := NewInstrumentationUpgrade(k8sClient, ctrl.Log.WithName("instrumentation-upgrade"), &record.FakeRecorder{}, cfg)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8317,7 +8317,6 @@ spec:
- --enable-leader-election
- --zap-log-level=info
- --zap-time-encoding=rfc3339nano
- --feature-gates=+operator.autoinstrumentation.go
image: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator:0.86.0
livenessProbe:
httpGet:
Expand Down
Loading