cmd/trace-agent/subcommands/run: fix file path for trace-agent logger (…

…DataDog#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
kkpan11 · Nov 24, 2023 · a2308e0 · a2308e0
1 parent 3e5c23d
commit a2308e0
Show file tree

Hide file tree

Showing 6 changed files with 48 additions and 14 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,
+)