From a2308e0baec0813cfbda57e661be97e239910b6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Fri, 24 Nov 2023 09:59:00 +0100 Subject: [PATCH] cmd/trace-agent/subcommands/run: fix file path for trace-agent logger (#21068) * comp/core: introduce support for mocking the trace logger * comp/trace: test that default log file path for trace-agent is properly set * cmd/trace-agent/command: use defined constant for logger name * cmd/trace-agent/subcommands/run: set the correct file path for trace logger * comp/core: simplify MockBundle params --- cmd/trace-agent/command/command.go | 2 +- cmd/trace-agent/subcommands/run/command.go | 6 ++---- comp/core/bundle_mock.go | 22 +++++++++++++++------- comp/core/log/component.go | 5 +++++ comp/core/log/mock.go | 12 ++++++++++++ comp/trace/bundle_test.go | 15 +++++++++++++-- 6 files changed, 48 insertions(+), 14 deletions(-) diff --git a/cmd/trace-agent/command/command.go b/cmd/trace-agent/command/command.go index 8bac0c9efc890..c30eedf5a924f 100644 --- a/cmd/trace-agent/command/command.go +++ b/cmd/trace-agent/command/command.go @@ -37,7 +37,7 @@ func makeCommands(globalParams *subcommands.GlobalParams) *cobra.Command { return &subcommands.GlobalParams{ ConfPath: globalParams.ConfPath, ConfigName: globalParams.ConfigName, - LoggerName: "TRACE", + LoggerName: LoggerName, } } commands := []*cobra.Command{ diff --git a/cmd/trace-agent/subcommands/run/command.go b/cmd/trace-agent/subcommands/run/command.go index 3ba935570f1d6..6ce9a9c7b1e0a 100644 --- a/cmd/trace-agent/subcommands/run/command.go +++ b/cmd/trace-agent/subcommands/run/command.go @@ -12,7 +12,6 @@ import ( "go.uber.org/fx" "github.com/DataDog/datadog-agent/cmd/agent/common" - "github.com/DataDog/datadog-agent/cmd/agent/common/path" "github.com/DataDog/datadog-agent/cmd/trace-agent/subcommands" coreconfig "github.com/DataDog/datadog-agent/comp/core/config" corelog "github.com/DataDog/datadog-agent/comp/core/log" @@ -70,9 +69,8 @@ func runFx(ctx context.Context, cliParams *RunParams, defaultConfPath string) er fx.Supply(secrets.NewEnabledParams()), coreconfig.Module, fx.Provide(func() corelog.Params { - p := corelog.ForDaemon("TRACE", "apm_config.log_file", path.DefaultLogFile) - return p - }), // fx.Supply(ctx) fails with a missing type error. + return corelog.ForDaemon("TRACE", "apm_config.log_file", config.DefaultLogFilePath) + }), corelog.TraceModule, // setup workloadmeta collectors.GetCatalog(), diff --git a/comp/core/bundle_mock.go b/comp/core/bundle_mock.go index 22a756b2b25cf..80f31d3beaab7 100644 --- a/comp/core/bundle_mock.go +++ b/comp/core/bundle_mock.go @@ -27,14 +27,22 @@ import ( // team: agent-shared-components +// MakeMockBundle returns a core bundle with a customized set of fx.Option including sane defaults. +func MakeMockBundle(logParams, logger fx.Option) fxutil.BundleOptions { + return fxutil.Bundle( + fx.Provide(func(params BundleParams) config.Params { return params.ConfigParams }), + config.MockModule, + logParams, + logger, + fx.Provide(func(params BundleParams) sysprobeconfigimpl.Params { return params.SysprobeConfigParams }), + sysprobeconfigimpl.MockModule, + telemetry.Module, + hostnameimpl.MockModule, + ) +} + // MockBundle defines the mock fx options for this bundle. -var MockBundle = fxutil.Bundle( - fx.Provide(func(params BundleParams) config.Params { return params.ConfigParams }), - config.MockModule, +var MockBundle = MakeMockBundle( fx.Supply(log.Params{}), log.MockModule, - fx.Provide(func(params BundleParams) sysprobeconfigimpl.Params { return params.SysprobeConfigParams }), - sysprobeconfigimpl.MockModule, - telemetry.Module, - hostnameimpl.MockModule, ) diff --git a/comp/core/log/component.go b/comp/core/log/component.go index 3467e208ed4a7..e32a84cad9494 100644 --- a/comp/core/log/component.go +++ b/comp/core/log/component.go @@ -89,3 +89,8 @@ var TraceModule fx.Option = fxutil.Component( var MockModule fx.Option = fxutil.Component( fx.Provide(newMockLogger), ) + +// TraceMockModule defines the fx options for the mock component in its Trace variant. +var TraceMockModule fx.Option = fxutil.Component( + fx.Provide(newTraceMockLogger), +) diff --git a/comp/core/log/mock.go b/comp/core/log/mock.go index 73809e3989273..75fb96c56d2ab 100644 --- a/comp/core/log/mock.go +++ b/comp/core/log/mock.go @@ -7,9 +7,12 @@ package log import ( "context" + "fmt" + "os" "strings" "testing" + "github.com/DataDog/datadog-agent/comp/core/config" "github.com/cihub/seelog" "go.uber.org/fx" @@ -49,3 +52,12 @@ func newMockLogger(t testing.TB, lc fx.Lifecycle) (Component, error) { return &logger{}, nil } + +func newTraceMockLogger(t testing.TB, lc fx.Lifecycle, params Params, cfg config.Component) (Component, error) { + // Make sure we are setting a default value on purpose. + logFilePath := params.logFileFn(cfg) + if logFilePath != os.Getenv("DDTEST_DEFAULT_LOG_FILE_PATH") { + return nil, fmt.Errorf("unexpected default log file path: %q", logFilePath) + } + return newMockLogger(t, lc) +} diff --git a/comp/trace/bundle_test.go b/comp/trace/bundle_test.go index 57e6babeafd5d..98377ab7cdccd 100644 --- a/comp/trace/bundle_test.go +++ b/comp/trace/bundle_test.go @@ -10,6 +10,7 @@ import ( "os" "testing" + "github.com/DataDog/datadog-agent/comp/core/log" "github.com/stretchr/testify/require" "go.uber.org/fx" @@ -30,7 +31,6 @@ func TestBundleDependencies(t *testing.T) { fx.Provide(func() context.Context { return context.TODO() }), // fx.Supply(ctx) fails with a missing type error. fx.Supply(core.BundleParams{}), core.Bundle, - fx.Supply(workloadmeta.NewParams()), workloadmeta.Module, statsd.Module, @@ -47,10 +47,14 @@ func TestMockBundleDependencies(t *testing.T) { os.Setenv("DD_DD_URL", "https://example.com") defer func() { os.Unsetenv("DD_DD_URL") }() + // Only for test purposes to avoid setting a different default value. + os.Setenv("DDTEST_DEFAULT_LOG_FILE_PATH", config.DefaultLogFilePath) + defer func() { os.Unsetenv("DDTEST_DEFAULT_LOG_FILE_PATH") }() + cfg := fxutil.Test[config.Component](t, fx.Options( fx.Provide(func() context.Context { return context.TODO() }), // fx.Supply(ctx) fails with a missing type error. fx.Supply(core.BundleParams{}), - core.MockBundle, + traceMockBundle, fx.Supply(workloadmeta.NewParams()), workloadmeta.Module, fx.Invoke(func(_ config.Component) {}), @@ -63,3 +67,10 @@ func TestMockBundleDependencies(t *testing.T) { require.NotNil(t, cfg.Object()) } + +var traceMockBundle = core.MakeMockBundle( + fx.Provide(func() log.Params { + return log.ForDaemon("TRACE", "apm_config.log_file", config.DefaultLogFilePath) + }), + log.TraceMockModule, +)