Skip to content

Commit

Permalink
Change apache http instrumentation into command line (#2652)
Browse files Browse the repository at this point in the history
* add multiIntrumentation to command line

* add multiinstrumentation to config and add test

* change github workflow for multi-instrumentation

* rename command

* remove unwanted comments

* add chlog

* modify chlog

* add missing description in config for multi-instrumentation

* add example

* finish upgrade httpd

* remove all usage of apache-http gate

* set default apache http instrumentation to true

* edit chlog

* remove unused featuregates

* replace instrumentationupgrade initialization

* callout featuregate removal

* change apache-http constant

* make apache httpd naming consistent

* correct name in chlog

* correct name in logging setup

* Update .chloggen/apache-http-instrumentation-command-flag.yaml

Co-authored-by: Tyler Helmuth <[email protected]>

* Update apache-http-instrumentation-command-flag.yaml

---------

Co-authored-by: Tyler Helmuth <[email protected]>
Co-authored-by: Jacob Aronoff <[email protected]>
  • Loading branch information
3 people authored Feb 26, 2024
1 parent b339979 commit 449d4d7
Show file tree
Hide file tree
Showing 10 changed files with 130 additions and 88 deletions.
16 changes: 16 additions & 0 deletions .chloggen/apache-http-instrumentation-command-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. 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: remove featuregate `EnableApacheHTTPAutoInstrumentationSupport`. Use command line flag `--enable-apache-httpd-instrumentation` instead

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

# (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:
6 changes: 6 additions & 0 deletions internal/config/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ type Config struct {
collectorConfigMapEntry string
createRBACPermissions bool
enableMultiInstrumentation bool
enableApacheHttpdInstrumentation bool
autoInstrumentationDotNetImage string
autoInstrumentationGoImage string
autoInstrumentationApacheHttpdImage string
Expand Down Expand Up @@ -77,6 +78,7 @@ func New(opts ...Option) Config {
collectorConfigMapEntry: o.collectorConfigMapEntry,
createRBACPermissions: o.createRBACPermissions,
enableMultiInstrumentation: o.enableMultiInstrumentation,
enableApacheHttpdInstrumentation: o.enableApacheHttpdInstrumentation,
targetAllocatorImage: o.targetAllocatorImage,
operatorOpAMPBridgeImage: o.operatorOpAMPBridgeImage,
targetAllocatorConfigMapEntry: o.targetAllocatorConfigMapEntry,
Expand Down Expand Up @@ -116,6 +118,10 @@ func (c *Config) EnableMultiInstrumentation() bool {
return c.enableMultiInstrumentation
}

func (c *Config) EnableApacheHttpdAutoInstrumentation() bool {
return c.enableApacheHttpdInstrumentation
}

// 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 @@ -43,6 +43,7 @@ type options struct {
collectorConfigMapEntry string
createRBACPermissions bool
enableMultiInstrumentation bool
enableApacheHttpdInstrumentation bool
targetAllocatorConfigMapEntry string
operatorOpAMPBridgeConfigMapEntry string
targetAllocatorImage string
Expand Down Expand Up @@ -86,6 +87,11 @@ func WithEnableMultiInstrumentation(s bool) Option {
o.enableMultiInstrumentation = s
}
}
func WithEnableApacheHttpdInstrumentation(s bool) Option {
return func(o *options) {
o.enableApacheHttpdInstrumentation = s
}
}
func WithTargetAllocatorConfigMapEntry(s string) Option {
return func(o *options) {
o.targetAllocatorConfigMapEntry = s
Expand Down
61 changes: 30 additions & 31 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import (
"github.com/open-telemetry/opentelemetry-operator/internal/version"
"github.com/open-telemetry/opentelemetry-operator/internal/webhook/podmutation"
collectorupgrade "github.com/open-telemetry/opentelemetry-operator/pkg/collector/upgrade"
"github.com/open-telemetry/opentelemetry-operator/pkg/constants"
"github.com/open-telemetry/opentelemetry-operator/pkg/featuregate"
"github.com/open-telemetry/opentelemetry-operator/pkg/instrumentation"
instrumentationupgrade "github.com/open-telemetry/opentelemetry-operator/pkg/instrumentation/upgrade"
Expand Down Expand Up @@ -99,25 +100,26 @@ func main() {

// add flags related to this operator
var (
metricsAddr string
probeAddr string
pprofAddr string
enableLeaderElection bool
createRBACPermissions bool
enableMultiInstrumentation bool
collectorImage string
targetAllocatorImage string
operatorOpAMPBridgeImage string
autoInstrumentationJava string
autoInstrumentationNodeJS string
autoInstrumentationPython string
autoInstrumentationDotNet string
autoInstrumentationApacheHttpd string
autoInstrumentationNginx string
autoInstrumentationGo string
labelsFilter []string
webhookPort int
tlsOpt tlsConfig
metricsAddr string
probeAddr string
pprofAddr string
enableLeaderElection bool
createRBACPermissions bool
enableMultiInstrumentation bool
enableApacheHttpdInstrumentation bool
collectorImage string
targetAllocatorImage string
operatorOpAMPBridgeImage string
autoInstrumentationJava string
autoInstrumentationNodeJS string
autoInstrumentationPython string
autoInstrumentationDotNet string
autoInstrumentationApacheHttpd string
autoInstrumentationNginx string
autoInstrumentationGo string
labelsFilter []string
webhookPort int
tlsOpt tlsConfig
)

pflag.StringVar(&metricsAddr, "metrics-addr", ":8080", "The address the metric endpoint binds to.")
Expand All @@ -128,6 +130,7 @@ func main() {
"Enabling this will ensure there is only one active controller manager.")
pflag.BoolVar(&createRBACPermissions, "create-rbac-permissions", false, "Automatically create RBAC permissions needed by the processors")
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")
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 @@ -166,6 +169,7 @@ func main() {
"go-os", runtime.GOOS,
"labels-filter", labelsFilter,
"enable-multi-instrumentation", enableMultiInstrumentation,
"enable-apache-httpd-instrumentation", enableApacheHttpdInstrumentation,
)

restConfig := ctrl.GetConfigOrDie()
Expand All @@ -183,6 +187,7 @@ func main() {
config.WithCollectorImage(collectorImage),
config.WithCreateRBACPermissions(createRBACPermissions),
config.WithEnableMultiInstrumentation(enableMultiInstrumentation),
config.WithEnableApacheHttpdInstrumentation(enableApacheHttpdInstrumentation),
config.WithTargetAllocatorImage(targetAllocatorImage),
config.WithOperatorOpAMPBridgeImage(operatorOpAMPBridgeImage),
config.WithAutoInstrumentationJavaImage(autoInstrumentationJava),
Expand Down Expand Up @@ -342,18 +347,12 @@ func addDependencies(_ context.Context, mgr ctrl.Manager, cfg config.Config, v v

// adds the upgrade mechanism to be executed once the manager is ready
err = mgr.Add(manager.RunnableFunc(func(c context.Context) error {
u := &instrumentationupgrade.InstrumentationUpgrade{
Logger: ctrl.Log.WithName("instrumentation-upgrade"),
DefaultAutoInstJava: cfg.AutoInstrumentationJavaImage(),
DefaultAutoInstNodeJS: cfg.AutoInstrumentationNodeJSImage(),
DefaultAutoInstPython: cfg.AutoInstrumentationPythonImage(),
DefaultAutoInstDotNet: cfg.AutoInstrumentationDotNetImage(),
DefaultAutoInstGo: cfg.AutoInstrumentationDotNetImage(),
DefaultAutoInstApacheHttpd: cfg.AutoInstrumentationApacheHttpdImage(),
DefaultAutoInstNginx: cfg.AutoInstrumentationNginxImage(),
Client: mgr.GetClient(),
Recorder: mgr.GetEventRecorderFor("opentelemetry-operator"),
}
u := instrumentationupgrade.NewInstrumentationUpgrade(
mgr.GetClient(),
ctrl.Log.WithName("instrumentation-upgrade"),
mgr.GetEventRecorderFor("opentelemetry-operator"),
cfg,
)
return u.ManagedInstances(c)
}))
if err != nil {
Expand Down
2 changes: 2 additions & 0 deletions pkg/constants/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,6 @@ const (
EnvPodName = "OTEL_RESOURCE_ATTRIBUTES_POD_NAME"
EnvPodUID = "OTEL_RESOURCE_ATTRIBUTES_POD_UID"
EnvNodeName = "OTEL_RESOURCE_ATTRIBUTES_NODE_NAME"

FlagApacheHttpd = "enable-apache-httpd-instrumentation"
)
13 changes: 0 additions & 13 deletions pkg/featuregate/featuregate.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,25 +55,12 @@ var (
featuregate.WithRegisterDescription("controls whether the operator supports Golang auto-instrumentation"),
featuregate.WithRegisterFromVersion("v0.77.0"),
)
EnableApacheHTTPAutoInstrumentationSupport = featuregate.GlobalRegistry().MustRegister(
"operator.autoinstrumentation.apache-httpd",
featuregate.StageBeta,
featuregate.WithRegisterDescription("controls whether the operator supports Apache HTTPD auto-instrumentation"),
featuregate.WithRegisterFromVersion("v0.80.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"),
)

EnableMultiInstrumentationSupport = featuregate.GlobalRegistry().MustRegister(
"operator.autoinstrumentation.multi-instrumentation",
featuregate.StageAlpha,
featuregate.WithRegisterFromVersion("0.86.0"),
featuregate.WithRegisterDescription("controls whether the operator supports multi instrumentation"))

// 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(
Expand Down
2 changes: 1 addition & 1 deletion pkg/instrumentation/podmutator.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,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.EnableApacheHTTPAutoInstrumentationSupport.IsEnabled() || inst == nil {
if pm.config.EnableApacheHttpdAutoInstrumentation() || inst == nil {
insts.ApacheHttpd.Instrumentation = inst
} else {
logger.Error(nil, "support for Apache HTTPD auto instrumentation is not enabled")
Expand Down
16 changes: 2 additions & 14 deletions pkg/instrumentation/podmutator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2751,13 +2751,7 @@ func TestMutatePod(t *testing.T) {
},
},
},
setFeatureGates: func(t *testing.T) {
originalVal := featuregate.EnableApacheHTTPAutoInstrumentationSupport.IsEnabled()
require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableApacheHTTPAutoInstrumentationSupport.ID(), true))
t.Cleanup(func() {
require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableApacheHTTPAutoInstrumentationSupport.ID(), originalVal))
})
},
config: config.New(config.WithEnableApacheHttpdInstrumentation(true)),
},
{
name: "apache httpd injection feature gate disabled",
Expand Down Expand Up @@ -2832,13 +2826,7 @@ func TestMutatePod(t *testing.T) {
},
},
},
setFeatureGates: func(t *testing.T) {
originalVal := featuregate.EnableApacheHTTPAutoInstrumentationSupport.IsEnabled()
require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableApacheHTTPAutoInstrumentationSupport.ID(), false))
t.Cleanup(func() {
require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableApacheHTTPAutoInstrumentationSupport.ID(), originalVal))
})
},
config: config.New(config.WithEnableApacheHttpdInstrumentation(false)),
},

{
Expand Down
64 changes: 52 additions & 12 deletions pkg/instrumentation/upgrade/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,22 +25,27 @@ import (
"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.AnnotationDefaultAutoInstrumentationJava: featuregate.EnableJavaAutoInstrumentationSupport,
constants.AnnotationDefaultAutoInstrumentationNodeJS: featuregate.EnableNodeJSAutoInstrumentationSupport,
constants.AnnotationDefaultAutoInstrumentationPython: featuregate.EnablePythonAutoInstrumentationSupport,
constants.AnnotationDefaultAutoInstrumentationDotNet: featuregate.EnableDotnetAutoInstrumentationSupport,
constants.AnnotationDefaultAutoInstrumentationGo: featuregate.EnableGoAutoInstrumentationSupport,
constants.AnnotationDefaultAutoInstrumentationApacheHttpd: featuregate.EnableApacheHTTPAutoInstrumentationSupport,
constants.AnnotationDefaultAutoInstrumentationNginx: featuregate.EnableNginxAutoInstrumentationSupport,
constants.AnnotationDefaultAutoInstrumentationJava: featuregate.EnableJavaAutoInstrumentationSupport,
constants.AnnotationDefaultAutoInstrumentationNodeJS: featuregate.EnableNodeJSAutoInstrumentationSupport,
constants.AnnotationDefaultAutoInstrumentationPython: featuregate.EnablePythonAutoInstrumentationSupport,
constants.AnnotationDefaultAutoInstrumentationDotNet: featuregate.EnableDotnetAutoInstrumentationSupport,
constants.AnnotationDefaultAutoInstrumentationGo: featuregate.EnableGoAutoInstrumentationSupport,
constants.AnnotationDefaultAutoInstrumentationNginx: featuregate.EnableNginxAutoInstrumentationSupport,
}
)

type autoInstConfig struct {
id string
enabled bool
}

type InstrumentationUpgrade struct {
Client client.Client
Logger logr.Logger
Expand All @@ -52,6 +57,28 @@ type InstrumentationUpgrade struct {
DefaultAutoInstApacheHttpd string
DefaultAutoInstNginx string
DefaultAutoInstGo string
defaultAnnotationToConfig map[string]autoInstConfig
}

func NewInstrumentationUpgrade(client client.Client, logger logr.Logger, recorder record.EventRecorder, cfg config.Config) *InstrumentationUpgrade {
defaultAnnotationToConfig := map[string]autoInstConfig{
constants.AnnotationDefaultAutoInstrumentationApacheHttpd: autoInstConfig{constants.FlagApacheHttpd, cfg.EnableApacheHttpdAutoInstrumentation()},
}

return &InstrumentationUpgrade{
Client: client,
Logger: logger,
DefaultAutoInstJava: cfg.AutoInstrumentationJavaImage(),
DefaultAutoInstNodeJS: cfg.AutoInstrumentationNodeJSImage(),
DefaultAutoInstPython: cfg.AutoInstrumentationPythonImage(),
DefaultAutoInstDotNet: cfg.AutoInstrumentationDotNetImage(),
DefaultAutoInstGo: cfg.AutoInstrumentationGoImage(),
DefaultAutoInstApacheHttpd: cfg.AutoInstrumentationApacheHttpdImage(),
DefaultAutoInstNginx: cfg.AutoInstrumentationNginxImage(),
Recorder: recorder,
defaultAnnotationToConfig: defaultAnnotationToConfig,
}

}

// +kubebuilder:rbac:groups=opentelemetry.io,resources=instrumentations,verbs=get;list;watch;update;patch
Expand Down Expand Up @@ -90,6 +117,24 @@ func (u *InstrumentationUpgrade) ManagedInstances(ctx context.Context) error {

func (u *InstrumentationUpgrade) upgrade(_ context.Context, inst v1alpha1.Instrumentation) *v1alpha1.Instrumentation {
upgraded := inst.DeepCopy()
for annotation, config := range u.defaultAnnotationToConfig {
autoInst := upgraded.Annotations[annotation]
if autoInst != "" {
if config.enabled {
switch annotation {
case constants.AnnotationDefaultAutoInstrumentationApacheHttpd:
if inst.Spec.ApacheHttpd.Image == autoInst {
upgraded.Spec.ApacheHttpd.Image = u.DefaultAutoInstApacheHttpd
upgraded.Annotations[annotation] = u.DefaultAutoInstApacheHttpd
}
}
} else {
u.Logger.Error(nil, "autoinstrumentation not enabled for this language", "flag", config.id)
u.Recorder.Event(upgraded, "Warning", "InstrumentationUpgradeRejected", fmt.Sprintf("support for is not enabled for %s", config.id))
}
}
}

for annotation, gate := range defaultAnnotationToGate {
autoInst := upgraded.Annotations[annotation]
if autoInst != "" {
Expand Down Expand Up @@ -120,11 +165,6 @@ func (u *InstrumentationUpgrade) upgrade(_ context.Context, inst v1alpha1.Instru
upgraded.Spec.Go.Image = u.DefaultAutoInstGo
upgraded.Annotations[annotation] = u.DefaultAutoInstGo
}
case constants.AnnotationDefaultAutoInstrumentationApacheHttpd:
if inst.Spec.ApacheHttpd.Image == autoInst {
upgraded.Spec.ApacheHttpd.Image = u.DefaultAutoInstApacheHttpd
upgraded.Annotations[annotation] = u.DefaultAutoInstApacheHttpd
}
case constants.AnnotationDefaultAutoInstrumentationNginx:
if inst.Spec.Nginx.Image == autoInst {
upgraded.Spec.Nginx.Image = u.DefaultAutoInstNginx
Expand Down
32 changes: 15 additions & 17 deletions pkg/instrumentation/upgrade/upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/tools/record"
ctrl "sigs.k8s.io/controller-runtime"

"github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1"
"github.com/open-telemetry/opentelemetry-operator/internal/config"
Expand All @@ -40,12 +42,6 @@ func TestUpgrade(t *testing.T) {
require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableGoAutoInstrumentationSupport.ID(), originalVal))
})

originalVal = featuregate.EnableApacheHTTPAutoInstrumentationSupport.IsEnabled()
require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableApacheHTTPAutoInstrumentationSupport.ID(), true))
t.Cleanup(func() {
require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableApacheHTTPAutoInstrumentationSupport.ID(), originalVal))
})

originalVal = featuregate.EnableNginxAutoInstrumentationSupport.IsEnabled()
require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableNginxAutoInstrumentationSupport.ID(), true))
t.Cleanup(func() {
Expand Down Expand Up @@ -82,6 +78,7 @@ func TestUpgrade(t *testing.T) {
config.WithAutoInstrumentationGoImage("go:1"),
config.WithAutoInstrumentationApacheHttpdImage("apache-httpd:1"),
config.WithAutoInstrumentationNginxImage("nginx:1"),
config.WithEnableApacheHttpdInstrumentation(true),
),
).Default(context.Background(), inst)
assert.Nil(t, err)
Expand All @@ -95,17 +92,18 @@ func TestUpgrade(t *testing.T) {
err = k8sClient.Create(context.Background(), inst)
require.NoError(t, err)

up := &InstrumentationUpgrade{
Logger: logr.Discard(),
DefaultAutoInstJava: "java:2",
DefaultAutoInstNodeJS: "nodejs:2",
DefaultAutoInstPython: "python:2",
DefaultAutoInstDotNet: "dotnet:2",
DefaultAutoInstGo: "go:2",
DefaultAutoInstApacheHttpd: "apache-httpd:2",
DefaultAutoInstNginx: "nginx:2",
Client: k8sClient,
}
cfg := config.New(
config.WithAutoInstrumentationJavaImage("java:2"),
config.WithAutoInstrumentationNodeJSImage("nodejs:2"),
config.WithAutoInstrumentationPythonImage("python:2"),
config.WithAutoInstrumentationDotNetImage("dotnet:2"),
config.WithAutoInstrumentationGoImage("go:2"),
config.WithAutoInstrumentationApacheHttpdImage("apache-httpd:2"),
config.WithAutoInstrumentationNginxImage("nginx:2"),
config.WithEnableApacheHttpdInstrumentation(true),
)
up := NewInstrumentationUpgrade(k8sClient, ctrl.Log.WithName("instrumentation-upgrade"), &record.FakeRecorder{}, cfg)

err = up.ManagedInstances(context.Background())
require.NoError(t, err)

Expand Down

0 comments on commit 449d4d7

Please sign in to comment.