Improve tsh debug messages with client version output by kshi36 · Pull Request #60297 · gravitational/teleport

kshi36 · 2025-10-15T23:32:27Z

Fixes #55738.
Added tsh client version to improve tsh command debugging, eg. client version mismatch.

Changelog: Updated tsh debug output to include tsh client version when --debug flag is set.

ravicious · 2025-10-16T08:51:56Z

+	// print tsh version when --debug flag is set
+	// to diagnose potential client version mismatch
+	if cf.Debug && command != ver.FullCommand() {
+		modules.GetModules().PrintVersion()


This prints to stdout, but on macOS tsh can be configured to log to os_log.

Instead of logging the version to stdout, I think a better approach might be to make sure that initLogger was already called twice (it was at this point in the Run function) and use logger.DebugContext instead of printing to stdout. This will make the logger print to whatever destination it is set, in case we add more destinations in the future.

This would require extending modules with something like GetVersionString. It unfortunately complicates the whole implementation, as adjusting the Modules interface means that you first have to add the relevant method to the enterprise Modules in the other repo, then update the e ref in the OSS repo and only then merge the OSS change which uses the new method. Otherwise merging just the OSS change would end up breaking the enterprise build.

https://github.com/gravitational/teleport.e?tab=readme-ov-file#working-with-submodules

I know it's much more complicated than what you initially wanted to do here. But I think it'll also be a good introduction to the peculiarities of the enterprise repo and how to work with it. :~)

I think it'd also be fine to merge this first and then make it use the logger in another PR!

I think we should log directly instead of using the modules function to print the version here.

That aligns better with what we already do in tbot, see #58033.

ravicious · 2025-10-16T08:53:21Z

+	// print tsh version when --debug flag is set
+	// to diagnose potential client version mismatch
+	if cf.Debug && command != ver.FullCommand() {
+		modules.GetModules().PrintVersion()


I think it'd also be fine to merge this first and then make it use the logger in another PR!

bernardjkim · 2025-10-16T16:31:41Z

Regarding the failed Unit Tests, it looks like it is most likely unrelated to your changes and is the result of a flaky test failure. The error message looks like the same one as seen in #32958.

In this case you can re-run the unit tests to pass the check. If the flaky test wasn't already being tracked, it'd also be helpful to create a GitHub issue to track it.

bernardjkim · 2025-10-16T18:24:48Z

My mistake, after taking a closer look at the error messages, I think the code change may actually be causing this particular test case to fail. So we'll probably need to go with the above suggestions and avoid writing to stdout here.

Messages:   	exit code: 2
        	            	stderr: protocol version mismatch -- is your shell clean?
        	            	(see the rsync manpage for an explanation)
        	            	rsync error: protocol incompatibility (code 2) at compat.c(622) [sender=3.2.7]

The rsync man page documents this under their Diagnostics section:

rsync occasionally produces error messages that may seem a little cryptic. The one that seems to cause the most confusion is lqprotocol version mismatch -- is your shell clean?rq.

This message is usually caused by your startup scripts or remote shell facility producing unwanted garbage on the stream that rsync is using for its transport.

rosstimothy

Looks good other than one small suggestion. Happy first commit @kshi36 🎉 !

rosstimothy · 2025-10-17T13:01:27Z

+	// print tsh version when --debug flag is set
+	// to diagnose potential client version mismatch
+	if cf.Debug && command != ver.FullCommand() {
+		logger.InfoContext(ctx, "Debugging tsh client",


I think we should alter the message to match what tbot uses.

Suggested change

logger.InfoContext(ctx, "Debugging tsh client",

logger.InfoContext(ctx, "Initializing tsh",

r0mant

Congrats on the first commit @kshi36! Looks good to me as well but can you please add test coverage before merging as we discussed yesterday?

r0mant

lgtm with one more suggestion

r0mant · 2025-10-17T19:36:39Z

+
+	output, err := exec.Command(testExecutable, "logout", "--debug").CombinedOutput()
+	require.NoError(t, err)
+	require.Contains(t, string(output), "Initializing tsh version")


Nit: Can we check that the output contains versions as well?

backport-bot-workflows · 2025-10-18T02:51:59Z

@kshi36 See the table below for backport results.

Branch	Result
branch/v18	Create PR

* Added client version output when --debug flag is added to tsh commands * Moved client version output to logger * Altered tsh client version output to match tbot's, added test coverage * Updated test to check versions in output

* Added client version output when --debug flag is added to tsh commands * Moved client version output to logger * Altered tsh client version output to match tbot's, added test coverage * Updated test to check versions in output Co-authored-by: Kevin <76608931+kshi36@users.noreply.github.com>

* Added client version output when --debug flag is added to tsh commands * Moved client version output to logger * Altered tsh client version output to match tbot's, added test coverage * Updated test to check versions in output

kshi36 and others added 2 commits October 15, 2025 15:53

Added client version output when --debug flag is added to tsh commands

7ca3fb8

Merge branch 'master' into kevin/tsh-debug-output-version

767759e

github-actions bot added size/sm tsh tsh - Teleport's command line tool for logging into nodes running Teleport. labels Oct 15, 2025

github-actions bot requested review from klizhentas and zmb3 October 15, 2025 23:32

r0mant requested review from bernardjkim and r0mant and removed request for klizhentas and zmb3 October 16, 2025 01:58

ravicious reviewed Oct 16, 2025

View reviewed changes

ravicious approved these changes Oct 16, 2025

View reviewed changes

r0mant added the backport/branch/v18 label Oct 16, 2025

Moved client version output to logger

a088820

rosstimothy approved these changes Oct 17, 2025

View reviewed changes

public-teleport-github-review-bot bot removed request for bernardjkim and r0mant October 17, 2025 13:03

r0mant reviewed Oct 17, 2025

View reviewed changes

Altered tsh client version output to match tbot's, added test coverage

c27b38d

kshi36 requested a review from r0mant October 17, 2025 19:13

r0mant approved these changes Oct 17, 2025

View reviewed changes

bernardjkim approved these changes Oct 17, 2025

View reviewed changes

Updated test to check versions in output

ec19c44

kshi36 added this pull request to the merge queue Oct 18, 2025

Merged via the queue into master with commit d90e4ea Oct 18, 2025
41 checks passed

kshi36 deleted the kevin/tsh-debug-output-version branch October 18, 2025 02:50

kshi36 mentioned this pull request Oct 20, 2025

[v18] Improve tsh debug messages with client version output #60407

Merged

codingllama mentioned this pull request Nov 19, 2025

[v17] Improve tsh debug messages with client version output #61526

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve tsh debug messages with client version output#60297

Improve tsh debug messages with client version output#60297
kshi36 merged 5 commits intomasterfrom
kevin/tsh-debug-output-version

kshi36 commented Oct 15, 2025

Uh oh!

ravicious Oct 16, 2025

Uh oh!

ravicious Oct 16, 2025

Uh oh!

rosstimothy Oct 16, 2025

Uh oh!

ravicious Oct 16, 2025

Uh oh!

bernardjkim commented Oct 16, 2025

Uh oh!

bernardjkim commented Oct 16, 2025 •

edited

Loading

Uh oh!

rosstimothy left a comment

Uh oh!

rosstimothy Oct 17, 2025

Uh oh!

r0mant left a comment

Uh oh!

r0mant left a comment

Uh oh!

r0mant Oct 17, 2025

Uh oh!

Uh oh!

backport-bot-workflows bot commented Oct 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	logger.InfoContext(ctx, "Debugging tsh client",
	logger.InfoContext(ctx, "Initializing tsh",

Conversation

kshi36 commented Oct 15, 2025

Uh oh!

ravicious Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

ravicious Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

rosstimothy Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

ravicious Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

bernardjkim commented Oct 16, 2025

Uh oh!

bernardjkim commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rosstimothy left a comment

Choose a reason for hiding this comment

Uh oh!

rosstimothy Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

r0mant left a comment

Choose a reason for hiding this comment

Uh oh!

r0mant left a comment

Choose a reason for hiding this comment

Uh oh!

r0mant Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

backport-bot-workflows bot commented Oct 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bernardjkim commented Oct 16, 2025 •

edited

Loading