Skip to content

Commit

Permalink
chore: update dotnet instr. to command-line (open-telemetry#2690)
Browse files Browse the repository at this point in the history
* chore: update `dotnet` instr. to command-line

* fix: `.NET` pkg instrumentation upgrade

* fix: remove `debug` logs

* fix: pod mutator tests for `.NET` instrumentations

* fix: using capital N for `DotNet`
  • Loading branch information
luizhlelis authored Mar 5, 2024
1 parent d8d16a6 commit c6d8691
Show file tree
Hide file tree
Showing 10 changed files with 66 additions and 33 deletions.
16 changes: 16 additions & 0 deletions .chloggen/dotnet-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: change dotnet instrumentation feature gate into command line flag --enable-dotnet-instrumentation

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

# (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 @@ -45,6 +45,7 @@ type Config struct {
createRBACPermissions bool
enableMultiInstrumentation bool
enableApacheHttpdInstrumentation bool
enableDotNetInstrumentation bool
autoInstrumentationDotNetImage string
autoInstrumentationGoImage string
autoInstrumentationApacheHttpdImage string
Expand Down Expand Up @@ -79,6 +80,7 @@ func New(opts ...Option) Config {
createRBACPermissions: o.createRBACPermissions,
enableMultiInstrumentation: o.enableMultiInstrumentation,
enableApacheHttpdInstrumentation: o.enableApacheHttpdInstrumentation,
enableDotNetInstrumentation: o.enableDotNetInstrumentation,
targetAllocatorImage: o.targetAllocatorImage,
operatorOpAMPBridgeImage: o.operatorOpAMPBridgeImage,
targetAllocatorConfigMapEntry: o.targetAllocatorConfigMapEntry,
Expand Down Expand Up @@ -118,10 +120,16 @@ func (c *Config) EnableMultiInstrumentation() bool {
return c.enableMultiInstrumentation
}

// EnableApacheHttpdAutoInstrumentation is true when the operator supports ApacheHttpd auto instrumentation.
func (c *Config) EnableApacheHttpdAutoInstrumentation() bool {
return c.enableApacheHttpdInstrumentation
}

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

// 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 @@ -44,6 +44,7 @@ type options struct {
createRBACPermissions bool
enableMultiInstrumentation bool
enableApacheHttpdInstrumentation bool
enableDotNetInstrumentation bool
targetAllocatorConfigMapEntry string
operatorOpAMPBridgeConfigMapEntry string
targetAllocatorImage string
Expand Down Expand Up @@ -92,6 +93,11 @@ func WithEnableApacheHttpdInstrumentation(s bool) Option {
o.enableApacheHttpdInstrumentation = s
}
}
func WithEnableDotNetInstrumentation(s bool) Option {
return func(o *options) {
o.enableDotNetInstrumentation = s
}
}
func WithTargetAllocatorConfigMapEntry(s string) Option {
return func(o *options) {
o.targetAllocatorConfigMapEntry = s
Expand Down
6 changes: 5 additions & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ func main() {
createRBACPermissions bool
enableMultiInstrumentation bool
enableApacheHttpdInstrumentation bool
enableDotNetInstrumentation bool
collectorImage string
targetAllocatorImage string
operatorOpAMPBridgeImage string
Expand All @@ -132,7 +133,8 @@ 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")
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")
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 @@ -172,6 +174,7 @@ func main() {
"labels-filter", labelsFilter,
"enable-multi-instrumentation", enableMultiInstrumentation,
"enable-apache-httpd-instrumentation", enableApacheHttpdInstrumentation,
"enable-dotnet-instrumentation", enableDotNetInstrumentation,
)

restConfig := ctrl.GetConfigOrDie()
Expand All @@ -190,6 +193,7 @@ func main() {
config.WithCreateRBACPermissions(createRBACPermissions),
config.WithEnableMultiInstrumentation(enableMultiInstrumentation),
config.WithEnableApacheHttpdInstrumentation(enableApacheHttpdInstrumentation),
config.WithEnableDotNetInstrumentation(enableDotNetInstrumentation),
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 @@ -36,4 +36,5 @@ const (
EnvNodeName = "OTEL_RESOURCE_ATTRIBUTES_NODE_NAME"

FlagApacheHttpd = "enable-apache-httpd-instrumentation"
FlagDotNet = "enable-dotnet-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 (
EnableDotnetAutoInstrumentationSupport = featuregate.GlobalRegistry().MustRegister(
"operator.autoinstrumentation.dotnet",
featuregate.StageBeta,
featuregate.WithRegisterDescription("controls whether the operator supports .NET auto-instrumentation"),
featuregate.WithRegisterFromVersion("v0.76.1"),
)
EnablePythonAutoInstrumentationSupport = featuregate.GlobalRegistry().MustRegister(
"operator.autoinstrumentation.python",
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 @@ -265,7 +265,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.EnableDotnetAutoInstrumentationSupport.IsEnabled() || inst == nil {
if pm.config.EnableDotNetAutoInstrumentation() || inst == nil {
insts.DotNet.Instrumentation = inst
insts.DotNet.AdditionalAnnotations = map[string]string{annotationDotNetRuntime: annotationValue(ns.ObjectMeta, pod.ObjectMeta, annotationDotNetRuntime)}
} else {
Expand Down
38 changes: 20 additions & 18 deletions pkg/instrumentation/podmutator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1811,6 +1811,7 @@ func TestMutatePod(t *testing.T) {
},
},
},
config: config.New(config.WithEnableDotNetInstrumentation(true)),
},
{
name: "dotnet injection, by namespace annotations",
Expand Down Expand Up @@ -1990,6 +1991,7 @@ func TestMutatePod(t *testing.T) {
},
},
},
config: config.New(config.WithEnableDotNetInstrumentation(true)),
},
{
name: "dotnet injection multiple containers, true",
Expand Down Expand Up @@ -2265,6 +2267,7 @@ func TestMutatePod(t *testing.T) {
},
},
},
config: config.New(config.WithEnableDotNetInstrumentation(true)),
},
{
name: "dotnet injection feature gate disabled",
Expand Down Expand Up @@ -2343,13 +2346,7 @@ func TestMutatePod(t *testing.T) {
},
},
},
setFeatureGates: func(t *testing.T) {
originalVal := featuregate.EnableDotnetAutoInstrumentationSupport.IsEnabled()
require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableDotnetAutoInstrumentationSupport.ID(), false))
t.Cleanup(func() {
require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableDotnetAutoInstrumentationSupport.ID(), originalVal))
})
},
config: config.New(config.WithEnableDotNetInstrumentation(false)),
},
{
name: "go injection, true",
Expand Down Expand Up @@ -3845,7 +3842,10 @@ func TestMutatePod(t *testing.T) {
},
},
},
config: config.New(config.WithEnableMultiInstrumentation(true)),
config: config.New(
config.WithEnableMultiInstrumentation(true),
config.WithEnableDotNetInstrumentation(true),
),
},
{
name: "multi instrumentation for multiple containers feature gate enabled, container-names not used",
Expand Down Expand Up @@ -4503,7 +4503,10 @@ func TestMutatePod(t *testing.T) {
},
},
},
config: config.New(config.WithEnableMultiInstrumentation(true)),
config: config.New(
config.WithEnableMultiInstrumentation(true),
config.WithEnableDotNetInstrumentation(true),
),
},
{
name: "multi instrumentation for multiple containers feature gate disabled, multiple instrumentation annotations set",
Expand Down Expand Up @@ -4980,7 +4983,10 @@ func TestMutatePod(t *testing.T) {
},
},
},
config: config.New(config.WithEnableMultiInstrumentation(true)),
config: config.New(
config.WithEnableMultiInstrumentation(true),
config.WithEnableDotNetInstrumentation(true),
),
},
{
name: "multi instrumentation feature gate disabled, instrumentation feature gate disabled and annotation set, multiple specific containers set",
Expand Down Expand Up @@ -5080,14 +5086,10 @@ func TestMutatePod(t *testing.T) {
},
},
},
config: config.New(config.WithEnableMultiInstrumentation(true)),
setFeatureGates: func(t *testing.T) {
originalValDotNetInstr := featuregate.EnableDotnetAutoInstrumentationSupport.IsEnabled()
require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableDotnetAutoInstrumentationSupport.ID(), false))
t.Cleanup(func() {
require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableDotnetAutoInstrumentationSupport.ID(), originalValDotNetInstr))
})
},
config: config.New(
config.WithEnableMultiInstrumentation(true),
config.WithEnableDotNetInstrumentation(false),
),
},
}

Expand Down
14 changes: 7 additions & 7 deletions pkg/instrumentation/upgrade/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ var (
constants.AnnotationDefaultAutoInstrumentationJava: featuregate.EnableJavaAutoInstrumentationSupport,
constants.AnnotationDefaultAutoInstrumentationNodeJS: featuregate.EnableNodeJSAutoInstrumentationSupport,
constants.AnnotationDefaultAutoInstrumentationPython: featuregate.EnablePythonAutoInstrumentationSupport,
constants.AnnotationDefaultAutoInstrumentationDotNet: featuregate.EnableDotnetAutoInstrumentationSupport,
constants.AnnotationDefaultAutoInstrumentationGo: featuregate.EnableGoAutoInstrumentationSupport,
constants.AnnotationDefaultAutoInstrumentationNginx: featuregate.EnableNginxAutoInstrumentationSupport,
}
Expand All @@ -62,7 +61,8 @@ type InstrumentationUpgrade struct {

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()},
constants.AnnotationDefaultAutoInstrumentationApacheHttpd: {constants.FlagApacheHttpd, cfg.EnableApacheHttpdAutoInstrumentation()},
constants.AnnotationDefaultAutoInstrumentationDotNet: {constants.FlagDotNet, cfg.EnableDotNetAutoInstrumentation()},
}

return &InstrumentationUpgrade{
Expand Down Expand Up @@ -127,6 +127,11 @@ func (u *InstrumentationUpgrade) upgrade(_ context.Context, inst v1alpha1.Instru
upgraded.Spec.ApacheHttpd.Image = u.DefaultAutoInstApacheHttpd
upgraded.Annotations[annotation] = u.DefaultAutoInstApacheHttpd
}
case constants.AnnotationDefaultAutoInstrumentationDotNet:
if inst.Spec.DotNet.Image == autoInst {
upgraded.Spec.DotNet.Image = u.DefaultAutoInstDotNet
upgraded.Annotations[annotation] = u.DefaultAutoInstDotNet
}
}
} else {
u.Logger.Error(nil, "autoinstrumentation not enabled for this language", "flag", config.id)
Expand Down Expand Up @@ -155,11 +160,6 @@ func (u *InstrumentationUpgrade) upgrade(_ context.Context, inst v1alpha1.Instru
upgraded.Spec.Python.Image = u.DefaultAutoInstPython
upgraded.Annotations[annotation] = u.DefaultAutoInstPython
}
case constants.AnnotationDefaultAutoInstrumentationDotNet:
if inst.Spec.DotNet.Image == autoInst {
upgraded.Spec.DotNet.Image = u.DefaultAutoInstDotNet
upgraded.Annotations[annotation] = u.DefaultAutoInstDotNet
}
case constants.AnnotationDefaultAutoInstrumentationGo:
if inst.Spec.Go.Image == autoInst {
upgraded.Spec.Go.Image = u.DefaultAutoInstGo
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 @@ -79,6 +79,7 @@ func TestUpgrade(t *testing.T) {
config.WithAutoInstrumentationApacheHttpdImage("apache-httpd:1"),
config.WithAutoInstrumentationNginxImage("nginx:1"),
config.WithEnableApacheHttpdInstrumentation(true),
config.WithEnableDotNetInstrumentation(true),
),
).Default(context.Background(), inst)
assert.Nil(t, err)
Expand All @@ -101,6 +102,7 @@ func TestUpgrade(t *testing.T) {
config.WithAutoInstrumentationApacheHttpdImage("apache-httpd:2"),
config.WithAutoInstrumentationNginxImage("nginx:2"),
config.WithEnableApacheHttpdInstrumentation(true),
config.WithEnableDotNetInstrumentation(true),
)
up := NewInstrumentationUpgrade(k8sClient, ctrl.Log.WithName("instrumentation-upgrade"), &record.FakeRecorder{}, cfg)

Expand Down

0 comments on commit c6d8691

Please sign in to comment.