Skip to content

Conversation

@jmatth
Copy link
Member

@jmatth jmatth commented Dec 1, 2025

This adds a traces command to the cli with various subcommands to display or import trace files produced by viam-server.

Base automatically changed from trace-file to main December 3, 2025 19:27
@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label Dec 3, 2025
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Dec 3, 2025
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Dec 3, 2025
@jmatth jmatth requested a review from benjirewis December 3, 2025 21:57
@jmatth jmatth marked this pull request as ready for review December 3, 2025 21:57
@jmatth jmatth requested a review from a team as a code owner December 3, 2025 21:57
@jmatth jmatth requested review from lia-viam and njooma December 3, 2025 21:57
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Dec 3, 2025
Co-authored-by: Benjamin Rewis <[email protected]>
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Dec 4, 2025
@viambot viambot removed the safe to test This pull request is marked safe to test from a trusted zone label Dec 4, 2025
@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label Dec 4, 2025
@jmatth jmatth requested a review from benjirewis December 4, 2025 20:33
Copy link
Member

@benjirewis benjirewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty much LGTM % the remaining note about defaulting to the current directory for the destination file.

@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Dec 8, 2025
@jmatth jmatth requested a review from benjirewis December 8, 2025 15:54
Copy link
Member

@benjirewis benjirewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks.

@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Dec 8, 2025
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Dec 8, 2025
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Dec 8, 2025
@jmatth
Copy link
Member Author

jmatth commented Dec 9, 2025

Ping @njooma and @lia-viam : it looks like you're required reviewers because of the codeowners file

@stuqdog
Copy link
Member

stuqdog commented Dec 9, 2025

Ping @njooma and @lia-viam : it looks like you're required reviewers because of the codeowners file

I'll review on behalf of team SDK, sorry for the delay!

Copy link
Member

@stuqdog stuqdog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A minor request to use the createUsageText helper (no need to rerequest review), otherwise lgtm!

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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to use createUsageText helper here (and in the other new methods) to get consistent usage text formatting. Also because currently, the usage text on viam traces get-remote --help includes USAGE: viam traces get-remote [command options] [target], but it isn't clear from this text that part is in fact a required field.

@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Dec 9, 2025
@jmatth jmatth merged commit 1b6bf8e into main Dec 9, 2025
19 checks passed
@jmatth jmatth deleted the trace-cli branch December 9, 2025 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test This pull request is marked safe to test from a trusted zone

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants