From 5b49f9f914a1177d5519bae4d422c57777e29455 Mon Sep 17 00:00:00 2001 From: Joshua Matthews Date: Tue, 25 Nov 2025 17:09:34 -0500 Subject: [PATCH 01/17] Add cli commands to import trace data --- cli/app.go | 25 +++++++++++ cli/client.go | 120 ++++++++++++++++++++++++++++++++++++++++++++++++++ go.mod | 2 + go.sum | 4 ++ 4 files changed, 151 insertions(+) diff --git a/cli/app.go b/cli/app.go index 1f364eeed46..e2befd0fcb0 100644 --- a/cli/app.go +++ b/cli/app.go @@ -456,6 +456,31 @@ var app = &cli.App{ }, }, Commands: []*cli.Command{ + { + Name: "traces", + Usage: "Work with viam-server traces", + Subcommands: []*cli.Command{ + { + Name: "import-local", + Description: "Import traces from a local viam server trace file to an OTLP endpoint.", + Flags: []cli.Flag{ + &cli.StringFlag{ + Name: "path", + TakesFile: true, + Required: true, + Usage: "path to file to import", + }, + }, + Action: createCommandWithT(ImportTraceFileAction), + }, + { + Name: "import-remote", + Description: "Import traces from a remote viam machine to an OTLP endpoint.", + Flags: commonPartFlags, + Action: createCommandWithT(MachinesPartImportTracesAction), + }, + }, + }, { Name: "login", // NOTE(benjirewis): maintain `auth` as an alias for backward compatibility. diff --git a/cli/client.go b/cli/client.go index 3769d21eeb0..9f9b3de35ba 100644 --- a/cli/client.go +++ b/cli/client.go @@ -30,6 +30,8 @@ import ( "github.com/nathan-fiscaletti/consolesize-go" "github.com/pkg/errors" "github.com/urfave/cli/v2" + "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" + tracepb "go.opentelemetry.io/proto/otlp/trace/v1" "go.uber.org/multierr" "go.uber.org/zap" buildpb "go.viam.com/api/app/build/v1" @@ -52,6 +54,7 @@ import ( rconfig "go.viam.com/rdk/config" "go.viam.com/rdk/grpc" "go.viam.com/rdk/logging" + "go.viam.com/rdk/protoutils" "go.viam.com/rdk/resource" "go.viam.com/rdk/robot/client" "go.viam.com/rdk/services/shell" @@ -81,6 +84,7 @@ const ( var ( errNoShellService = errors.New("shell service is not enabled on this machine part") ftdcPath = path.Join("~", ".viam", "diagnostics.data") + tracesPath = path.Join("~", ".viam", "trace") ) // viamClient wraps a cli.Context and provides all the CLI command functionality @@ -1322,6 +1326,10 @@ type machinesPartGetFTDCArgs struct { Part string } +type importTracesFileArgs struct { + Path string +} + // MachinesPartGetFTDCAction is the corresponding Action for 'machines part get-ftdc'. func MachinesPartGetFTDCAction(c *cli.Context, args machinesPartGetFTDCArgs) error { client, err := newViamClient(c) @@ -1338,6 +1346,32 @@ func MachinesPartGetFTDCAction(c *cli.Context, args machinesPartGetFTDCArgs) err return client.machinesPartGetFTDCAction(c, args, globalArgs.Debug, logger) } +// MachinesPartImportTracesAction is the corresponding Action for 'machines part import-traces'. +func MachinesPartImportTracesAction(c *cli.Context, args machinesPartGetFTDCArgs) error { + client, err := newViamClient(c) + if err != nil { + return err + } + + globalArgs, err := getGlobalArgs(c) + if err != nil { + return err + } + logger := globalArgs.createLogger() + + return client.machinesPartImportTracesAction(c, args, globalArgs.Debug, logger) +} + +// ImportTraceFileAction is the corresponding action for 'import-traces'. +func ImportTraceFileAction(c *cli.Context, args importTracesFileArgs) error { + client, err := newViamClient(c) + if err != nil { + return err + } + + return client.importTraceFileAction(c, args) +} + // MachinesPartCopyFilesAction is the corresponding Action for 'machines part cp'. func MachinesPartCopyFilesAction(c *cli.Context, args machinesPartCopyFilesArgs) error { client, err := newViamClient(c) @@ -1517,6 +1551,92 @@ func (c *viamClient) machinesPartGetFTDCAction( return nil } +func (c *viamClient) machinesPartImportTracesAction( + ctx *cli.Context, + flagArgs machinesPartGetFTDCArgs, + debug bool, + logger logging.Logger, +) error { + targetPath, err := os.MkdirTemp(os.TempDir(), "rdktraceimport") + if err != nil { + return err + } + //nolint: errcheck + defer os.RemoveAll(targetPath) + + part, err := c.robotPart(flagArgs.Organization, flagArgs.Location, flagArgs.Machine, flagArgs.Part) + if err != nil { + return err + } + // Intentional use of path instead of filepath: Windows understands both / and + // \ as path separators, and we don't want a cli running on Windows to send + // a path using \ to a *NIX machine. + src := path.Join(tracesPath, part.Id) + gArgs, err := getGlobalArgs(ctx) + quiet := err == nil && gArgs != nil && gArgs.Quiet + var startTime time.Time + if !quiet { + startTime = time.Now() + printf(ctx.App.Writer, "Saving to %s ...", path.Join(targetPath, part.GetId())) + } + if err := c.copyFilesFromMachine( + flagArgs.Organization, + flagArgs.Location, + flagArgs.Machine, + flagArgs.Part, + debug, + true, + false, + []string{src}, + targetPath, + logger, + ); err != nil { + if statusErr := status.Convert(err); statusErr != nil && + statusErr.Code() == codes.InvalidArgument && + statusErr.Message() == shell.ErrMsgDirectoryCopyRequestNoRecursion { + return errDirectoryCopyRequestNoRecursion + } + return err + } + printf(ctx.App.Writer, "Done in %s. Files at %s", time.Since(startTime), targetPath) + traceFilePath := filepath.Join(targetPath, part.GetId(), "traces") + return c.importTraceFileAction(ctx, importTracesFileArgs{Path: traceFilePath}) +} + +func (c *viamClient) importTraceFileAction( + ctx *cli.Context, + args importTracesFileArgs, +) error { + traceFile, err := os.Open(args.Path) + if err != nil { + if os.IsNotExist(err) { + printf(ctx.App.Writer, "No traces found") + return nil + } + return errors.Wrap(err, "failed to open trace file") + } + traceReader := protoutils.NewDelimitedProtoReader[tracepb.ResourceSpans](traceFile) + //nolint: errcheck + defer traceReader.Close() + otlpClient := otlptracegrpc.NewClient( + otlptracegrpc.WithEndpoint("localhost:4317"), + otlptracegrpc.WithInsecure(), + ) + if err := otlpClient.Start(ctx.Context); err != nil { + return err + } + //nolint: errcheck + defer otlpClient.Stop(ctx.Context) + var msg tracepb.ResourceSpans + for span := range traceReader.AllWithMemory(&msg) { + err := otlpClient.UploadTraces(ctx.Context, []*tracepb.ResourceSpans{span}) + if err != nil { + printf(ctx.App.Writer, "Error uploading trace: %v", err) + } + } + return nil +} + type robotsPartTunnelArgs struct { Organization string Location string diff --git a/go.mod b/go.mod index 63c704530fe..86c0216d7f9 100644 --- a/go.mod +++ b/go.mod @@ -85,6 +85,7 @@ require ( go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.54.0 go.opentelemetry.io/otel v1.38.0 go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.38.0 + go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.38.0 go.opentelemetry.io/otel/sdk v1.38.0 go.opentelemetry.io/otel/trace v1.38.0 go.opentelemetry.io/proto/otlp v1.7.1 @@ -160,6 +161,7 @@ require ( github.com/catppuccin/go v0.2.0 // indirect github.com/cenkalti/backoff v2.2.1+incompatible // indirect github.com/cenkalti/backoff/v4 v4.3.0 // indirect + github.com/cenkalti/backoff/v5 v5.0.3 // indirect github.com/census-instrumentation/opencensus-proto v0.4.1 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/charmbracelet/bubbles v0.20.0 // indirect diff --git a/go.sum b/go.sum index 838fc577f4d..a9328eb4af6 100644 --- a/go.sum +++ b/go.sum @@ -178,6 +178,8 @@ github.com/cenkalti/backoff v2.2.1+incompatible/go.mod h1:90ReRw6GdpyfrHakVjL/QH github.com/cenkalti/backoff/v4 v4.1.1/go.mod h1:scbssz8iZGpm3xbr14ovlUdkxfGXNInqkPWOWmG2CLw= github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8= github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE= +github.com/cenkalti/backoff/v5 v5.0.3 h1:ZN+IMa753KfX5hd8vVaMixjnqRZ3y8CuJKRKj1xcsSM= +github.com/cenkalti/backoff/v5 v5.0.3/go.mod h1:rkhZdG3JZukswDf7f0cwqPNk4K0sa+F97BxZthm/crw= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= github.com/census-instrumentation/opencensus-proto v0.4.1 h1:iKLQ0xPNFxR/2hzXZMrBo8f1j86j5WHzznCCQxV/b8g= github.com/census-instrumentation/opencensus-proto v0.4.1/go.mod h1:4T9NM4+4Vw91VeyqjLS6ao50K5bOcLKN6Q42XnYaRYw= @@ -1073,6 +1075,8 @@ go.opentelemetry.io/otel v1.38.0 h1:RkfdswUDRimDg0m2Az18RKOsnI8UDzppJAtj01/Ymk8= go.opentelemetry.io/otel v1.38.0/go.mod h1:zcmtmQ1+YmQM9wrNsTGV/q/uyusom3P8RxwExxkZhjM= go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.38.0 h1:GqRJVj7UmLjCVyVJ3ZFLdPRmhDUp2zFmQe3RHIOsw24= go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.38.0/go.mod h1:ri3aaHSmCTVYu2AWv44YMauwAQc0aqI9gHKIcSbI1pU= +go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.38.0 h1:lwI4Dc5leUqENgGuQImwLo4WnuXFPetmPpkLi2IrX54= +go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.38.0/go.mod h1:Kz/oCE7z5wuyhPxsXDuaPteSWqjSBD5YaSdbxZYGbGk= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.24.0 h1:Xw8U6u2f8DK2XAkGRFV7BBLENgnTGX9i4rQRxJf+/vs= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.24.0/go.mod h1:6KW1Fm6R/s6Z3PGXwSJN2K4eT6wQB3vXX6CVnYX9NmM= go.opentelemetry.io/otel/metric v1.38.0 h1:Kl6lzIYGAh5M159u9NgiRkmoMKjvbsKtYRwgfrA6WpA= From 07827f0ad8f1d19a56e56a0bee70f66c2039d5df Mon Sep 17 00:00:00 2001 From: Joshua Matthews Date: Wed, 26 Nov 2025 13:57:41 -0500 Subject: [PATCH 02/17] Redirect stderr to stop make from complaining about missing dpkg --- packaging.make | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packaging.make b/packaging.make index 0509c1a00a6..046c029d761 100644 --- a/packaging.make +++ b/packaging.make @@ -1,7 +1,7 @@ BUILD_CHANNEL?=local # note: UNAME_M is overrideable because it is wrong in 32-bit arm container executing natively on 64-bit arm UNAME_M ?= $(shell uname -m) -ifneq ($(shell which dpkg), "") +ifneq ($(shell which dpkg 2>/dev/null), "") DPKG_ARCH ?= $(shell dpkg --print-architecture) APPIMAGE_ARCH ?= $(shell dpkg --print-architecture) endif From a8cc3dcf27b0f6af97acce5ca7a0d8bd1967ec19 Mon Sep 17 00:00:00 2001 From: Joshua Matthews Date: Wed, 26 Nov 2025 16:06:11 -0500 Subject: [PATCH 03/17] Fix cli flags --- cli/app.go | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/cli/app.go b/cli/app.go index e2befd0fcb0..3410521c750 100644 --- a/cli/app.go +++ b/cli/app.go @@ -8,6 +8,7 @@ import ( "regexp" "strings" + "github.com/samber/lo" "github.com/urfave/cli/v2" "go.viam.com/rdk/logging" @@ -189,6 +190,15 @@ var commonPartFlags = []cli.Flag{ }, } +var commonOtlpFlags = []cli.Flag{ + &cli.StringFlag{ + Name: "endpoint", + DefaultText: "localhost:4317", + Usage: "OTLP endpoint in host:port format", + Required: true, + }, +} + // matches all uppercase characters that follow lowercase chars and aren't at the [0] index of a string. // This is useful for converting camel case into kabob case when getting values out of a CLI Context // based on a flag name, and putting them into a struct with a camel cased field name. @@ -463,21 +473,25 @@ var app = &cli.App{ { Name: "import-local", Description: "Import traces from a local viam server trace file to an OTLP endpoint.", - Flags: []cli.Flag{ - &cli.StringFlag{ + Flags: lo.Flatten([][]cli.Flag{ + {&cli.StringFlag{ Name: "path", TakesFile: true, Required: true, Usage: "path to file to import", - }, - }, + }}, + commonOtlpFlags, + }), Action: createCommandWithT(ImportTraceFileAction), }, { Name: "import-remote", Description: "Import traces from a remote viam machine to an OTLP endpoint.", - Flags: commonPartFlags, - Action: createCommandWithT(MachinesPartImportTracesAction), + Flags: lo.Flatten([][]cli.Flag{ + commonOtlpFlags, + commonPartFlags, + }), + Action: createCommandWithT(MachinesPartImportTracesAction), }, }, }, From eba223c3293658fb98af3e18edfd0e8baba0eba8 Mon Sep 17 00:00:00 2001 From: Joshua Matthews Date: Thu, 27 Nov 2025 00:04:55 -0500 Subject: [PATCH 04/17] Rearrange cli trace commands and add support for printing to console --- cli/app.go | 13 +++++++++++++ cli/client.go | 53 +++++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 56 insertions(+), 10 deletions(-) diff --git a/cli/app.go b/cli/app.go index 3410521c750..c5c9144a056 100644 --- a/cli/app.go +++ b/cli/app.go @@ -493,6 +493,19 @@ var app = &cli.App{ }), Action: createCommandWithT(MachinesPartImportTracesAction), }, + { + Name: "print-local", + Description: "Print traces in a local file to the console", + Flags: lo.Flatten([][]cli.Flag{ + {&cli.StringFlag{ + Name: "path", + TakesFile: true, + Required: true, + Usage: "path to file to import", + }}, + }), + Action: createCommandWithT(PrintTraceFileAction), + }, }, }, { diff --git a/cli/client.go b/cli/client.go index 9f9b3de35ba..1c812fe81e1 100644 --- a/cli/client.go +++ b/cli/client.go @@ -5,6 +5,7 @@ import ( "bufio" "context" "encoding/json" + stderrors "errors" "fmt" "io" "io/fs" @@ -44,6 +45,7 @@ import ( apppb "go.viam.com/api/app/v1" commonpb "go.viam.com/api/common/v1" "go.viam.com/utils" + "go.viam.com/utils/perf" "go.viam.com/utils/rpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/metadata" @@ -1346,7 +1348,7 @@ func MachinesPartGetFTDCAction(c *cli.Context, args machinesPartGetFTDCArgs) err return client.machinesPartGetFTDCAction(c, args, globalArgs.Debug, logger) } -// MachinesPartImportTracesAction is the corresponding Action for 'machines part import-traces'. +// MachinesPartImportTracesAction is the corresponding Action for 'trace import-remote'. func MachinesPartImportTracesAction(c *cli.Context, args machinesPartGetFTDCArgs) error { client, err := newViamClient(c) if err != nil { @@ -1362,14 +1364,14 @@ func MachinesPartImportTracesAction(c *cli.Context, args machinesPartGetFTDCArgs return client.machinesPartImportTracesAction(c, args, globalArgs.Debug, logger) } -// ImportTraceFileAction is the corresponding action for 'import-traces'. -func ImportTraceFileAction(c *cli.Context, args importTracesFileArgs) error { - client, err := newViamClient(c) - if err != nil { - return err - } +// PrintTraceFileAction is the corresponding action for 'trace print-local'. +func PrintTraceFileAction(c *cli.Context, args importTracesFileArgs) error { + return printTraceFileAction(c, args) +} - return client.importTraceFileAction(c, args) +// ImportTraceFileAction is the corresponding action for 'trace import-local'. +func ImportTraceFileAction(c *cli.Context, args importTracesFileArgs) error { + return importTraceFileAction(c, args) } // MachinesPartCopyFilesAction is the corresponding Action for 'machines part cp'. @@ -1600,10 +1602,41 @@ func (c *viamClient) machinesPartImportTracesAction( } printf(ctx.App.Writer, "Done in %s. Files at %s", time.Since(startTime), targetPath) traceFilePath := filepath.Join(targetPath, part.GetId(), "traces") - return c.importTraceFileAction(ctx, importTracesFileArgs{Path: traceFilePath}) + return importTraceFileAction(ctx, importTracesFileArgs{Path: traceFilePath}) +} + +func printTraceFileAction( + ctx *cli.Context, + args importTracesFileArgs, +) error { + traceFile, err := os.Open(args.Path) + if err != nil { + if os.IsNotExist(err) { + printf(ctx.App.Writer, "No traces found") + return nil + } + return errors.Wrap(err, "failed to open trace file") + } + traceReader := protoutils.NewDelimitedProtoReader[tracepb.ResourceSpans](traceFile) + //nolint: errcheck + defer traceReader.Close() + + devExporter := perf.NewOtelDevelopmentExporter() + if err := devExporter.Start(); err != nil { + return err + } + defer devExporter.Stop() + var msg tracepb.ResourceSpans + err = nil + for resource := range traceReader.AllWithMemory(&msg) { + for _, scope := range resource.ScopeSpans { + err = stderrors.Join(err, devExporter.ExportOTLPSpans(ctx.Context, scope.Spans)) + } + } + return err } -func (c *viamClient) importTraceFileAction( +func importTraceFileAction( ctx *cli.Context, args importTracesFileArgs, ) error { From 3a9a80b611c7664fc07f3ca6162105303bf05d59 Mon Sep 17 00:00:00 2001 From: Joshua Matthews Date: Mon, 1 Dec 2025 10:35:53 -0500 Subject: [PATCH 05/17] WIP: better organize trace related cli code --- cli/app.go | 34 ++++++++++++++---- cli/client.go | 96 ++++++++++++++++++++++++++++++++++++++------------- 2 files changed, 100 insertions(+), 30 deletions(-) diff --git a/cli/app.go b/cli/app.go index c5c9144a056..60ce85762f4 100644 --- a/cli/app.go +++ b/cli/app.go @@ -195,7 +195,6 @@ var commonOtlpFlags = []cli.Flag{ Name: "endpoint", DefaultText: "localhost:4317", Usage: "OTLP endpoint in host:port format", - Required: true, }, } @@ -472,7 +471,7 @@ var app = &cli.App{ Subcommands: []*cli.Command{ { Name: "import-local", - Description: "Import traces from a local viam server trace file to an OTLP endpoint.", + Usage: "Import traces from a local viam server trace file to an OTLP endpoint.", Flags: lo.Flatten([][]cli.Flag{ {&cli.StringFlag{ Name: "path", @@ -486,16 +485,16 @@ var app = &cli.App{ }, { Name: "import-remote", - Description: "Import traces from a remote viam machine to an OTLP endpoint.", + Usage: "Import traces from a remote viam machine to an OTLP endpoint.", Flags: lo.Flatten([][]cli.Flag{ commonOtlpFlags, commonPartFlags, }), - Action: createCommandWithT(MachinesPartImportTracesAction), + Action: createCommandWithT(traceImportRemoteAction), }, { Name: "print-local", - Description: "Print traces in a local file to the console", + Usage: "Print traces in a local file to the console", Flags: lo.Flatten([][]cli.Flag{ {&cli.StringFlag{ Name: "path", @@ -504,7 +503,30 @@ var app = &cli.App{ Usage: "path to file to import", }}, }), - Action: createCommandWithT(PrintTraceFileAction), + Action: createCommandWithT(tracePrintLocalAction), + }, + { + Name: "print-remote", + Usage: "Print traces from a remote viam machine to the console", + Flags: lo.Flatten([][]cli.Flag{ + commonPartFlags, + }), + Action: createCommandWithT(tracePrintRemoteAction), + }, + { + Name: "fetch-remote", + Usage: "Download a traces from a viam machine and save them to disk", + Flags: lo.Flatten([][]cli.Flag{ + commonPartFlags, + { + &cli.PathFlag{ + Name: generalFlagDestination, + Required: true, + Usage: "output directory for downloaded traces", + }, + }, + }), + Action: createCommandWithT(tracePrintLocalAction), }, }, }, diff --git a/cli/client.go b/cli/client.go index 1c812fe81e1..058eb6b5e3c 100644 --- a/cli/client.go +++ b/cli/client.go @@ -1328,6 +1328,14 @@ type machinesPartGetFTDCArgs struct { Part string } +type traceFetchRemoteArgs struct { + Organization string + Location string + Machine string + Part string + Destination string +} + type importTracesFileArgs struct { Path string } @@ -1348,25 +1356,30 @@ func MachinesPartGetFTDCAction(c *cli.Context, args machinesPartGetFTDCArgs) err return client.machinesPartGetFTDCAction(c, args, globalArgs.Debug, logger) } -// MachinesPartImportTracesAction is the corresponding Action for 'trace import-remote'. -func MachinesPartImportTracesAction(c *cli.Context, args machinesPartGetFTDCArgs) error { - client, err := newViamClient(c) +func traceImportRemoteAction(ctx *cli.Context, args machinesPartGetFTDCArgs) error { + client, err := newViamClient(ctx) if err != nil { return err } - globalArgs, err := getGlobalArgs(c) + globalArgs, err := getGlobalArgs(ctx) if err != nil { return err } logger := globalArgs.createLogger() - return client.machinesPartImportTracesAction(c, args, globalArgs.Debug, logger) -} + targetPath, err := os.MkdirTemp("", "rdktraceimport") + if err != nil { + return err + } + //nolint: errcheck() + defer os.RemoveAll(targetPath) + + if err := client.machinesPartGetTracesAction(ctx, args, targetPath, globalArgs.Debug, logger); err != nil { + return err + } -// PrintTraceFileAction is the corresponding action for 'trace print-local'. -func PrintTraceFileAction(c *cli.Context, args importTracesFileArgs) error { - return printTraceFileAction(c, args) + return importTraceFileAction(ctx, importTracesFileArgs{Path: filepath.Join(targetPath, "traces")}) } // ImportTraceFileAction is the corresponding action for 'trace import-local'. @@ -1553,19 +1566,13 @@ func (c *viamClient) machinesPartGetFTDCAction( return nil } -func (c *viamClient) machinesPartImportTracesAction( +func (c *viamClient) machinesPartGetTracesAction( ctx *cli.Context, flagArgs machinesPartGetFTDCArgs, + destination string, debug bool, logger logging.Logger, ) error { - targetPath, err := os.MkdirTemp(os.TempDir(), "rdktraceimport") - if err != nil { - return err - } - //nolint: errcheck - defer os.RemoveAll(targetPath) - part, err := c.robotPart(flagArgs.Organization, flagArgs.Location, flagArgs.Machine, flagArgs.Part) if err != nil { return err @@ -1573,13 +1580,13 @@ func (c *viamClient) machinesPartImportTracesAction( // Intentional use of path instead of filepath: Windows understands both / and // \ as path separators, and we don't want a cli running on Windows to send // a path using \ to a *NIX machine. - src := path.Join(tracesPath, part.Id) + src := path.Join(tracesPath, part.Id, "traces") gArgs, err := getGlobalArgs(ctx) quiet := err == nil && gArgs != nil && gArgs.Quiet var startTime time.Time if !quiet { startTime = time.Now() - printf(ctx.App.Writer, "Saving to %s ...", path.Join(targetPath, part.GetId())) + printf(ctx.App.Writer, "Saving to %s ...", path.Join(destination, part.GetId())) } if err := c.copyFilesFromMachine( flagArgs.Organization, @@ -1590,7 +1597,7 @@ func (c *viamClient) machinesPartImportTracesAction( true, false, []string{src}, - targetPath, + destination, logger, ); err != nil { if statusErr := status.Convert(err); statusErr != nil && @@ -1600,12 +1607,53 @@ func (c *viamClient) machinesPartImportTracesAction( } return err } - printf(ctx.App.Writer, "Done in %s. Files at %s", time.Since(startTime), targetPath) - traceFilePath := filepath.Join(targetPath, part.GetId(), "traces") - return importTraceFileAction(ctx, importTracesFileArgs{Path: traceFilePath}) + if !quiet { + printf(ctx.App.Writer, "Done in %s.", time.Since(startTime)) + } + return nil +} + +func tracePrintRemoteAction( + ctx *cli.Context, + args machinesPartGetFTDCArgs, +) error { + client, err := newViamClient(ctx) + if err != nil { + return err + } + + globalArgs, err := getGlobalArgs(ctx) + if err != nil { + return err + } + logger := globalArgs.createLogger() + tmp, err := os.MkdirTemp("", "rdktraceimport") + if err != nil { + return err + } + defer os.RemoveAll(tmp) + if err := client.machinesPartGetTracesAction(ctx, args, tmp, globalArgs.Debug, logger); err != nil { + return err + } + return tracePrintLocalAction(ctx, importTracesFileArgs{Path: filepath.Join(tmp, "traces")}) +} + +func traceFetchRemoteAction(ctx *cli.Context, args traceFetchRemoteArgs) error { + client, err := newViamClient(ctx) + if err != nil { + return err + } + + globalArgs, err := getGlobalArgs(ctx) + if err != nil { + return err + } + logger := globalArgs.createLogger() + + client.machinesPartGetTracesAction(ctx, args, "", globalArgs.Debug, logger) } -func printTraceFileAction( +func tracePrintLocalAction( ctx *cli.Context, args importTracesFileArgs, ) error { From 6bb79196eea73ed789dd045d74571799e6ed7e0f Mon Sep 17 00:00:00 2001 From: Joshua Matthews Date: Wed, 3 Dec 2025 12:18:13 -0500 Subject: [PATCH 06/17] Better organize cli traces code --- cli/app.go | 22 ++++++++--------- cli/client.go | 59 ++++++++++++++++++++++++++-------------------- cli/client_test.go | 6 ++--- 3 files changed, 47 insertions(+), 40 deletions(-) diff --git a/cli/app.go b/cli/app.go index 60ce85762f4..1b5aa3cc6c7 100644 --- a/cli/app.go +++ b/cli/app.go @@ -470,7 +470,7 @@ var app = &cli.App{ Usage: "Work with viam-server traces", Subcommands: []*cli.Command{ { - Name: "import-local", + Name: "import-local", Usage: "Import traces from a local viam server trace file to an OTLP endpoint.", Flags: lo.Flatten([][]cli.Flag{ {&cli.StringFlag{ @@ -481,10 +481,10 @@ var app = &cli.App{ }}, commonOtlpFlags, }), - Action: createCommandWithT(ImportTraceFileAction), + Action: createCommandWithT(traceImportLocalAction), }, { - Name: "import-remote", + Name: "import-remote", Usage: "Import traces from a remote viam machine to an OTLP endpoint.", Flags: lo.Flatten([][]cli.Flag{ commonOtlpFlags, @@ -493,20 +493,20 @@ var app = &cli.App{ Action: createCommandWithT(traceImportRemoteAction), }, { - Name: "print-local", + Name: "print-local", Usage: "Print traces in a local file to the console", - Flags: lo.Flatten([][]cli.Flag{ - {&cli.StringFlag{ + Flags: []cli.Flag{ + &cli.StringFlag{ Name: "path", TakesFile: true, Required: true, Usage: "path to file to import", - }}, - }), + }, + }, Action: createCommandWithT(tracePrintLocalAction), }, { - Name: "print-remote", + Name: "print-remote", Usage: "Print traces from a remote viam machine to the console", Flags: lo.Flatten([][]cli.Flag{ commonPartFlags, @@ -514,7 +514,7 @@ var app = &cli.App{ Action: createCommandWithT(tracePrintRemoteAction), }, { - Name: "fetch-remote", + Name: "fetch-remote", Usage: "Download a traces from a viam machine and save them to disk", Flags: lo.Flatten([][]cli.Flag{ commonPartFlags, @@ -526,7 +526,7 @@ var app = &cli.App{ }, }, }), - Action: createCommandWithT(tracePrintLocalAction), + Action: createCommandWithT(traceFetchRemoteAction), }, }, }, diff --git a/cli/client.go b/cli/client.go index 058eb6b5e3c..4779b7d0002 100644 --- a/cli/client.go +++ b/cli/client.go @@ -1321,7 +1321,7 @@ func (err wrongNumArgsError) Error() string { return fmt.Sprintf("expected %d-%d arguments but got %d", err.min, err.max, err.have) } -type machinesPartGetFTDCArgs struct { +type baseRemotePartArgs struct { Organization string Location string Machine string @@ -1329,11 +1329,8 @@ type machinesPartGetFTDCArgs struct { } type traceFetchRemoteArgs struct { - Organization string - Location string - Machine string - Part string - Destination string + baseRemotePartArgs + Destination string } type importTracesFileArgs struct { @@ -1341,7 +1338,7 @@ type importTracesFileArgs struct { } // MachinesPartGetFTDCAction is the corresponding Action for 'machines part get-ftdc'. -func MachinesPartGetFTDCAction(c *cli.Context, args machinesPartGetFTDCArgs) error { +func MachinesPartGetFTDCAction(c *cli.Context, args baseRemotePartArgs) error { client, err := newViamClient(c) if err != nil { return err @@ -1356,7 +1353,7 @@ func MachinesPartGetFTDCAction(c *cli.Context, args machinesPartGetFTDCArgs) err return client.machinesPartGetFTDCAction(c, args, globalArgs.Debug, logger) } -func traceImportRemoteAction(ctx *cli.Context, args machinesPartGetFTDCArgs) error { +func traceImportRemoteAction(ctx *cli.Context, args baseRemotePartArgs) error { client, err := newViamClient(ctx) if err != nil { return err @@ -1372,19 +1369,22 @@ func traceImportRemoteAction(ctx *cli.Context, args machinesPartGetFTDCArgs) err if err != nil { return err } - //nolint: errcheck() + //nolint: errcheck defer os.RemoveAll(targetPath) - if err := client.machinesPartGetTracesAction(ctx, args, targetPath, globalArgs.Debug, logger); err != nil { + if err := client.machinesPartGetTracesAction( + ctx, + traceFetchRemoteArgs{ + baseRemotePartArgs: args, + Destination: targetPath, + }, + globalArgs.Debug, + logger, + ); err != nil { return err } - return importTraceFileAction(ctx, importTracesFileArgs{Path: filepath.Join(targetPath, "traces")}) -} - -// ImportTraceFileAction is the corresponding action for 'trace import-local'. -func ImportTraceFileAction(c *cli.Context, args importTracesFileArgs) error { - return importTraceFileAction(c, args) + return traceImportLocalAction(ctx, importTracesFileArgs{Path: filepath.Join(targetPath, "traces")}) } // MachinesPartCopyFilesAction is the corresponding Action for 'machines part cp'. @@ -1507,7 +1507,7 @@ func (c *viamClient) machinesPartCopyFilesAction( func (c *viamClient) machinesPartGetFTDCAction( ctx *cli.Context, - flagArgs machinesPartGetFTDCArgs, + flagArgs baseRemotePartArgs, debug bool, logger logging.Logger, ) error { @@ -1568,8 +1568,7 @@ func (c *viamClient) machinesPartGetFTDCAction( func (c *viamClient) machinesPartGetTracesAction( ctx *cli.Context, - flagArgs machinesPartGetFTDCArgs, - destination string, + flagArgs traceFetchRemoteArgs, debug bool, logger logging.Logger, ) error { @@ -1586,7 +1585,7 @@ func (c *viamClient) machinesPartGetTracesAction( var startTime time.Time if !quiet { startTime = time.Now() - printf(ctx.App.Writer, "Saving to %s ...", path.Join(destination, part.GetId())) + printf(ctx.App.Writer, "Saving to %s ...", path.Join(flagArgs.Destination, part.GetId())) } if err := c.copyFilesFromMachine( flagArgs.Organization, @@ -1597,7 +1596,7 @@ func (c *viamClient) machinesPartGetTracesAction( true, false, []string{src}, - destination, + flagArgs.Destination, logger, ); err != nil { if statusErr := status.Convert(err); statusErr != nil && @@ -1615,7 +1614,7 @@ func (c *viamClient) machinesPartGetTracesAction( func tracePrintRemoteAction( ctx *cli.Context, - args machinesPartGetFTDCArgs, + args baseRemotePartArgs, ) error { client, err := newViamClient(ctx) if err != nil { @@ -1632,7 +1631,15 @@ func tracePrintRemoteAction( return err } defer os.RemoveAll(tmp) - if err := client.machinesPartGetTracesAction(ctx, args, tmp, globalArgs.Debug, logger); err != nil { + if err := client.machinesPartGetTracesAction( + ctx, + traceFetchRemoteArgs{ + baseRemotePartArgs: args, + Destination: tmp, + }, + globalArgs.Debug, + logger, + ); err != nil { return err } return tracePrintLocalAction(ctx, importTracesFileArgs{Path: filepath.Join(tmp, "traces")}) @@ -1649,8 +1656,8 @@ func traceFetchRemoteAction(ctx *cli.Context, args traceFetchRemoteArgs) error { return err } logger := globalArgs.createLogger() - - client.machinesPartGetTracesAction(ctx, args, "", globalArgs.Debug, logger) + + return client.machinesPartGetTracesAction(ctx, args, globalArgs.Debug, logger) } func tracePrintLocalAction( @@ -1684,7 +1691,7 @@ func tracePrintLocalAction( return err } -func importTraceFileAction( +func traceImportLocalAction( ctx *cli.Context, args importTracesFileArgs, ) error { diff --git a/cli/client_test.go b/cli/client_test.go index e8311577548..34c16afb870 100644 --- a/cli/client_test.go +++ b/cli/client_test.go @@ -1224,7 +1224,7 @@ func TestShellGetFTDC(t *testing.T) { args := []string{"foo", "bar"} cCtx, viamClient, _, _ := setup(asc, nil, nil, partFlags, "token", args...) test.That(t, - viamClient.machinesPartGetFTDCAction(cCtx, parseStructFromCtx[machinesPartGetFTDCArgs](cCtx), true, logger), + viamClient.machinesPartGetFTDCAction(cCtx, parseStructFromCtx[baseRemotePartArgs](cCtx), true, logger), test.ShouldBeError, wrongNumArgsError{2, 0, 1}) }) @@ -1243,7 +1243,7 @@ func TestShellGetFTDC(t *testing.T) { cCtx, viamClient, _, _ := setupWithRunningPart( t, asc, nil, nil, partFlags, "token", partFqdn, args...) test.That(t, - viamClient.machinesPartGetFTDCAction(cCtx, parseStructFromCtx[machinesPartGetFTDCArgs](cCtx), true, logger), + viamClient.machinesPartGetFTDCAction(cCtx, parseStructFromCtx[baseRemotePartArgs](cCtx), true, logger), test.ShouldNotBeNil) entries, err := os.ReadDir(tempDir) @@ -1277,7 +1277,7 @@ func TestShellGetFTDC(t *testing.T) { cCtx, viamClient, _, _ := setupWithRunningPart( t, asc, nil, nil, partFlags, "token", partFqdn, args...) test.That(t, - viamClient.machinesPartGetFTDCAction(cCtx, parseStructFromCtx[machinesPartGetFTDCArgs](cCtx), true, logger), + viamClient.machinesPartGetFTDCAction(cCtx, parseStructFromCtx[baseRemotePartArgs](cCtx), true, logger), test.ShouldBeNil) entries, err := os.ReadDir(filepath.Join(targetPath, partID)) From 2680cadfd2235e42489aa652839b83f51b816516 Mon Sep 17 00:00:00 2001 From: Joshua Matthews Date: Wed, 3 Dec 2025 16:53:22 -0500 Subject: [PATCH 07/17] Split argument structs up, fix otlp endpoint not getting passed through --- cli/client.go | 82 +++++++++++++++++++++++++++++++--------------- cli/client_test.go | 6 ++-- 2 files changed, 58 insertions(+), 30 deletions(-) diff --git a/cli/client.go b/cli/client.go index 4779b7d0002..c1f3e3d4279 100644 --- a/cli/client.go +++ b/cli/client.go @@ -1321,7 +1321,7 @@ func (err wrongNumArgsError) Error() string { return fmt.Sprintf("expected %d-%d arguments but got %d", err.min, err.max, err.have) } -type baseRemotePartArgs struct { +type machinesPartGetFTDCArgs struct { Organization string Location string Machine string @@ -1329,16 +1329,32 @@ type baseRemotePartArgs struct { } type traceFetchRemoteArgs struct { - baseRemotePartArgs - Destination string + Organization string + Location string + Machine string + Part string + Destination string +} + +type traceImportRemoteArgs struct { + Organization string + Location string + Machine string + Part string + Endpoint string } -type importTracesFileArgs struct { +type traceImportLocalArgs struct { + Path string + Endpoint string +} + +type tracePrintLocalArgs struct { Path string } // MachinesPartGetFTDCAction is the corresponding Action for 'machines part get-ftdc'. -func MachinesPartGetFTDCAction(c *cli.Context, args baseRemotePartArgs) error { +func MachinesPartGetFTDCAction(c *cli.Context, args machinesPartGetFTDCArgs) error { client, err := newViamClient(c) if err != nil { return err @@ -1353,7 +1369,7 @@ func MachinesPartGetFTDCAction(c *cli.Context, args baseRemotePartArgs) error { return client.machinesPartGetFTDCAction(c, args, globalArgs.Debug, logger) } -func traceImportRemoteAction(ctx *cli.Context, args baseRemotePartArgs) error { +func traceImportRemoteAction(ctx *cli.Context, args traceImportRemoteArgs) error { client, err := newViamClient(ctx) if err != nil { return err @@ -1364,19 +1380,20 @@ func traceImportRemoteAction(ctx *cli.Context, args baseRemotePartArgs) error { return err } logger := globalArgs.createLogger() - - targetPath, err := os.MkdirTemp("", "rdktraceimport") + tmp, err := os.MkdirTemp("", "viamtraceimport") if err != nil { return err } //nolint: errcheck - defer os.RemoveAll(targetPath) - - if err := client.machinesPartGetTracesAction( + defer os.RemoveAll(tmp) + if err := client.tracesFetchRemoteAction( ctx, traceFetchRemoteArgs{ - baseRemotePartArgs: args, - Destination: targetPath, + Organization: args.Organization, + Location: args.Location, + Machine: args.Machine, + Part: args.Part, + Destination: tmp, }, globalArgs.Debug, logger, @@ -1384,7 +1401,10 @@ func traceImportRemoteAction(ctx *cli.Context, args baseRemotePartArgs) error { return err } - return traceImportLocalAction(ctx, importTracesFileArgs{Path: filepath.Join(targetPath, "traces")}) + return traceImportLocalAction(ctx, traceImportLocalArgs{ + Path: filepath.Join(tmp, "traces"), + Endpoint: args.Endpoint, + }) } // MachinesPartCopyFilesAction is the corresponding Action for 'machines part cp'. @@ -1507,7 +1527,7 @@ func (c *viamClient) machinesPartCopyFilesAction( func (c *viamClient) machinesPartGetFTDCAction( ctx *cli.Context, - flagArgs baseRemotePartArgs, + flagArgs machinesPartGetFTDCArgs, debug bool, logger logging.Logger, ) error { @@ -1566,7 +1586,7 @@ func (c *viamClient) machinesPartGetFTDCAction( return nil } -func (c *viamClient) machinesPartGetTracesAction( +func (c *viamClient) tracesFetchRemoteAction( ctx *cli.Context, flagArgs traceFetchRemoteArgs, debug bool, @@ -1607,14 +1627,14 @@ func (c *viamClient) machinesPartGetTracesAction( return err } if !quiet { - printf(ctx.App.Writer, "Done in %s.", time.Since(startTime)) + printf(ctx.App.Writer, "Download finished in %s.", time.Since(startTime)) } return nil } func tracePrintRemoteAction( ctx *cli.Context, - args baseRemotePartArgs, + args machinesPartGetFTDCArgs, ) error { client, err := newViamClient(ctx) if err != nil { @@ -1626,23 +1646,27 @@ func tracePrintRemoteAction( return err } logger := globalArgs.createLogger() - tmp, err := os.MkdirTemp("", "rdktraceimport") + tmp, err := os.MkdirTemp("", "viamtraceimport") if err != nil { return err } + //nolint: errcheck defer os.RemoveAll(tmp) - if err := client.machinesPartGetTracesAction( + if err := client.tracesFetchRemoteAction( ctx, traceFetchRemoteArgs{ - baseRemotePartArgs: args, - Destination: tmp, + Organization: args.Organization, + Location: args.Location, + Machine: args.Machine, + Part: args.Part, + Destination: tmp, }, globalArgs.Debug, logger, ); err != nil { return err } - return tracePrintLocalAction(ctx, importTracesFileArgs{Path: filepath.Join(tmp, "traces")}) + return tracePrintLocalAction(ctx, tracePrintLocalArgs{Path: filepath.Join(tmp, "traces")}) } func traceFetchRemoteAction(ctx *cli.Context, args traceFetchRemoteArgs) error { @@ -1657,12 +1681,12 @@ func traceFetchRemoteAction(ctx *cli.Context, args traceFetchRemoteArgs) error { } logger := globalArgs.createLogger() - return client.machinesPartGetTracesAction(ctx, args, globalArgs.Debug, logger) + return client.tracesFetchRemoteAction(ctx, args, globalArgs.Debug, logger) } func tracePrintLocalAction( ctx *cli.Context, - args importTracesFileArgs, + args tracePrintLocalArgs, ) error { traceFile, err := os.Open(args.Path) if err != nil { @@ -1693,7 +1717,7 @@ func tracePrintLocalAction( func traceImportLocalAction( ctx *cli.Context, - args importTracesFileArgs, + args traceImportLocalArgs, ) error { traceFile, err := os.Open(args.Path) if err != nil { @@ -1706,8 +1730,12 @@ func traceImportLocalAction( traceReader := protoutils.NewDelimitedProtoReader[tracepb.ResourceSpans](traceFile) //nolint: errcheck defer traceReader.Close() + endpoint := args.Endpoint + if endpoint == "" { + endpoint = "localhost:4317" + } otlpClient := otlptracegrpc.NewClient( - otlptracegrpc.WithEndpoint("localhost:4317"), + otlptracegrpc.WithEndpoint(endpoint), otlptracegrpc.WithInsecure(), ) if err := otlpClient.Start(ctx.Context); err != nil { diff --git a/cli/client_test.go b/cli/client_test.go index 34c16afb870..e8311577548 100644 --- a/cli/client_test.go +++ b/cli/client_test.go @@ -1224,7 +1224,7 @@ func TestShellGetFTDC(t *testing.T) { args := []string{"foo", "bar"} cCtx, viamClient, _, _ := setup(asc, nil, nil, partFlags, "token", args...) test.That(t, - viamClient.machinesPartGetFTDCAction(cCtx, parseStructFromCtx[baseRemotePartArgs](cCtx), true, logger), + viamClient.machinesPartGetFTDCAction(cCtx, parseStructFromCtx[machinesPartGetFTDCArgs](cCtx), true, logger), test.ShouldBeError, wrongNumArgsError{2, 0, 1}) }) @@ -1243,7 +1243,7 @@ func TestShellGetFTDC(t *testing.T) { cCtx, viamClient, _, _ := setupWithRunningPart( t, asc, nil, nil, partFlags, "token", partFqdn, args...) test.That(t, - viamClient.machinesPartGetFTDCAction(cCtx, parseStructFromCtx[baseRemotePartArgs](cCtx), true, logger), + viamClient.machinesPartGetFTDCAction(cCtx, parseStructFromCtx[machinesPartGetFTDCArgs](cCtx), true, logger), test.ShouldNotBeNil) entries, err := os.ReadDir(tempDir) @@ -1277,7 +1277,7 @@ func TestShellGetFTDC(t *testing.T) { cCtx, viamClient, _, _ := setupWithRunningPart( t, asc, nil, nil, partFlags, "token", partFqdn, args...) test.That(t, - viamClient.machinesPartGetFTDCAction(cCtx, parseStructFromCtx[baseRemotePartArgs](cCtx), true, logger), + viamClient.machinesPartGetFTDCAction(cCtx, parseStructFromCtx[machinesPartGetFTDCArgs](cCtx), true, logger), test.ShouldBeNil) entries, err := os.ReadDir(filepath.Join(targetPath, partID)) From 1e1db56c0252520661e1fa25bf9880f1168ca0bd Mon Sep 17 00:00:00 2001 From: Joshua Matthews Date: Wed, 3 Dec 2025 16:57:06 -0500 Subject: [PATCH 08/17] Split trace cli implementation into separate file --- cli/client.go | 236 ---------------------------------------------- cli/traces.go | 253 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 253 insertions(+), 236 deletions(-) create mode 100644 cli/traces.go diff --git a/cli/client.go b/cli/client.go index c1f3e3d4279..3769d21eeb0 100644 --- a/cli/client.go +++ b/cli/client.go @@ -5,7 +5,6 @@ import ( "bufio" "context" "encoding/json" - stderrors "errors" "fmt" "io" "io/fs" @@ -31,8 +30,6 @@ import ( "github.com/nathan-fiscaletti/consolesize-go" "github.com/pkg/errors" "github.com/urfave/cli/v2" - "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" - tracepb "go.opentelemetry.io/proto/otlp/trace/v1" "go.uber.org/multierr" "go.uber.org/zap" buildpb "go.viam.com/api/app/build/v1" @@ -45,7 +42,6 @@ import ( apppb "go.viam.com/api/app/v1" commonpb "go.viam.com/api/common/v1" "go.viam.com/utils" - "go.viam.com/utils/perf" "go.viam.com/utils/rpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/metadata" @@ -56,7 +52,6 @@ import ( rconfig "go.viam.com/rdk/config" "go.viam.com/rdk/grpc" "go.viam.com/rdk/logging" - "go.viam.com/rdk/protoutils" "go.viam.com/rdk/resource" "go.viam.com/rdk/robot/client" "go.viam.com/rdk/services/shell" @@ -86,7 +81,6 @@ const ( var ( errNoShellService = errors.New("shell service is not enabled on this machine part") ftdcPath = path.Join("~", ".viam", "diagnostics.data") - tracesPath = path.Join("~", ".viam", "trace") ) // viamClient wraps a cli.Context and provides all the CLI command functionality @@ -1328,31 +1322,6 @@ type machinesPartGetFTDCArgs struct { Part string } -type traceFetchRemoteArgs struct { - Organization string - Location string - Machine string - Part string - Destination string -} - -type traceImportRemoteArgs struct { - Organization string - Location string - Machine string - Part string - Endpoint string -} - -type traceImportLocalArgs struct { - Path string - Endpoint string -} - -type tracePrintLocalArgs struct { - Path string -} - // MachinesPartGetFTDCAction is the corresponding Action for 'machines part get-ftdc'. func MachinesPartGetFTDCAction(c *cli.Context, args machinesPartGetFTDCArgs) error { client, err := newViamClient(c) @@ -1369,44 +1338,6 @@ func MachinesPartGetFTDCAction(c *cli.Context, args machinesPartGetFTDCArgs) err return client.machinesPartGetFTDCAction(c, args, globalArgs.Debug, logger) } -func traceImportRemoteAction(ctx *cli.Context, args traceImportRemoteArgs) error { - client, err := newViamClient(ctx) - if err != nil { - return err - } - - globalArgs, err := getGlobalArgs(ctx) - if err != nil { - return err - } - logger := globalArgs.createLogger() - tmp, err := os.MkdirTemp("", "viamtraceimport") - if err != nil { - return err - } - //nolint: errcheck - defer os.RemoveAll(tmp) - if err := client.tracesFetchRemoteAction( - ctx, - traceFetchRemoteArgs{ - Organization: args.Organization, - Location: args.Location, - Machine: args.Machine, - Part: args.Part, - Destination: tmp, - }, - globalArgs.Debug, - logger, - ); err != nil { - return err - } - - return traceImportLocalAction(ctx, traceImportLocalArgs{ - Path: filepath.Join(tmp, "traces"), - Endpoint: args.Endpoint, - }) -} - // MachinesPartCopyFilesAction is the corresponding Action for 'machines part cp'. func MachinesPartCopyFilesAction(c *cli.Context, args machinesPartCopyFilesArgs) error { client, err := newViamClient(c) @@ -1586,173 +1517,6 @@ func (c *viamClient) machinesPartGetFTDCAction( return nil } -func (c *viamClient) tracesFetchRemoteAction( - ctx *cli.Context, - flagArgs traceFetchRemoteArgs, - debug bool, - logger logging.Logger, -) error { - part, err := c.robotPart(flagArgs.Organization, flagArgs.Location, flagArgs.Machine, flagArgs.Part) - if err != nil { - return err - } - // Intentional use of path instead of filepath: Windows understands both / and - // \ as path separators, and we don't want a cli running on Windows to send - // a path using \ to a *NIX machine. - src := path.Join(tracesPath, part.Id, "traces") - gArgs, err := getGlobalArgs(ctx) - quiet := err == nil && gArgs != nil && gArgs.Quiet - var startTime time.Time - if !quiet { - startTime = time.Now() - printf(ctx.App.Writer, "Saving to %s ...", path.Join(flagArgs.Destination, part.GetId())) - } - if err := c.copyFilesFromMachine( - flagArgs.Organization, - flagArgs.Location, - flagArgs.Machine, - flagArgs.Part, - debug, - true, - false, - []string{src}, - flagArgs.Destination, - logger, - ); err != nil { - if statusErr := status.Convert(err); statusErr != nil && - statusErr.Code() == codes.InvalidArgument && - statusErr.Message() == shell.ErrMsgDirectoryCopyRequestNoRecursion { - return errDirectoryCopyRequestNoRecursion - } - return err - } - if !quiet { - printf(ctx.App.Writer, "Download finished in %s.", time.Since(startTime)) - } - return nil -} - -func tracePrintRemoteAction( - ctx *cli.Context, - args machinesPartGetFTDCArgs, -) error { - client, err := newViamClient(ctx) - if err != nil { - return err - } - - globalArgs, err := getGlobalArgs(ctx) - if err != nil { - return err - } - logger := globalArgs.createLogger() - tmp, err := os.MkdirTemp("", "viamtraceimport") - if err != nil { - return err - } - //nolint: errcheck - defer os.RemoveAll(tmp) - if err := client.tracesFetchRemoteAction( - ctx, - traceFetchRemoteArgs{ - Organization: args.Organization, - Location: args.Location, - Machine: args.Machine, - Part: args.Part, - Destination: tmp, - }, - globalArgs.Debug, - logger, - ); err != nil { - return err - } - return tracePrintLocalAction(ctx, tracePrintLocalArgs{Path: filepath.Join(tmp, "traces")}) -} - -func traceFetchRemoteAction(ctx *cli.Context, args traceFetchRemoteArgs) error { - client, err := newViamClient(ctx) - if err != nil { - return err - } - - globalArgs, err := getGlobalArgs(ctx) - if err != nil { - return err - } - logger := globalArgs.createLogger() - - return client.tracesFetchRemoteAction(ctx, args, globalArgs.Debug, logger) -} - -func tracePrintLocalAction( - ctx *cli.Context, - args tracePrintLocalArgs, -) error { - traceFile, err := os.Open(args.Path) - if err != nil { - if os.IsNotExist(err) { - printf(ctx.App.Writer, "No traces found") - return nil - } - return errors.Wrap(err, "failed to open trace file") - } - traceReader := protoutils.NewDelimitedProtoReader[tracepb.ResourceSpans](traceFile) - //nolint: errcheck - defer traceReader.Close() - - devExporter := perf.NewOtelDevelopmentExporter() - if err := devExporter.Start(); err != nil { - return err - } - defer devExporter.Stop() - var msg tracepb.ResourceSpans - err = nil - for resource := range traceReader.AllWithMemory(&msg) { - for _, scope := range resource.ScopeSpans { - err = stderrors.Join(err, devExporter.ExportOTLPSpans(ctx.Context, scope.Spans)) - } - } - return err -} - -func traceImportLocalAction( - ctx *cli.Context, - args traceImportLocalArgs, -) error { - traceFile, err := os.Open(args.Path) - if err != nil { - if os.IsNotExist(err) { - printf(ctx.App.Writer, "No traces found") - return nil - } - return errors.Wrap(err, "failed to open trace file") - } - traceReader := protoutils.NewDelimitedProtoReader[tracepb.ResourceSpans](traceFile) - //nolint: errcheck - defer traceReader.Close() - endpoint := args.Endpoint - if endpoint == "" { - endpoint = "localhost:4317" - } - otlpClient := otlptracegrpc.NewClient( - otlptracegrpc.WithEndpoint(endpoint), - otlptracegrpc.WithInsecure(), - ) - if err := otlpClient.Start(ctx.Context); err != nil { - return err - } - //nolint: errcheck - defer otlpClient.Stop(ctx.Context) - var msg tracepb.ResourceSpans - for span := range traceReader.AllWithMemory(&msg) { - err := otlpClient.UploadTraces(ctx.Context, []*tracepb.ResourceSpans{span}) - if err != nil { - printf(ctx.App.Writer, "Error uploading trace: %v", err) - } - } - return nil -} - type robotsPartTunnelArgs struct { Organization string Location string diff --git a/cli/traces.go b/cli/traces.go new file mode 100644 index 00000000000..4d709bec29e --- /dev/null +++ b/cli/traces.go @@ -0,0 +1,253 @@ +package cli + +import ( + stderrors "errors" + "os" + "path" + "path/filepath" + "time" + + "github.com/pkg/errors" + "github.com/urfave/cli/v2" + "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" + tracepb "go.opentelemetry.io/proto/otlp/trace/v1" + "go.viam.com/utils/perf" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + "go.viam.com/rdk/logging" + "go.viam.com/rdk/protoutils" + "go.viam.com/rdk/services/shell" +) + +var tracesPath = path.Join("~", ".viam", "trace") + +type traceFetchRemoteArgs struct { + Organization string + Location string + Machine string + Part string + Destination string +} + +type traceImportRemoteArgs struct { + Organization string + Location string + Machine string + Part string + Endpoint string +} + +type traceImportLocalArgs struct { + Path string + Endpoint string +} + +type tracePrintLocalArgs struct { + Path string +} + +func traceImportRemoteAction(ctx *cli.Context, args traceImportRemoteArgs) error { + client, err := newViamClient(ctx) + if err != nil { + return err + } + + globalArgs, err := getGlobalArgs(ctx) + if err != nil { + return err + } + logger := globalArgs.createLogger() + tmp, err := os.MkdirTemp("", "viamtraceimport") + if err != nil { + return err + } + //nolint: errcheck + defer os.RemoveAll(tmp) + if err := client.tracesFetchRemoteAction( + ctx, + traceFetchRemoteArgs{ + Organization: args.Organization, + Location: args.Location, + Machine: args.Machine, + Part: args.Part, + Destination: tmp, + }, + globalArgs.Debug, + logger, + ); err != nil { + return err + } + + return traceImportLocalAction(ctx, traceImportLocalArgs{ + Path: filepath.Join(tmp, "traces"), + Endpoint: args.Endpoint, + }) +} + +func (c *viamClient) tracesFetchRemoteAction( + ctx *cli.Context, + flagArgs traceFetchRemoteArgs, + debug bool, + logger logging.Logger, +) error { + part, err := c.robotPart(flagArgs.Organization, flagArgs.Location, flagArgs.Machine, flagArgs.Part) + if err != nil { + return err + } + // Intentional use of path instead of filepath: Windows understands both / and + // \ as path separators, and we don't want a cli running on Windows to send + // a path using \ to a *NIX machine. + src := path.Join(tracesPath, part.Id, "traces") + gArgs, err := getGlobalArgs(ctx) + quiet := err == nil && gArgs != nil && gArgs.Quiet + var startTime time.Time + if !quiet { + startTime = time.Now() + printf(ctx.App.Writer, "Saving to %s ...", path.Join(flagArgs.Destination, part.GetId())) + } + if err := c.copyFilesFromMachine( + flagArgs.Organization, + flagArgs.Location, + flagArgs.Machine, + flagArgs.Part, + debug, + true, + false, + []string{src}, + flagArgs.Destination, + logger, + ); err != nil { + if statusErr := status.Convert(err); statusErr != nil && + statusErr.Code() == codes.InvalidArgument && + statusErr.Message() == shell.ErrMsgDirectoryCopyRequestNoRecursion { + return errDirectoryCopyRequestNoRecursion + } + return err + } + if !quiet { + printf(ctx.App.Writer, "Download finished in %s.", time.Since(startTime)) + } + return nil +} + +func tracePrintRemoteAction( + ctx *cli.Context, + args machinesPartGetFTDCArgs, +) error { + client, err := newViamClient(ctx) + if err != nil { + return err + } + + globalArgs, err := getGlobalArgs(ctx) + if err != nil { + return err + } + logger := globalArgs.createLogger() + tmp, err := os.MkdirTemp("", "viamtraceimport") + if err != nil { + return err + } + //nolint: errcheck + defer os.RemoveAll(tmp) + if err := client.tracesFetchRemoteAction( + ctx, + traceFetchRemoteArgs{ + Organization: args.Organization, + Location: args.Location, + Machine: args.Machine, + Part: args.Part, + Destination: tmp, + }, + globalArgs.Debug, + logger, + ); err != nil { + return err + } + return tracePrintLocalAction(ctx, tracePrintLocalArgs{Path: filepath.Join(tmp, "traces")}) +} + +func traceFetchRemoteAction(ctx *cli.Context, args traceFetchRemoteArgs) error { + client, err := newViamClient(ctx) + if err != nil { + return err + } + + globalArgs, err := getGlobalArgs(ctx) + if err != nil { + return err + } + logger := globalArgs.createLogger() + + return client.tracesFetchRemoteAction(ctx, args, globalArgs.Debug, logger) +} + +func tracePrintLocalAction( + ctx *cli.Context, + args tracePrintLocalArgs, +) error { + traceFile, err := os.Open(args.Path) + if err != nil { + if os.IsNotExist(err) { + printf(ctx.App.Writer, "No traces found") + return nil + } + return errors.Wrap(err, "failed to open trace file") + } + traceReader := protoutils.NewDelimitedProtoReader[tracepb.ResourceSpans](traceFile) + //nolint: errcheck + defer traceReader.Close() + + devExporter := perf.NewOtelDevelopmentExporter() + if err := devExporter.Start(); err != nil { + return err + } + defer devExporter.Stop() + var msg tracepb.ResourceSpans + err = nil + for resource := range traceReader.AllWithMemory(&msg) { + for _, scope := range resource.ScopeSpans { + err = stderrors.Join(err, devExporter.ExportOTLPSpans(ctx.Context, scope.Spans)) + } + } + return err +} + +func traceImportLocalAction( + ctx *cli.Context, + args traceImportLocalArgs, +) error { + traceFile, err := os.Open(args.Path) + if err != nil { + if os.IsNotExist(err) { + printf(ctx.App.Writer, "No traces found") + return nil + } + return errors.Wrap(err, "failed to open trace file") + } + traceReader := protoutils.NewDelimitedProtoReader[tracepb.ResourceSpans](traceFile) + //nolint: errcheck + defer traceReader.Close() + endpoint := args.Endpoint + if endpoint == "" { + endpoint = "localhost:4317" + } + otlpClient := otlptracegrpc.NewClient( + otlptracegrpc.WithEndpoint(endpoint), + otlptracegrpc.WithInsecure(), + ) + if err := otlpClient.Start(ctx.Context); err != nil { + return err + } + //nolint: errcheck + defer otlpClient.Stop(ctx.Context) + var msg tracepb.ResourceSpans + for span := range traceReader.AllWithMemory(&msg) { + err := otlpClient.UploadTraces(ctx.Context, []*tracepb.ResourceSpans{span}) + if err != nil { + printf(ctx.App.Writer, "Error uploading trace: %v", err) + } + } + return nil +} From 4b81f6a8af99b817498d11d9f545da700a4bd365 Mon Sep 17 00:00:00 2001 From: Joshua Matthews Date: Wed, 3 Dec 2025 17:03:18 -0500 Subject: [PATCH 09/17] Update goutils to 0.3.6 --- go.mod | 8 ++++---- go.sum | 16 ++++++++-------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/go.mod b/go.mod index 86c0216d7f9..46f68077d48 100644 --- a/go.mod +++ b/go.mod @@ -88,14 +88,14 @@ require ( go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.38.0 go.opentelemetry.io/otel/sdk v1.38.0 go.opentelemetry.io/otel/trace v1.38.0 - go.opentelemetry.io/proto/otlp v1.7.1 + go.opentelemetry.io/proto/otlp v1.9.0 go.uber.org/atomic v1.11.0 go.uber.org/goleak v1.3.0 go.uber.org/multierr v1.11.0 go.uber.org/zap v1.27.0 go.viam.com/api v0.1.496 go.viam.com/test v1.2.4 - go.viam.com/utils v0.3.5 + go.viam.com/utils v0.3.6 goji.io v2.0.2+incompatible golang.org/x/image v0.25.0 golang.org/x/mobile v0.0.0-20240112133503-c713f31d574b @@ -109,9 +109,9 @@ require ( gonum.org/v1/plot v0.15.2 google.golang.org/genproto/googleapis/api v0.0.0-20250825161204-c5933d9347a5 google.golang.org/genproto/googleapis/rpc v0.0.0-20250825161204-c5933d9347a5 - google.golang.org/grpc v1.75.0 + google.golang.org/grpc v1.75.1 google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.5.1 - google.golang.org/protobuf v1.36.8 + google.golang.org/protobuf v1.36.10 gopkg.in/natefinch/lumberjack.v2 v2.2.1 gopkg.in/src-d/go-billy.v4 v4.3.2 gorgonia.org/tensor v0.9.24 diff --git a/go.sum b/go.sum index a9328eb4af6..591baee18e6 100644 --- a/go.sum +++ b/go.sum @@ -1088,8 +1088,8 @@ go.opentelemetry.io/otel/sdk/metric v1.38.0/go.mod h1:dg9PBnW9XdQ1Hd6ZnRz689Cbtr go.opentelemetry.io/otel/trace v1.38.0 h1:Fxk5bKrDZJUH+AMyyIXGcFAPah0oRcT+LuNtJrmcNLE= go.opentelemetry.io/otel/trace v1.38.0/go.mod h1:j1P9ivuFsTceSWe1oY+EeW3sc+Pp42sO++GHkg4wwhs= go.opentelemetry.io/proto/otlp v0.7.0/go.mod h1:PqfVotwruBrMGOCsRd/89rSnXhoiJIqeYNgFYFoEGnI= -go.opentelemetry.io/proto/otlp v1.7.1 h1:gTOMpGDb0WTBOP8JaO72iL3auEZhVmAQg4ipjOVAtj4= -go.opentelemetry.io/proto/otlp v1.7.1/go.mod h1:b2rVh6rfI/s2pHWNlB7ILJcRALpcNDzKhACevjI+ZnE= +go.opentelemetry.io/proto/otlp v1.9.0 h1:l706jCMITVouPOqEnii2fIAuO3IVGBRPV5ICjceRb/A= +go.opentelemetry.io/proto/otlp v1.9.0/go.mod h1:xE+Cx5E/eEHw+ISFkwPLwCZefwVjY+pqKg1qcK03+/4= go.uber.org/atomic v1.3.2/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE= go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE= go.uber.org/atomic v1.5.0/go.mod h1:sABNBOSYdrvTF6hTgEIbc7YasKWGhgEQZyfxyTvoXHQ= @@ -1115,8 +1115,8 @@ go.viam.com/api v0.1.496 h1:gMaNoFipMJZcsV36ZTbwYqffnXnYbOGiXii2s3aOPjM= go.viam.com/api v0.1.496/go.mod h1:p/am76zx8SZ74V/F4rEAYQIpHaaLUwJgY2q3Uw3FIWk= go.viam.com/test v1.2.4 h1:JYgZhsuGAQ8sL9jWkziAXN9VJJiKbjoi9BsO33TW3ug= go.viam.com/test v1.2.4/go.mod h1:zI2xzosHdqXAJ/kFqcN+OIF78kQuTV2nIhGZ8EzvaJI= -go.viam.com/utils v0.3.5 h1:k1aVB0Vc5qFSCp9b9uYazcpvLJaYNjkqNAcvnyNPcjk= -go.viam.com/utils v0.3.5/go.mod h1:70GwXRZJzWYrWM3aWdMm+y+TzW5UU7OsEC4ibjo6Fug= +go.viam.com/utils v0.3.6 h1:hi/cBBu7TPkaq4dhmCKJrgEtZCfRIcrK0uvhIIdbu/I= +go.viam.com/utils v0.3.6/go.mod h1:uwGsYzJ3od5wnvizJd59oLSJiqrOqle/TC9m7JU6D7g= go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= go.yaml.in/yaml/v4 v4.0.0-rc.2 h1:/FrI8D64VSr4HtGIlUtlFMGsm7H7pWTbj6vOLVZcA6s= @@ -1489,8 +1489,8 @@ google.golang.org/grpc v1.33.1/go.mod h1:fr5YgcSWrqhRRxogOsw7RzIpsmvOZ6IcH4kBYTp google.golang.org/grpc v1.33.2/go.mod h1:JMHMWHQWaTccqQQlmk3MJZS+GWXOdAesneDmEnv2fbc= google.golang.org/grpc v1.36.0/go.mod h1:qjiiYl8FncCW8feJPdyg3v6XW24KsRHe+dy9BAGRRjU= google.golang.org/grpc v1.44.0/go.mod h1:k+4IHHFw41K8+bbowsex27ge2rCb65oeWqe4jJ590SU= -google.golang.org/grpc v1.75.0 h1:+TW+dqTd2Biwe6KKfhE5JpiYIBWq865PhKGSXiivqt4= -google.golang.org/grpc v1.75.0/go.mod h1:JtPAzKiq4v1xcAB2hydNlWI2RnF85XXcV0mhKXr2ecQ= +google.golang.org/grpc v1.75.1 h1:/ODCNEuf9VghjgO3rqLcfg8fiOP0nSluljWFlDxELLI= +google.golang.org/grpc v1.75.1/go.mod h1:JtPAzKiq4v1xcAB2hydNlWI2RnF85XXcV0mhKXr2ecQ= google.golang.org/grpc/cmd/protoc-gen-go-grpc v0.0.0-20200910201057-6591123024b3/go.mod h1:6Kw0yEErY5E/yWrBtf03jp27GLLJujG4z/JK95pnjjw= google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.5.1 h1:F29+wU6Ee6qgu9TddPgooOdaqsxTMunOoj8KA5yuS5A= google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.5.1/go.mod h1:5KF+wpkbTSbGcR9zteSqZV6fqFOWBl4Yde8En8MryZA= @@ -1508,8 +1508,8 @@ google.golang.org/protobuf v1.25.1-0.20200805231151-a709e31e5d12/go.mod h1:9JNX7 google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= google.golang.org/protobuf v1.33.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= -google.golang.org/protobuf v1.36.8 h1:xHScyCOEuuwZEc6UtSOvPbAT4zRh0xcNRYekJwfqyMc= -google.golang.org/protobuf v1.36.8/go.mod h1:fuxRtAxBytpl4zzqUh6/eyUujkJdNiuEkXntxiD/uRU= +google.golang.org/protobuf v1.36.10 h1:AYd7cD/uASjIL6Q9LiTjz8JLcrh/88q5UObnmY3aOOE= +google.golang.org/protobuf v1.36.10/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= From 62de9b674ac6215a5e3a01c3fc17dc0599bae928 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Thu, 4 Dec 2025 13:22:25 -0500 Subject: [PATCH 10/17] Fix typo Co-authored-by: Benjamin Rewis <32186188+benjirewis@users.noreply.github.com> --- cli/app.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/app.go b/cli/app.go index 1b5aa3cc6c7..a2636b1a2e2 100644 --- a/cli/app.go +++ b/cli/app.go @@ -515,7 +515,7 @@ var app = &cli.App{ }, { Name: "fetch-remote", - Usage: "Download a traces from a viam machine and save them to disk", + Usage: "Download traces from a viam machine and save them to disk", Flags: lo.Flatten([][]cli.Flag{ commonPartFlags, { From 5979391ad239ba71123a754cd717c5c10a4296eb Mon Sep 17 00:00:00 2001 From: Joshua Matthews Date: Thu, 4 Dec 2025 14:29:09 -0500 Subject: [PATCH 11/17] Add more help text to remote trace commands, s/fetch/get to match ftdc command naming --- cli/app.go | 21 ++++++++++++++++++--- cli/traces.go | 18 +++++++++--------- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/cli/app.go b/cli/app.go index a2636b1a2e2..dac4ab45345 100644 --- a/cli/app.go +++ b/cli/app.go @@ -484,7 +484,12 @@ var app = &cli.App{ Action: createCommandWithT(traceImportLocalAction), }, { - Name: "import-remote", + Name: "import-remote", + Description: ` +In order to use the import-remote command, the machine must have a valid shell type service. +Organization and location are required flags if using name (rather than ID) for the part. +Note: There is no progress meter while copying is in progress. +`, Usage: "Import traces from a remote viam machine to an OTLP endpoint.", Flags: lo.Flatten([][]cli.Flag{ commonOtlpFlags, @@ -508,14 +513,24 @@ var app = &cli.App{ { Name: "print-remote", Usage: "Print traces from a remote viam machine to the console", + Description: ` +In order to use the print-remote command, the machine must have a valid shell type service. +Organization and location are required flags if using name (rather than ID) for the part. +Note: There is no progress meter while copying is in progress. +`, Flags: lo.Flatten([][]cli.Flag{ commonPartFlags, }), Action: createCommandWithT(tracePrintRemoteAction), }, { - Name: "fetch-remote", Usage: "Download traces from a viam machine and save them to disk", + Name: "get-remote", + Description: ` +In order to use the get-remote command, the machine must have a valid shell type service. +Organization and location are required flags if using name (rather than ID) for the part. +Note: There is no progress meter while copying is in progress. +`, Flags: lo.Flatten([][]cli.Flag{ commonPartFlags, { @@ -526,7 +541,7 @@ var app = &cli.App{ }, }, }), - Action: createCommandWithT(traceFetchRemoteAction), + Action: createCommandWithT(traceGetRemoteAction), }, }, }, diff --git a/cli/traces.go b/cli/traces.go index 4d709bec29e..29acf4e3688 100644 --- a/cli/traces.go +++ b/cli/traces.go @@ -22,7 +22,7 @@ import ( var tracesPath = path.Join("~", ".viam", "trace") -type traceFetchRemoteArgs struct { +type traceGetRemoteArgs struct { Organization string Location string Machine string @@ -64,9 +64,9 @@ func traceImportRemoteAction(ctx *cli.Context, args traceImportRemoteArgs) error } //nolint: errcheck defer os.RemoveAll(tmp) - if err := client.tracesFetchRemoteAction( + if err := client.tracesGetRemoteAction( ctx, - traceFetchRemoteArgs{ + traceGetRemoteArgs{ Organization: args.Organization, Location: args.Location, Machine: args.Machine, @@ -85,9 +85,9 @@ func traceImportRemoteAction(ctx *cli.Context, args traceImportRemoteArgs) error }) } -func (c *viamClient) tracesFetchRemoteAction( +func (c *viamClient) tracesGetRemoteAction( ctx *cli.Context, - flagArgs traceFetchRemoteArgs, + flagArgs traceGetRemoteArgs, debug bool, logger logging.Logger, ) error { @@ -151,9 +151,9 @@ func tracePrintRemoteAction( } //nolint: errcheck defer os.RemoveAll(tmp) - if err := client.tracesFetchRemoteAction( + if err := client.tracesGetRemoteAction( ctx, - traceFetchRemoteArgs{ + traceGetRemoteArgs{ Organization: args.Organization, Location: args.Location, Machine: args.Machine, @@ -168,7 +168,7 @@ func tracePrintRemoteAction( return tracePrintLocalAction(ctx, tracePrintLocalArgs{Path: filepath.Join(tmp, "traces")}) } -func traceFetchRemoteAction(ctx *cli.Context, args traceFetchRemoteArgs) error { +func traceGetRemoteAction(ctx *cli.Context, args traceGetRemoteArgs) error { client, err := newViamClient(ctx) if err != nil { return err @@ -180,7 +180,7 @@ func traceFetchRemoteAction(ctx *cli.Context, args traceFetchRemoteArgs) error { } logger := globalArgs.createLogger() - return client.tracesFetchRemoteAction(ctx, args, globalArgs.Debug, logger) + return client.tracesGetRemoteAction(ctx, args, globalArgs.Debug, logger) } func tracePrintLocalAction( From ad3ad5ba16ecb51f147f01c485ba48690b34eea9 Mon Sep 17 00:00:00 2001 From: Joshua Matthews Date: Thu, 4 Dec 2025 14:57:39 -0500 Subject: [PATCH 12/17] Add tests for trace file download --- cli/traces_test.go | 134 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 cli/traces_test.go diff --git a/cli/traces_test.go b/cli/traces_test.go new file mode 100644 index 00000000000..3fb07bd701b --- /dev/null +++ b/cli/traces_test.go @@ -0,0 +1,134 @@ +package cli + +import ( + "context" + "maps" + "os" + "path/filepath" + "testing" + + "github.com/google/uuid" + apppb "go.viam.com/api/app/v1" + "go.viam.com/test" + "google.golang.org/grpc" + + "go.viam.com/rdk/logging" + shelltestutils "go.viam.com/rdk/services/shell/testutils" + "go.viam.com/rdk/testutils/inject" +) + +func TestTraceGetRemote(t *testing.T) { + logger := logging.NewTestLogger(t) + + listOrganizationsFunc := func(ctx context.Context, in *apppb.ListOrganizationsRequest, + opts ...grpc.CallOption, + ) (*apppb.ListOrganizationsResponse, error) { + orgs := []*apppb.Organization{{Name: "jedi", Id: uuid.NewString(), PublicNamespace: "anakin"}, {Name: "mandalorians"}} + return &apppb.ListOrganizationsResponse{Organizations: orgs}, nil + } + listLocationsFunc := func(ctx context.Context, in *apppb.ListLocationsRequest, + opts ...grpc.CallOption, + ) (*apppb.ListLocationsResponse, error) { + locs := []*apppb.Location{{Name: "naboo"}} + return &apppb.ListLocationsResponse{Locations: locs}, nil + } + listRobotsFunc := func(ctx context.Context, in *apppb.ListRobotsRequest, + opts ...grpc.CallOption, + ) (*apppb.ListRobotsResponse, error) { + robots := []*apppb.Robot{{Name: "r2d2"}} + return &apppb.ListRobotsResponse{Robots: robots}, nil + } + + partFqdn := uuid.NewString() + partID := uuid.NewString() + getRobotPartsFunc := func(ctx context.Context, in *apppb.GetRobotPartsRequest, + opts ...grpc.CallOption, + ) (*apppb.GetRobotPartsResponse, error) { + parts := []*apppb.RobotPart{{Name: "main", Fqdn: partFqdn, Id: partID}} + return &apppb.GetRobotPartsResponse{Parts: parts}, nil + } + + asc := &inject.AppServiceClient{ + ListOrganizationsFunc: listOrganizationsFunc, + ListLocationsFunc: listLocationsFunc, + ListRobotsFunc: listRobotsFunc, + GetRobotPartsFunc: getRobotPartsFunc, + } + + basePartFlags := map[string]any{ + "organization": "jedi", + "location": "naboo", + "robot": "r2d2", + "part": "main", + } + + tfs := shelltestutils.SetupTestFileSystem(t) + + t.Run("trace data does not exist", func(t *testing.T) { + tempDir := t.TempDir() + + output := t.TempDir() + testPartFlags := maps.Collect(maps.All(basePartFlags)) + testPartFlags["destination"] = output + + originalTracesPath := tracesPath + tracesPath = filepath.Join(tfs.Root, "FAKEDIR") + t.Cleanup(func() { + tracesPath = originalTracesPath + }) + + cCtx, viamClient, _, _ := setupWithRunningPart( + t, asc, nil, nil, testPartFlags, "token", partFqdn) + test.That(t, + viamClient.tracesGetRemoteAction(cCtx, parseStructFromCtx[traceGetRemoteArgs](cCtx), true, logger), + test.ShouldNotBeNil) + + entries, err := os.ReadDir(tempDir) + test.That(t, err, test.ShouldBeNil) + test.That(t, entries, test.ShouldHaveLength, 0) + }) + + t.Run("trace data exists", func(t *testing.T) { + tmpPartTracePath := filepath.Join(tfs.Root, partID) + err := os.Mkdir(tmpPartTracePath, 0o750) + test.That(t, err, test.ShouldBeNil) + t.Cleanup(func() { + err = os.RemoveAll(tmpPartTracePath) + test.That(t, err, test.ShouldBeNil) + }) + err = os.WriteFile(filepath.Join(tmpPartTracePath, "traces"), nil, 0o640) + test.That(t, err, test.ShouldBeNil) + originalTracePath := tracesPath + tracesPath = tfs.Root + t.Cleanup(func() { + tracesPath = originalTracePath + }) + + testDownload := func(t *testing.T, targetPath string) { + testFlags := maps.Collect(maps.All(basePartFlags)) + testFlags["destination"] = targetPath + + cCtx, viamClient, _, _ := setupWithRunningPart( + t, asc, nil, nil, testFlags, "token", partFqdn) + test.That(t, + viamClient.tracesGetRemoteAction(cCtx, parseStructFromCtx[traceGetRemoteArgs](cCtx), true, logger), + test.ShouldBeNil) + + entries, err := os.ReadDir(targetPath) + test.That(t, err, test.ShouldBeNil) + test.That(t, entries, test.ShouldHaveLength, 1) + traceFile := entries[0] + test.That(t, traceFile.Name(), test.ShouldEqual, "traces") + test.That(t, traceFile.IsDir(), test.ShouldBeFalse) + } + + t.Run("download to cwd", func(t *testing.T) { + tempDir := t.TempDir() + t.Chdir(tempDir) + testDownload(t, ".") + }) + t.Run("download to specified path", func(t *testing.T) { + testDownload(t, t.TempDir()) + }) + }) +} From f9c641078e8647fd68c100b90cad3adf28a0981a Mon Sep 17 00:00:00 2001 From: Joshua Matthews Date: Mon, 8 Dec 2025 10:28:13 -0500 Subject: [PATCH 13/17] Fix delimited proto reader silently dropping messages that exceed the default bufio.Scanner buffer max size --- protoutils/delimited_protos.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/protoutils/delimited_protos.go b/protoutils/delimited_protos.go index 780b5c0aaa3..33088894416 100644 --- a/protoutils/delimited_protos.go +++ b/protoutils/delimited_protos.go @@ -130,8 +130,15 @@ func (o *DelimitedProtoReader[T, M]) allWithMessageProvider(getMessage func() M) // yielded may be overwritten on subsequent iterations. If you need to use the // yieded []byte outside an iteration you must copy it somewhere else. func (o *RawDelimitedProtoReader) All() iter.Seq[[]byte] { + // 2 GiB, as defined by the protobuf spec + const protoMaxBytes = 1024 * 1024 * 1024 * 2 + // Max message size + 4 bytes for the length header + const bufferMaxSize = protoMaxBytes + 4 return func(yield func([]byte) bool) { scanner := bufio.NewScanner(o.reader) + // Start with no buffer and let bufio figure out the initial allocation + + // when it needs to be resized. + scanner.Buffer(nil, bufferMaxSize) scanner.Split(splitMessages) for scanner.Scan() { From 1202a35664420c6c3bb6dfba24935f1684e8a0db Mon Sep 17 00:00:00 2001 From: Joshua Matthews Date: Mon, 8 Dec 2025 10:53:54 -0500 Subject: [PATCH 14/17] Update trace commands that deal with output location to match get-ftdc command style, add more output --- cli/app.go | 53 +++++++--------------- cli/traces.go | 110 ++++++++++++++++++++++++++++++++++----------- cli/traces_test.go | 7 ++- 3 files changed, 102 insertions(+), 68 deletions(-) diff --git a/cli/app.go b/cli/app.go index dac4ab45345..3f0a58f0c1f 100644 --- a/cli/app.go +++ b/cli/app.go @@ -470,18 +470,11 @@ var app = &cli.App{ Usage: "Work with viam-server traces", Subcommands: []*cli.Command{ { - Name: "import-local", - Usage: "Import traces from a local viam server trace file to an OTLP endpoint.", - Flags: lo.Flatten([][]cli.Flag{ - {&cli.StringFlag{ - Name: "path", - TakesFile: true, - Required: true, - Usage: "path to file to import", - }}, - commonOtlpFlags, - }), - Action: createCommandWithT(traceImportLocalAction), + Name: "import-local", + Usage: "Import traces from a local viam server trace file to an OTLP endpoint.", + ArgsUsage: "", + Flags: commonOtlpFlags, + Action: createCommandWithT(traceImportLocalAction), }, { Name: "import-remote", @@ -498,17 +491,10 @@ Note: There is no progress meter while copying is in progress. Action: createCommandWithT(traceImportRemoteAction), }, { - Name: "print-local", - Usage: "Print traces in a local file to the console", - Flags: []cli.Flag{ - &cli.StringFlag{ - Name: "path", - TakesFile: true, - Required: true, - Usage: "path to file to import", - }, - }, - Action: createCommandWithT(tracePrintLocalAction), + Name: "print-local", + Usage: "Print traces in a local file to the console", + ArgsUsage: "", + Action: createCommandWithT(tracePrintLocalAction), }, { Name: "print-remote", @@ -518,29 +504,20 @@ In order to use the print-remote command, the machine must have a valid shell ty Organization and location are required flags if using name (rather than ID) for the part. Note: There is no progress meter while copying is in progress. `, - Flags: lo.Flatten([][]cli.Flag{ - commonPartFlags, - }), + Flags: commonPartFlags, Action: createCommandWithT(tracePrintRemoteAction), }, { - Usage: "Download traces from a viam machine and save them to disk", - Name: "get-remote", + Usage: "Download traces from a viam machine and save them to disk", + Name: "get-remote", + ArgsUsage: "[target]", Description: ` In order to use the get-remote command, the machine must have a valid shell type service. Organization and location are required flags if using name (rather than ID) for the part. +If [target] is not specified then the traces file will be saved to the current working directory. Note: There is no progress meter while copying is in progress. `, - Flags: lo.Flatten([][]cli.Flag{ - commonPartFlags, - { - &cli.PathFlag{ - Name: generalFlagDestination, - Required: true, - Usage: "output directory for downloaded traces", - }, - }, - }), + Flags: commonPartFlags, Action: createCommandWithT(traceGetRemoteAction), }, }, diff --git a/cli/traces.go b/cli/traces.go index 29acf4e3688..1078be03ebb 100644 --- a/cli/traces.go +++ b/cli/traces.go @@ -27,7 +27,6 @@ type traceGetRemoteArgs struct { Location string Machine string Part string - Destination string } type traceImportRemoteArgs struct { @@ -39,14 +38,9 @@ type traceImportRemoteArgs struct { } type traceImportLocalArgs struct { - Path string Endpoint string } -type tracePrintLocalArgs struct { - Path string -} - func traceImportRemoteAction(ctx *cli.Context, args traceImportRemoteArgs) error { client, err := newViamClient(ctx) if err != nil { @@ -71,23 +65,27 @@ func traceImportRemoteAction(ctx *cli.Context, args traceImportRemoteArgs) error Location: args.Location, Machine: args.Machine, Part: args.Part, - Destination: tmp, }, + tmp, + false, globalArgs.Debug, logger, ); err != nil { return err } - return traceImportLocalAction(ctx, traceImportLocalArgs{ - Path: filepath.Join(tmp, "traces"), + return traceImportLocal(ctx, traceImportLocalArgs{ Endpoint: args.Endpoint, - }) + }, + filepath.Join(tmp, "traces"), + ) } func (c *viamClient) tracesGetRemoteAction( ctx *cli.Context, flagArgs traceGetRemoteArgs, + target string, + getAll bool, debug bool, logger logging.Logger, ) error { @@ -98,13 +96,18 @@ func (c *viamClient) tracesGetRemoteAction( // Intentional use of path instead of filepath: Windows understands both / and // \ as path separators, and we don't want a cli running on Windows to send // a path using \ to a *NIX machine. - src := path.Join(tracesPath, part.Id, "traces") + src := path.Join(tracesPath, part.Id) + // if getAll is set then download the entire directory, including rotated + // files. Otherwise just get the current file. + if !getAll { + src = path.Join(src, "traces") + } gArgs, err := getGlobalArgs(ctx) quiet := err == nil && gArgs != nil && gArgs.Quiet var startTime time.Time if !quiet { startTime = time.Now() - printf(ctx.App.Writer, "Saving to %s ...", path.Join(flagArgs.Destination, part.GetId())) + printf(ctx.App.Writer, "Saving to %s ...", path.Join(target)) } if err := c.copyFilesFromMachine( flagArgs.Organization, @@ -115,7 +118,7 @@ func (c *viamClient) tracesGetRemoteAction( true, false, []string{src}, - flagArgs.Destination, + target, logger, ); err != nil { if statusErr := status.Convert(err); statusErr != nil && @@ -133,7 +136,7 @@ func (c *viamClient) tracesGetRemoteAction( func tracePrintRemoteAction( ctx *cli.Context, - args machinesPartGetFTDCArgs, + args traceGetRemoteArgs, ) error { client, err := newViamClient(ctx) if err != nil { @@ -153,22 +156,45 @@ func tracePrintRemoteAction( defer os.RemoveAll(tmp) if err := client.tracesGetRemoteAction( ctx, - traceGetRemoteArgs{ - Organization: args.Organization, - Location: args.Location, - Machine: args.Machine, - Part: args.Part, - Destination: tmp, - }, + args, + tmp, + false, globalArgs.Debug, logger, ); err != nil { return err } - return tracePrintLocalAction(ctx, tracePrintLocalArgs{Path: filepath.Join(tmp, "traces")}) + return tracePrintLocal(ctx, filepath.Join(tmp, "traces")) +} + +func getSingularArg(ctx *cli.Context) (string, error) { + cliArgs := ctx.Args().Slice() + var result string + switch numArgs := len(cliArgs); numArgs { + case 1: + result = cliArgs[0] + default: + return "", wrongNumArgsError{have: numArgs, min: 1} + } + return result, nil } func traceGetRemoteAction(ctx *cli.Context, args traceGetRemoteArgs) error { + cliArgs := ctx.Args().Slice() + var targetPath string + switch numArgs := len(cliArgs); numArgs { + case 0: + var err error + targetPath, err = os.Getwd() + if err != nil { + return err + } + case 1: + targetPath = cliArgs[0] + default: + return wrongNumArgsError{numArgs, 0, 1} + } + client, err := newViamClient(ctx) if err != nil { return err @@ -180,14 +206,26 @@ func traceGetRemoteAction(ctx *cli.Context, args traceGetRemoteArgs) error { } logger := globalArgs.createLogger() - return client.tracesGetRemoteAction(ctx, args, globalArgs.Debug, logger) + return client.tracesGetRemoteAction(ctx, args, targetPath, true, globalArgs.Debug, logger) } func tracePrintLocalAction( ctx *cli.Context, - args tracePrintLocalArgs, + _ struct{}, +) error { + target, err := getSingularArg(ctx) + if err != nil { + return err + } + return tracePrintLocal(ctx, target) +} + +func tracePrintLocal( + ctx *cli.Context, + source string, ) error { - traceFile, err := os.Open(args.Path) + //nolint: gosec + traceFile, err := os.Open(source) if err != nil { if os.IsNotExist(err) { printf(ctx.App.Writer, "No traces found") @@ -218,7 +256,20 @@ func traceImportLocalAction( ctx *cli.Context, args traceImportLocalArgs, ) error { - traceFile, err := os.Open(args.Path) + target, err := getSingularArg(ctx) + if err != nil { + return err + } + return traceImportLocal(ctx, args, target) +} + +func traceImportLocal( + ctx *cli.Context, + args traceImportLocalArgs, + source string, +) error { + //nolint: gosec + traceFile, err := os.Open(source) if err != nil { if os.IsNotExist(err) { printf(ctx.App.Writer, "No traces found") @@ -243,11 +294,18 @@ func traceImportLocalAction( //nolint: errcheck defer otlpClient.Stop(ctx.Context) var msg tracepb.ResourceSpans + msgSuccess := 0 + msgTotal := 0 + printf(ctx.App.Writer, "Importing spans to %v...", endpoint) for span := range traceReader.AllWithMemory(&msg) { + msgTotal++ err := otlpClient.UploadTraces(ctx.Context, []*tracepb.ResourceSpans{span}) if err != nil { printf(ctx.App.Writer, "Error uploading trace: %v", err) + } else { + msgSuccess++ } } + printf(ctx.App.Writer, "Imported %d/%d messages", msgSuccess, msgTotal) return nil } diff --git a/cli/traces_test.go b/cli/traces_test.go index 3fb07bd701b..47803efe87f 100644 --- a/cli/traces_test.go +++ b/cli/traces_test.go @@ -69,7 +69,6 @@ func TestTraceGetRemote(t *testing.T) { output := t.TempDir() testPartFlags := maps.Collect(maps.All(basePartFlags)) - testPartFlags["destination"] = output originalTracesPath := tracesPath tracesPath = filepath.Join(tfs.Root, "FAKEDIR") @@ -80,7 +79,7 @@ func TestTraceGetRemote(t *testing.T) { cCtx, viamClient, _, _ := setupWithRunningPart( t, asc, nil, nil, testPartFlags, "token", partFqdn) test.That(t, - viamClient.tracesGetRemoteAction(cCtx, parseStructFromCtx[traceGetRemoteArgs](cCtx), true, logger), + viamClient.tracesGetRemoteAction(cCtx, parseStructFromCtx[traceGetRemoteArgs](cCtx), output, true, true, logger), test.ShouldNotBeNil) entries, err := os.ReadDir(tempDir) @@ -106,12 +105,12 @@ func TestTraceGetRemote(t *testing.T) { testDownload := func(t *testing.T, targetPath string) { testFlags := maps.Collect(maps.All(basePartFlags)) - testFlags["destination"] = targetPath + output := t.TempDir() cCtx, viamClient, _, _ := setupWithRunningPart( t, asc, nil, nil, testFlags, "token", partFqdn) test.That(t, - viamClient.tracesGetRemoteAction(cCtx, parseStructFromCtx[traceGetRemoteArgs](cCtx), true, logger), + viamClient.tracesGetRemoteAction(cCtx, parseStructFromCtx[traceGetRemoteArgs](cCtx), output, true, true, logger), test.ShouldBeNil) entries, err := os.ReadDir(targetPath) From d222d8eccfc0cc7d0e231ec319759c87df2e408d Mon Sep 17 00:00:00 2001 From: Joshua Matthews Date: Mon, 8 Dec 2025 17:00:39 -0500 Subject: [PATCH 15/17] Attempt to fix compilation on 32-bit --- protoutils/delimited_protos.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/protoutils/delimited_protos.go b/protoutils/delimited_protos.go index 33088894416..f9da3566a35 100644 --- a/protoutils/delimited_protos.go +++ b/protoutils/delimited_protos.go @@ -5,6 +5,7 @@ import ( "encoding/binary" "io" "iter" + "math" "google.golang.org/protobuf/proto" ) @@ -134,11 +135,13 @@ func (o *RawDelimitedProtoReader) All() iter.Seq[[]byte] { const protoMaxBytes = 1024 * 1024 * 1024 * 2 // Max message size + 4 bytes for the length header const bufferMaxSize = protoMaxBytes + 4 + // Fall back to max int size if necessary so the 32-bit tests pass. + const realMaxSize = min(bufferMaxSize, math.MaxInt) return func(yield func([]byte) bool) { scanner := bufio.NewScanner(o.reader) // Start with no buffer and let bufio figure out the initial allocation + // when it needs to be resized. - scanner.Buffer(nil, bufferMaxSize) + scanner.Buffer(nil, realMaxSize) scanner.Split(splitMessages) for scanner.Scan() { From 746a6fb4d01a3eecd03089fbf8869c7a9ddd0687 Mon Sep 17 00:00:00 2001 From: Joshua Matthews Date: Mon, 8 Dec 2025 17:39:41 -0500 Subject: [PATCH 16/17] Fix cli tests --- cli/traces_test.go | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/cli/traces_test.go b/cli/traces_test.go index 47803efe87f..556b1370c6b 100644 --- a/cli/traces_test.go +++ b/cli/traces_test.go @@ -95,7 +95,8 @@ func TestTraceGetRemote(t *testing.T) { err = os.RemoveAll(tmpPartTracePath) test.That(t, err, test.ShouldBeNil) }) - err = os.WriteFile(filepath.Join(tmpPartTracePath, "traces"), nil, 0o640) + testData := []byte("test") + err = os.WriteFile(filepath.Join(tmpPartTracePath, "traces"), testData, 0o640) test.That(t, err, test.ShouldBeNil) originalTracePath := tracesPath tracesPath = tfs.Root @@ -103,31 +104,38 @@ func TestTraceGetRemote(t *testing.T) { tracesPath = originalTracePath }) - testDownload := func(t *testing.T, targetPath string) { + t.Run("only recent", func(t *testing.T) { testFlags := maps.Collect(maps.All(basePartFlags)) output := t.TempDir() cCtx, viamClient, _, _ := setupWithRunningPart( t, asc, nil, nil, testFlags, "token", partFqdn) test.That(t, - viamClient.tracesGetRemoteAction(cCtx, parseStructFromCtx[traceGetRemoteArgs](cCtx), output, true, true, logger), + viamClient.tracesGetRemoteAction(cCtx, parseStructFromCtx[traceGetRemoteArgs](cCtx), output, false, true, logger), test.ShouldBeNil) - entries, err := os.ReadDir(targetPath) + contents, err := os.ReadFile(filepath.Join(output, "traces")) test.That(t, err, test.ShouldBeNil) - test.That(t, entries, test.ShouldHaveLength, 1) - traceFile := entries[0] - test.That(t, traceFile.Name(), test.ShouldEqual, "traces") - test.That(t, traceFile.IsDir(), test.ShouldBeFalse) - } - - t.Run("download to cwd", func(t *testing.T) { - tempDir := t.TempDir() - t.Chdir(tempDir) - testDownload(t, ".") + test.That(t, contents, test.ShouldResemble, testData) }) - t.Run("download to specified path", func(t *testing.T) { - testDownload(t, t.TempDir()) + t.Run("full directory", func(t *testing.T) { + testFlags := maps.Collect(maps.All(basePartFlags)) + output := t.TempDir() + + cCtx, viamClient, _, _ := setupWithRunningPart( + t, asc, nil, nil, testFlags, "token", partFqdn) + test.That(t, + viamClient.tracesGetRemoteAction(cCtx, parseStructFromCtx[traceGetRemoteArgs](cCtx), output, true, true, logger), + test.ShouldBeNil) + + contents, err := os.ReadDir(output) + test.That(t, err, test.ShouldBeNil) + test.That(t, contents, test.ShouldHaveLength, 1) + subdir := contents[0] + + fileContents, err := os.ReadFile(filepath.Join(output, subdir.Name(), "traces")) + test.That(t, err, test.ShouldBeNil) + test.That(t, fileContents, test.ShouldResemble, testData) }) }) } From 048db332e668354f9c8d5bafd86b5b40fe411761 Mon Sep 17 00:00:00 2001 From: Joshua Matthews Date: Tue, 9 Dec 2025 11:16:47 -0500 Subject: [PATCH 17/17] Add usage text to traces commands --- cli/app.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/cli/app.go b/cli/app.go index 3f0a58f0c1f..76e2d7e79ae 100644 --- a/cli/app.go +++ b/cli/app.go @@ -466,12 +466,14 @@ var app = &cli.App{ }, Commands: []*cli.Command{ { - Name: "traces", - Usage: "Work with viam-server traces", + Name: "traces", + Usage: "Work with viam-server traces", + UsageText: createUsageText("traces", nil, false, true), Subcommands: []*cli.Command{ { Name: "import-local", Usage: "Import traces from a local viam server trace file to an OTLP endpoint.", + UsageText: createUsageText("traces import-local", nil, true, false, ""), ArgsUsage: "", Flags: commonOtlpFlags, Action: createCommandWithT(traceImportLocalAction), @@ -483,7 +485,8 @@ In order to use the import-remote command, the machine must have a valid shell t Organization and location are required flags if using name (rather than ID) for the part. Note: There is no progress meter while copying is in progress. `, - Usage: "Import traces from a remote viam machine to an OTLP endpoint.", + Usage: "Import traces from a remote viam machine to an OTLP endpoint.", + UsageText: createUsageText("traces import-remote", []string{generalFlagPart}, true, false), Flags: lo.Flatten([][]cli.Flag{ commonOtlpFlags, commonPartFlags, @@ -493,12 +496,14 @@ Note: There is no progress meter while copying is in progress. { Name: "print-local", Usage: "Print traces in a local file to the console", + UsageText: createUsageText("traces print-local", nil, true, false, ""), ArgsUsage: "", Action: createCommandWithT(tracePrintLocalAction), }, { - Name: "print-remote", - Usage: "Print traces from a remote viam machine to the console", + Name: "print-remote", + Usage: "Print traces from a remote viam machine to the console", + UsageText: createUsageText("traces print-remote", []string{generalFlagPart}, true, false), Description: ` In order to use the print-remote command, the machine must have a valid shell type service. Organization and location are required flags if using name (rather than ID) for the part. @@ -508,8 +513,9 @@ Note: There is no progress meter while copying is in progress. Action: createCommandWithT(tracePrintRemoteAction), }, { - Usage: "Download traces from a viam machine and save them to disk", Name: "get-remote", + Usage: "Download traces from a viam machine and save them to disk", + UsageText: createUsageText("traces get-remote", []string{generalFlagPart}, true, false, "[target]"), ArgsUsage: "[target]", Description: ` In order to use the get-remote command, the machine must have a valid shell type service.