Refactor command telemetry (#198)

Author: carole-lavillonniere

.claude/skills/add-command/SKILL.md

@@ -61,7 +61,7 @@ Create `internal/<package>/<name>.go` (use an existing package if it fits, or cr
 
 In `cmd/root.go`, add the new command to `root.AddCommand(...)`.
 
-If the command constructor needs dependencies (like `*env.Env` or `*telemetry.Client`), add them as parameters matching the existing pattern.
+If the command constructor needs dependencies (like `*env.Env`), add them as parameters matching the existing pattern.
 
 ## Step 4: Add new event types (if needed)
 
@@ -96,9 +96,7 @@ Create `test/integration/<name>_test.go` with:
 
 ## Telemetry
 
-Every new command must emit an `lstk_command` telemetry event. Wrap the command's `RunE` with `commandWithTelemetry(name, tel, fn)` — this handles timing, exit code, and error message automatically.
-
-Start and stop are exceptions: they emit `lstk_lifecycle` events in addition to `lstk_command`, so they manage their own telemetry manually instead of using `commandWithTelemetry`.
+`lstk_command` telemetry is emitted automatically for every command via `instrumentCommands` in `cmd/root.go` — no per-command wiring needed. Do NOT pass `*telemetry.Client` to a new command constructor unless the command emits additional lifecycle events (like `lstk_lifecycle` for start/stop/restart) or needs to update the auth token (like login).
 
 In the corresponding integration test, add an assertion that the `lstk_command` event was emitted.
 

cmd/aws.go

@@ -12,12 +12,11 @@ import (
 	"github.com/localstack/lstk/internal/env"
 	"github.com/localstack/lstk/internal/output"
 	"github.com/localstack/lstk/internal/runtime"
-	"github.com/localstack/lstk/internal/telemetry"
 	"github.com/localstack/lstk/internal/terminal"
 	"github.com/spf13/cobra"
 )
 
-func newAWSCmd(cfg *env.Env, tel *telemetry.Client) *cobra.Command {
+func newAWSCmd(cfg *env.Env) *cobra.Command {
 	return &cobra.Command{
 		Use:   "aws [args...]",
 		Short: "Run AWS CLI commands against LocalStack",
@@ -35,7 +34,7 @@ Examples:
   lstk aws s3 mb s3://my-bucket`,
 		DisableFlagParsing: true,
 		PreRunE:            initConfig,
-		RunE: commandWithTelemetry("aws", tel, func(cmd *cobra.Command, args []string) error {
+		RunE: func(cmd *cobra.Command, args []string) error {
 			rt, err := runtime.NewDockerRuntime(cfg.DockerHost)
 			if err != nil {
 				return err
@@ -93,6 +92,6 @@ Examples:
 			}
 
 			return awscli.Exec(cmd.Context(), "http://"+host, profileExists, stdout, stderr, args)
-		}),
+		},
 	}
 }

cmd/config.go

@@ -5,27 +5,26 @@ import (
 
 	"github.com/localstack/lstk/internal/config"
 	"github.com/localstack/lstk/internal/env"
-	"github.com/localstack/lstk/internal/telemetry"
 	"github.com/localstack/lstk/internal/ui"
 	"github.com/spf13/cobra"
 )
 
-func newConfigCmd(cfg *env.Env, tel *telemetry.Client) *cobra.Command {
+func newConfigCmd(cfg *env.Env) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "config",
 		Short: "Manage configuration",
 	}
-	cmd.AddCommand(newConfigProfileCmd(cfg, tel))
-	cmd.AddCommand(newConfigPathCmd(cfg, tel))
+	cmd.AddCommand(newConfigProfileCmd(cfg))
+	cmd.AddCommand(newConfigPathCmd())
 	return cmd
 }
 
-func newConfigProfileCmd(cfg *env.Env, tel *telemetry.Client) *cobra.Command {
+func newConfigProfileCmd(cfg *env.Env) *cobra.Command {
 	return &cobra.Command{
 		Use:     "profile",
 		Short:   "Deprecated: use 'lstk setup aws' instead",
 		PreRunE: initConfig,
-		RunE: commandWithTelemetry("config profile", tel, func(cmd *cobra.Command, args []string) error {
+		RunE: func(cmd *cobra.Command, args []string) error {
 			appConfig, err := config.Get()
 			if err != nil {
 				return fmt.Errorf("failed to get config: %w", err)
@@ -37,15 +36,15 @@ func newConfigProfileCmd(cfg *env.Env, tel *telemetry.Client) *cobra.Command {
 
 			// Delegate to the same handler as "lstk setup aws"
 			return ui.RunConfigProfile(cmd.Context(), appConfig.Containers, cfg.LocalStackHost)
-		}),
+		},
 	}
 }
 
-func newConfigPathCmd(cfg *env.Env, tel *telemetry.Client) *cobra.Command {
+func newConfigPathCmd() *cobra.Command {
 	return &cobra.Command{
 		Use:   "path",
 		Short: "Print the configuration file path",
-		RunE: commandWithTelemetry("config path", tel, func(cmd *cobra.Command, args []string) error {
+		RunE: func(cmd *cobra.Command, args []string) error {
 			path, err := cmd.Flags().GetString("config")
 			if err != nil {
 				return err
@@ -62,6 +61,6 @@ func newConfigPathCmd(cfg *env.Env, tel *telemetry.Client) *cobra.Command {
 
 			_, err = fmt.Fprintln(cmd.OutOrStdout(), configPath)
 			return err
-		}),
+		},
 	}
 }

cmd/login.go

@@ -19,7 +19,7 @@ func newLoginCmd(cfg *env.Env, tel *telemetry.Client, logger log.Logger) *cobra.
 		Short:   "Manage login",
 		Long:    "Manage login and store credentials in system keyring",
 		PreRunE: initConfig,
-		RunE: commandWithTelemetry("login", tel, func(cmd *cobra.Command, args []string) error {
+		RunE: func(cmd *cobra.Command, args []string) error {
 			if !isInteractiveMode(cfg) {
 				return fmt.Errorf("login requires an interactive terminal")
 			}
@@ -33,6 +33,6 @@ func newLoginCmd(cfg *env.Env, tel *telemetry.Client, logger log.Logger) *cobra.
 				}
 			}
 			return nil
-		}),
+		},
 	}
 }

cmd/logout.go

@@ -13,17 +13,16 @@ import (
 	"github.com/localstack/lstk/internal/log"
 	"github.com/localstack/lstk/internal/output"
 	"github.com/localstack/lstk/internal/runtime"
-	"github.com/localstack/lstk/internal/telemetry"
 	"github.com/localstack/lstk/internal/ui"
 	"github.com/spf13/cobra"
 )
 
-func newLogoutCmd(cfg *env.Env, tel *telemetry.Client, logger log.Logger) *cobra.Command {
+func newLogoutCmd(cfg *env.Env, logger log.Logger) *cobra.Command {
 	return &cobra.Command{
 		Use:     "logout",
 		Short:   "Remove stored authentication credentials",
 		PreRunE: initConfig,
-		RunE: commandWithTelemetry("logout", tel, func(cmd *cobra.Command, args []string) error {
+		RunE: func(cmd *cobra.Command, args []string) error {
 			platformClient := api.NewPlatformClient(cfg.APIEndpoint, logger)
 			appConfig, err := config.Get()
 			if err != nil {
@@ -58,6 +57,6 @@ func newLogoutCmd(cfg *env.Env, tel *telemetry.Client, logger log.Logger) *cobra
 				}
 			}
 			return nil
-		}),
+		},
 	}
 }

cmd/logs.go

@@ -9,18 +9,17 @@ import (
 	"github.com/localstack/lstk/internal/env"
 	"github.com/localstack/lstk/internal/output"
 	"github.com/localstack/lstk/internal/runtime"
-	"github.com/localstack/lstk/internal/telemetry"
 	"github.com/localstack/lstk/internal/ui"
 	"github.com/spf13/cobra"
 )
 
-func newLogsCmd(cfg *env.Env, tel *telemetry.Client) *cobra.Command {
+func newLogsCmd(cfg *env.Env) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:     "logs",
 		Short:   "Show emulator logs",
 		Long:    "Show logs from the emulator. Use --follow to stream in real-time.",
 		PreRunE: initConfig,
-		RunE: commandWithTelemetry("logs", tel, func(cmd *cobra.Command, args []string) error {
+		RunE: func(cmd *cobra.Command, args []string) error {
 			follow, err := cmd.Flags().GetBool("follow")
 			if err != nil {
 				return err
@@ -41,7 +40,7 @@ func newLogsCmd(cfg *env.Env, tel *telemetry.Client) *cobra.Command {
 				return ui.RunLogs(cmd.Context(), rt, appConfig.Containers, follow, verbose)
 			}
 			return container.Logs(cmd.Context(), rt, output.NewPlainSink(os.Stdout), appConfig.Containers, follow, verbose)
-		}),
+		},
 	}
 	cmd.Flags().BoolP("follow", "f", false, "Follow log output")
 	cmd.Flags().BoolP("verbose", "v", false, "Show all log output without filtering")

cmd/restart.go

@@ -21,7 +21,7 @@ func newRestartCmd(cfg *env.Env, tel *telemetry.Client, logger log.Logger) *cobr
 		Short:   "Restart emulator",
 		Long:    "Stop and restart emulator and services.",
 		PreRunE: initConfig,
-		RunE: commandWithTelemetry("restart", tel, func(cmd *cobra.Command, args []string) error {
+		RunE: func(cmd *cobra.Command, args []string) error {
 			rt, err := runtime.NewDockerRuntime(cfg.DockerHost)
 			if err != nil {
 				return err
@@ -43,6 +43,6 @@ func newRestartCmd(cfg *env.Env, tel *telemetry.Client, logger log.Logger) *cobr
 
 			sink := output.NewPlainSink(os.Stdout)
 			return container.Restart(cmd.Context(), rt, sink, stopOpts, startOpts, false)
-		}),
+		},
 	}
 }

cmd/root.go

@@ -40,7 +40,7 @@ func NewRootCmd(cfg *env.Env, tel *telemetry.Client, logger log.Logger) *cobra.C
 			if err != nil {
 				return err
 			}
-			return runStart(cmd.Context(), cmd.Flags(), rt, cfg, tel, logger)
+			return startEmulator(cmd.Context(), rt, cfg, tel, logger)
 		},
 	}
 
@@ -63,15 +63,15 @@ func NewRootCmd(cfg *env.Env, tel *telemetry.Client, logger log.Logger) *cobra.C
 		newStopCmd(cfg, tel),
 		newRestartCmd(cfg, tel, logger),
 		newLoginCmd(cfg, tel, logger),
-		newLogoutCmd(cfg, tel, logger),
-		newStatusCmd(cfg, tel),
-		newLogsCmd(cfg, tel),
-		newSetupCmd(cfg, tel),
-		newConfigCmd(cfg, tel),
-		newVolumeCmd(cfg, tel),
-		newUpdateCmd(cfg, tel),
+		newLogoutCmd(cfg, logger),
+		newStatusCmd(cfg),
+		newLogsCmd(cfg),
+		newSetupCmd(cfg),
+		newConfigCmd(cfg),
+		newVolumeCmd(cfg),
+		newUpdateCmd(cfg),
 		newDocsCmd(),
-		newAWSCmd(cfg, tel),
+		newAWSCmd(cfg),
 	)
 
 	return root
@@ -118,6 +118,7 @@ func Execute(ctx context.Context) error {
 	root := NewRootCmd(cfg, tel, logger)
 	root.SilenceErrors = true
 	root.SilenceUsage = true
+	instrumentCommands(root, tel)
 	if cfg.TracesEnabled {
 		wrapCommandsWithTracing(root)
 	}
@@ -192,48 +193,37 @@ func startEmulator(ctx context.Context, rt runtime.Runtime, cfg *env.Env, tel *t
 	return container.Start(ctx, rt, sink, opts, false)
 }
 
-func runStart(ctx context.Context, cmdFlags *pflag.FlagSet, rt runtime.Runtime, cfg *env.Env, tel *telemetry.Client, logger log.Logger) error {
-	startTime := time.Now()
-
-	var flags []string
-	cmdFlags.Visit(func(f *pflag.Flag) {
-		flags = append(flags, "--"+f.Name)
-	})
-
-	runErr := startEmulator(ctx, rt, cfg, tel, logger)
-
-	exitCode := 0
-	errorMsg := ""
-	if runErr != nil {
-		exitCode = 1
-		errorMsg = runErr.Error()
-	}
-	tel.EmitCommand(ctx, "start", flags, time.Since(startTime).Milliseconds(), exitCode, errorMsg)
-
-	return runErr
-}
-
-// wraps a RunE function so that an lstk_command event is emitted after every invocation
-// used for commands that do not emit lstk_lifecycle events (i.e. status, logs, config path, etc)
-func commandWithTelemetry(name string, tel *telemetry.Client, fn func(*cobra.Command, []string) error) func(*cobra.Command, []string) error {
-	return func(cmd *cobra.Command, args []string) error {
-		startTime := time.Now()
-		runErr := fn(cmd, args)
+// instrumentCommands walks the Cobra command tree and wraps every RunE with telemetry emission.
+func instrumentCommands(cmd *cobra.Command, tel *telemetry.Client) {
+	if cmd.RunE != nil {
+		original := cmd.RunE
+		cmd.RunE = func(c *cobra.Command, args []string) error {
+			startTime := time.Now()
+			runErr := original(c, args)
+
+			var flags []string
+			c.Flags().Visit(func(f *pflag.Flag) {
+				flags = append(flags, "--"+f.Name)
+			})
+
+			exitCode := 0
+			errorMsg := ""
+			if runErr != nil {
+				exitCode = 1
+				errorMsg = runErr.Error()
+			}
 
-		var flags []string
-		cmd.Flags().Visit(func(f *pflag.Flag) {
-			flags = append(flags, "--"+f.Name)
-		})
+			commandName := strings.TrimPrefix(c.CommandPath(), c.Root().Name()+" ")
+			if c == c.Root() {
+				commandName = "start"
+			}
+			tel.EmitCommand(c.Context(), commandName, flags, time.Since(startTime).Milliseconds(), exitCode, errorMsg)
 
-		exitCode := 0
-		errorMsg := ""
-		if runErr != nil {
-			exitCode = 1
-			errorMsg = runErr.Error()
+			return runErr
 		}
-		tel.EmitCommand(cmd.Context(), name, flags, time.Since(startTime).Milliseconds(), exitCode, errorMsg)
-
-		return runErr
+	}
+	for _, child := range cmd.Commands() {
+		instrumentCommands(child, tel)
 	}
 }
 

cmd/setup.go

@@ -5,28 +5,27 @@ import (
 
 	"github.com/localstack/lstk/internal/config"
 	"github.com/localstack/lstk/internal/env"
-	"github.com/localstack/lstk/internal/telemetry"
 	"github.com/localstack/lstk/internal/ui"
 	"github.com/spf13/cobra"
 )
 
-func newSetupCmd(cfg *env.Env, tel *telemetry.Client) *cobra.Command {
+func newSetupCmd(cfg *env.Env) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "setup",
 		Short: "Set up emulator CLI integration",
 		Long:  "Set up emulator CLI integration. Currently only AWS is supported.",
 	}
-	cmd.AddCommand(newSetupAWSCmd(cfg, tel))
+	cmd.AddCommand(newSetupAWSCmd(cfg))
 	return cmd
 }
 
-func newSetupAWSCmd(cfg *env.Env, tel *telemetry.Client) *cobra.Command {
+func newSetupAWSCmd(cfg *env.Env) *cobra.Command {
 	return &cobra.Command{
 		Use:     "aws",
 		Short:   "Set up the LocalStack AWS profile",
 		Long:    "Set up the LocalStack AWS profile in ~/.aws/config and ~/.aws/credentials for use with AWS CLI and SDKs.",
 		PreRunE: initConfig,
-		RunE: commandWithTelemetry("setup aws", tel, func(cmd *cobra.Command, args []string) error {
+		RunE: func(cmd *cobra.Command, args []string) error {
 			appConfig, err := config.Get()
 			if err != nil {
 				return fmt.Errorf("failed to get config: %w", err)
@@ -37,6 +36,6 @@ func newSetupAWSCmd(cfg *env.Env, tel *telemetry.Client) *cobra.Command {
 			}
 
 			return ui.RunConfigProfile(cmd.Context(), appConfig.Containers, cfg.LocalStackHost)
-		}),
+		},
 	}
 }

cmd/start.go

@@ -19,7 +19,7 @@ func newStartCmd(cfg *env.Env, tel *telemetry.Client, logger log.Logger) *cobra.
 			if err != nil {
 				return err
 			}
-			return runStart(cmd.Context(), cmd.Flags(), rt, cfg, tel, logger)
+			return startEmulator(cmd.Context(), rt, cfg, tel, logger)
 		},
 	}
 }