diff --git a/cmd/otel-agent/subcommands/run/command.go b/cmd/otel-agent/subcommands/run/command.go index 08c1472998157..d7958c6dabc59 100644 --- a/cmd/otel-agent/subcommands/run/command.go +++ b/cmd/otel-agent/subcommands/run/command.go @@ -112,15 +112,15 @@ func runOTelAgentCommand(ctx context.Context, params *subcommands.GlobalParams, }), logfx.Module(), fetchonlyimpl.Module(), - // TODO: don't rely on this pattern; remove this `OptionalModuleWithParams` thing + // TODO: don't rely on this pattern; remove this `ModuleWithParams` thing // and instead adapt OptionalModule to allow parameter passing naturally. // See: https://github.com/DataDog/datadog-agent/pull/28386 - configsyncimpl.OptionalModuleWithParams(), + configsyncimpl.ModuleWithParams(), fx.Provide(func() configsyncimpl.Params { return configsyncimpl.NewParams(params.SyncTimeout, params.SyncDelay, true) }), converterfx.Module(), - fx.Provide(func(cp converter.Component, _ optional.Option[configsync.Component]) confmap.Converter { + fx.Provide(func(cp converter.Component, _ configsync.Component) confmap.Converter { return cp }), collectorcontribFx.Module(), @@ -191,10 +191,10 @@ func runOTelAgentCommand(ctx context.Context, params *subcommands.GlobalParams, fx.Invoke(func(_ collectordef.Component, _ defaultforwarder.Forwarder, _ optional.Option[logsagentpipeline.Component]) { }), - // TODO: don't rely on this pattern; remove this `OptionalModuleWithParams` thing + // TODO: don't rely on this pattern; remove this `ModuleWithParams` thing // and instead adapt OptionalModule to allow parameter passing naturally. // See: https://github.com/DataDog/datadog-agent/pull/28386 - configsyncimpl.OptionalModuleWithParams(), + configsyncimpl.ModuleWithParams(), fx.Provide(func() configsyncimpl.Params { return configsyncimpl.NewParams(params.SyncTimeout, params.SyncDelay, true) }), @@ -218,7 +218,7 @@ func runOTelAgentCommand(ctx context.Context, params *subcommands.GlobalParams, // TODO: consider adding configsync.Component as an explicit dependency for traceconfig // to avoid this sort of dependency tree hack. - fx.Provide(func(deps traceconfig.Dependencies, _ optional.Option[configsync.Component]) (traceconfig.Component, error) { + fx.Provide(func(deps traceconfig.Dependencies, _ configsync.Component) (traceconfig.Component, error) { // TODO: this would be much better if we could leverage traceconfig.Module // Must add a new parameter to traconfig.Module to handle this. return traceconfig.NewConfig(deps) @@ -236,13 +236,13 @@ func runOTelAgentCommand(ctx context.Context, params *subcommands.GlobalParams, // ForwarderBundle returns the fx.Option for the forwarder bundle. // TODO: cleanup the forwarder instantiation with fx. -// This is a bit of a hack because we need to enforce optional.Option[configsync.Component] +// This is a bit of a hack because we need to enforce configsync.Component // is passed to newForwarder to enforce the correct instantiation order. Currently, the // new forwarder.BundleWithProvider makes a few assumptions in its generic prototype, and // this is the current workaround to leverage it. func ForwarderBundle() fx.Option { return defaultforwarder.ModulWithOptionTMP( - fx.Provide(func(_ optional.Option[configsync.Component]) defaultforwarder.Params { + fx.Provide(func(_ configsync.Component) defaultforwarder.Params { return defaultforwarder.NewParams() })) } diff --git a/cmd/process-agent/command/main_common.go b/cmd/process-agent/command/main_common.go index dcb9287e74f04..4d6b33d5ad831 100644 --- a/cmd/process-agent/command/main_common.go +++ b/cmd/process-agent/command/main_common.go @@ -65,7 +65,6 @@ import ( "github.com/DataDog/datadog-agent/pkg/util/fxutil" "github.com/DataDog/datadog-agent/pkg/util/fxutil/logging" "github.com/DataDog/datadog-agent/pkg/util/log" - "github.com/DataDog/datadog-agent/pkg/util/optional" "github.com/DataDog/datadog-agent/pkg/version" ) @@ -156,7 +155,7 @@ func runApp(ctx context.Context, globalParams *GlobalParams) error { fetchonlyimpl.Module(), // Provide configsync module - configsyncimpl.OptionalModule(), + configsyncimpl.Module(), // Provide autoexit module autoexitimpl.Module(), @@ -215,7 +214,7 @@ func runApp(ctx context.Context, globalParams *GlobalParams) error { _ expvars.Component, _ apiserver.Component, cfg config.Component, - _ optional.Option[configsync.Component], + _ configsync.Component, // TODO: This is needed by the container-provider which is not currently a component. // We should ensure the tagger is a dependency when converting to a component. _ tagger.Component, diff --git a/cmd/security-agent/main_windows.go b/cmd/security-agent/main_windows.go index a729e7a2003f0..f6a2e3aedf28f 100644 --- a/cmd/security-agent/main_windows.go +++ b/cmd/security-agent/main_windows.go @@ -55,7 +55,6 @@ import ( "github.com/DataDog/datadog-agent/pkg/util/defaultpaths" "github.com/DataDog/datadog-agent/pkg/util/fxutil" - "github.com/DataDog/datadog-agent/pkg/util/optional" "github.com/DataDog/datadog-agent/pkg/util/startstop" "github.com/DataDog/datadog-agent/pkg/util/winutil/servicemain" ) @@ -166,9 +165,9 @@ func (s *service) Run(svcctx context.Context) error { statusimpl.Module(), fetchonlyimpl.Module(), - configsyncimpl.OptionalModule(), + configsyncimpl.Module(), // Force the instantiation of the component - fx.Invoke(func(_ optional.Option[configsync.Component]) {}), + fx.Invoke(func(_ configsync.Component) {}), autoexitimpl.Module(), fx.Provide(func(c config.Component) settings.Params { return settings.Params{ diff --git a/cmd/security-agent/subcommands/start/command.go b/cmd/security-agent/subcommands/start/command.go index 2a3dd883dcd84..933a44f4a0135 100644 --- a/cmd/security-agent/subcommands/start/command.go +++ b/cmd/security-agent/subcommands/start/command.go @@ -66,7 +66,6 @@ import ( "github.com/DataDog/datadog-agent/pkg/status/health" "github.com/DataDog/datadog-agent/pkg/util/coredump" "github.com/DataDog/datadog-agent/pkg/util/fxutil" - "github.com/DataDog/datadog-agent/pkg/util/optional" "github.com/DataDog/datadog-agent/pkg/util/profiling" "github.com/DataDog/datadog-agent/pkg/util/startstop" "github.com/DataDog/datadog-agent/pkg/version" @@ -174,9 +173,9 @@ func Commands(globalParams *command.GlobalParams) []*cobra.Command { }), statusimpl.Module(), fetchonlyimpl.Module(), - configsyncimpl.OptionalModule(), + configsyncimpl.Module(), // Force the instantiation of the component - fx.Invoke(func(_ optional.Option[configsync.Component]) {}), + fx.Invoke(func(_ configsync.Component) {}), autoexitimpl.Module(), fx.Supply(pidimpl.NewParams(params.pidfilePath)), fx.Provide(func(c config.Component) settings.Params { diff --git a/cmd/trace-agent/subcommands/run/command.go b/cmd/trace-agent/subcommands/run/command.go index 1d0788501ffae..4526101362de1 100644 --- a/cmd/trace-agent/subcommands/run/command.go +++ b/cmd/trace-agent/subcommands/run/command.go @@ -113,9 +113,9 @@ func runTraceAgentProcess(ctx context.Context, cliParams *Params, defaultConfPat zstdfx.Module(), trace.Bundle(), fetchonlyimpl.Module(), - configsyncimpl.OptionalModule(), + configsyncimpl.Module(), // Force the instantiation of the components - fx.Invoke(func(_ traceagent.Component, _ optional.Option[configsync.Component], _ autoexit.Component) {}), + fx.Invoke(func(_ traceagent.Component, _ configsync.Component, _ autoexit.Component) {}), ) if err != nil && errors.Is(err, traceagentimpl.ErrAgentDisabled) { return nil diff --git a/comp/core/configsync/configsyncimpl/module.go b/comp/core/configsync/configsyncimpl/module.go index 0b77c1370de20..a89bbacc91197 100644 --- a/comp/core/configsync/configsyncimpl/module.go +++ b/comp/core/configsync/configsyncimpl/module.go @@ -22,7 +22,6 @@ import ( log "github.com/DataDog/datadog-agent/comp/core/log/def" apiutil "github.com/DataDog/datadog-agent/pkg/api/util" "github.com/DataDog/datadog-agent/pkg/util/fxutil" - "github.com/DataDog/datadog-agent/pkg/util/optional" ) type dependencies struct { @@ -35,20 +34,20 @@ type dependencies struct { SyncParams Params } -// OptionalModule defines the fx options for this component. -func OptionalModule() fxutil.Module { +// Module defines the fx options for this component. +func Module() fxutil.Module { return fxutil.Component( - fx.Provide(newOptionalConfigSync), + fx.Provide(newComponent), fx.Supply(Params{}), ) } -// OptionalModuleWithParams defines the fx options for this component, but +// ModuleWithParams defines the fx options for this component, but // requires additionally specifying custom Params from the fx App, to be // passed to the constructor. -func OptionalModuleWithParams() fxutil.Module { +func ModuleWithParams() fxutil.Module { return fxutil.Component( - fx.Provide(newOptionalConfigSync), + fx.Provide(newComponent), ) } @@ -61,19 +60,21 @@ type configSync struct { client *http.Client connected bool ctx context.Context + enabled bool } -// newOptionalConfigSync checks if the component was enabled as per the config, and returns an optional.Option -func newOptionalConfigSync(deps dependencies) optional.Option[configsync.Component] { +// newComponent checks if the component was enabled as per the config and return a enable/disabled configsync +func newComponent(deps dependencies) configsync.Component { agentIPCPort := deps.Config.GetInt("agent_ipc.port") configRefreshIntervalSec := deps.Config.GetInt("agent_ipc.config_refresh_interval") if agentIPCPort <= 0 || configRefreshIntervalSec <= 0 { - return optional.NewNoneOption[configsync.Component]() + deps.Log.Infof("configsync disabled (agent_ipc.port: %d | agent_ipc.config_refresh_interval: %d)", agentIPCPort, configRefreshIntervalSec) + return configSync{} } - configSync := newConfigSync(deps, agentIPCPort, configRefreshIntervalSec) - return optional.NewOption(configSync) + deps.Log.Infof("configsync enabled (agent_ipc '%s:%d' | agent_ipc.config_refresh_interval: %d)", deps.Config.GetString("agent_ipc.host"), agentIPCPort, configRefreshIntervalSec) + return newConfigSync(deps, agentIPCPort, configRefreshIntervalSec) } // newConfigSync creates a new configSync component. @@ -98,6 +99,7 @@ func newConfigSync(deps dependencies, agentIPCPort int, configRefreshIntervalSec url: url, client: client, ctx: ctx, + enabled: true, } if deps.SyncParams.OnInit { diff --git a/comp/core/configsync/configsyncimpl/module_integration_test.go b/comp/core/configsync/configsyncimpl/module_integration_test.go index e4cfa8ec1ba63..5d0fde62a449a 100644 --- a/comp/core/configsync/configsyncimpl/module_integration_test.go +++ b/comp/core/configsync/configsyncimpl/module_integration_test.go @@ -22,7 +22,6 @@ import ( "github.com/DataDog/datadog-agent/comp/core/config" "github.com/DataDog/datadog-agent/comp/core/configsync" "github.com/DataDog/datadog-agent/pkg/util/fxutil" - "github.com/DataDog/datadog-agent/pkg/util/optional" ) func TestOptionalModule(t *testing.T) { @@ -44,16 +43,14 @@ func TestOptionalModule(t *testing.T) { "agent_ipc.port": port, "agent_ipc.config_refresh_interval": 1, } - csopt := fxutil.Test[optional.Option[configsync.Component]](t, fx.Options( + comp := fxutil.Test[configsync.Component](t, fx.Options( core.MockBundle(), fetchonlyimpl.Module(), - OptionalModule(), + Module(), fx.Populate(&cfg), fx.Replace(config.MockParams{Overrides: overrides}), )) - - _, ok := csopt.Get() - require.True(t, ok) + require.True(t, comp.(configSync).enabled) require.EventuallyWithT(t, func(t *assert.CollectT) { assert.Equal(t, "value1", cfg.Get("key1")) diff --git a/comp/core/configsync/configsyncimpl/module_test.go b/comp/core/configsync/configsyncimpl/module_test.go index 076288737a65e..8051e4cce793c 100644 --- a/comp/core/configsync/configsyncimpl/module_test.go +++ b/comp/core/configsync/configsyncimpl/module_test.go @@ -8,34 +8,30 @@ package configsyncimpl import ( "testing" - "github.com/stretchr/testify/require" - pkgconfigmodel "github.com/DataDog/datadog-agent/pkg/config/model" + "github.com/stretchr/testify/assert" ) -func TestNewOptionalConfigSync(t *testing.T) { +func TestNewConfigSync(t *testing.T) { t.Run("enabled", func(t *testing.T) { deps := makeDeps(t) deps.Config.Set("agent_ipc.port", 1234, pkgconfigmodel.SourceFile) deps.Config.Set("agent_ipc.config_refresh_interval", 30, pkgconfigmodel.SourceFile) - optConfigSync := newOptionalConfigSync(deps) - _, ok := optConfigSync.Get() - require.True(t, ok) + comp := newComponent(deps) + assert.True(t, comp.(configSync).enabled) }) t.Run("disabled ipc port zero", func(t *testing.T) { deps := makeDeps(t) deps.Config.Set("agent_ipc.port", 0, pkgconfigmodel.SourceFile) - optConfigSync := newOptionalConfigSync(deps) - _, ok := optConfigSync.Get() - require.False(t, ok) + comp := newComponent(deps) + assert.False(t, comp.(configSync).enabled) }) t.Run("disabled config refresh interval zero", func(t *testing.T) { deps := makeDeps(t) deps.Config.Set("agent_ipc.config_refresh_interval", 0, pkgconfigmodel.SourceFile) - optConfigSync := newOptionalConfigSync(deps) - _, ok := optConfigSync.Get() - require.False(t, ok) + comp := newComponent(deps) + assert.False(t, comp.(configSync).enabled) }) }