Add --trace flag to CLI commands #406

fyrchik · 2023-05-29T09:44:19Z

fyrchik commented

2023-05-29 09:44:19 +00:00

The task proved to be more difficult than I initially thought, I am not yet satisfied with the solution, thus WIP.

Ideally, we would like automatically have --trace flag for all commands which use --rpc-endpoint (or --endpoint for a control service) flag.
This is not completely feasible:

Because we exit with an OS syscall, trace id won't be printed on command errors, but this could be sth we want to do (e.g. commands failed with the timeout).
Some commands use JSON output, we need to take this into account.

So the current solution is to have 2 functions: start and print trace. Their inner logic depends on whether --trace flag was provided.
If a command has tracing enabled, it MUST register these handlers in Pre/Post run functions.

May be it is better to defer in every function which prints trace: this way we can have more fine-graineg control.
Another option is to cover on a case-by-case basis, but I see this flag useful and "polished" only if done simultaneously for all commands, you don't always new what you want to trace.

The task proved to be more difficult than I initially thought, I am not yet satisfied with the solution, thus WIP. Ideally, we would like automatically have `--trace` flag for all commands which use `--rpc-endpoint` (or `--endpoint` for a control service) flag. This is not completely feasible: 1. Because we exit with an OS syscall, trace id won't be printed on command errors, but this _could_ be sth we want to do (e.g. commands failed with the timeout). 2. Some commands use JSON output, we need to take this into account. So the current solution is to have 2 functions: start and print trace. Their inner logic depends on whether --trace flag was provided. If a command has tracing enabled, it MUST register these handlers in Pre/Post run functions. May be it is better to defer in every function which prints trace: this way we can have more fine-graineg control. Another option is to cover on a case-by-case basis, but I see this flag useful and "polished" only if done simultaneously for all commands, you don't always new what you want to trace.

fyrchik requested review from storage-core-committers 2023-05-29 09:44:30 +00:00

fyrchik requested review from storage-core-developers 2023-05-29 09:44:30 +00:00

dstepanov-yadro reviewed 2023-05-30 13:20:48 +00:00

cmd/frostfs-cli/internal/common/tracing.go Outdated

													
				@ -0,0 +17,4 @@

				type spanKey struct{}

				// StartClientCommandSpan stops tracing span for the command and prints trace ID on the standard output.

				func StopClientCommandSpan(cmd *cobra.Command, opts ...trace.SpanEndOption) {

dstepanov-yadro commented

2023-05-30 13:17:18 +00:00

opts ...trace.SpanEndOption is unused arg. I think it can be deleted.

`opts ...trace.SpanEndOption` is unused arg. I think it can be deleted.

fyrchik commented

2023-06-01 14:29:19 +00:00

Poster

Fixed.

dstepanov-yadro commented

2023-05-30 13:26:09 +00:00

func ExitOnErr(cmd *cobra.Command, errFmt string, err error) is called in the most of commands to exit with error. So you can place here trace id printing.

`func ExitOnErr(cmd *cobra.Command, errFmt string, err error)` is called in the most of commands to exit with error. So you can place here trace id printing.

👍 1

fyrchik force-pushed tracing-cli from d8de1fc8d9 to 48394ab285

2023-06-01 14:27:48 +00:00

Compare

fyrchik force-pushed tracing-cli from 48394ab285 to d72bace127

2023-06-01 14:28:58 +00:00

Compare

fyrchik changed title from ~~WIP: Add --trace flag to CLI commands~~ to Add --trace flag to CLI commands

2023-06-01 14:29:12 +00:00

dstepanov-yadro reviewed 2023-06-02 07:49:49 +00:00

go.mod Outdated

													
				@ -121,2 +121,4 @@

					v1.22.0 // Published accidentally.

				)

				replace git.frostfs.info/TrueCloudLab/frostfs-api-go/v2 => ../api-go

dstepanov-yadro commented

2023-06-02 07:49:49 +00:00

drop

fyrchik commented

2023-06-02 09:15:05 +00:00

Poster

Fixed

acid-ant approved these changes 2023-06-02 08:53:09 +00:00

cmd/frostfs-cli/internal/common/tracing.go Outdated

													
				@ -0,0 +16,4 @@

				type spanKey struct{}

				// StartClientCommandSpan stops tracing span for the command and prints trace ID on the standard output.