Skip to content

Provide named function for squashing usage errors; start using it#7451

Merged
rohit-nayak-ps merged 1 commit intovitessio:masterfrom
tinyspeck:am_vtctldclient_errors
Feb 5, 2021
Merged

Provide named function for squashing usage errors; start using it#7451
rohit-nayak-ps merged 1 commit intovitessio:masterfrom
tinyspeck:am_vtctldclient_errors

Conversation

@ajm188
Copy link
Contributor

@ajm188 ajm188 commented Feb 5, 2021

Description

Also includes a lot of documentation about why we want this thing, which
hopefully is clear!

Signed-off-by: Andrew Mason amason@slack-corp.com

Related Issue(s)

Checklist

  • Should this PR be backported? No
  • Tests were added or are not required N/A
  • Documentation was added or is not required

Deployment Notes

Impacted Areas in Vitess

Components that this PR will affect:

  • Query Serving
  • VReplication
  • Cluster Management
  • Build/CI
  • VTAdmin

Also includes a lot of documentation about why we want this thing, which
hopefully is clear!

Signed-off-by: Andrew Mason <amason@slack-corp.com>
@ajm188 ajm188 requested review from doeg and rohit-nayak-ps February 5, 2021 02:22
Copy link
Contributor

@doeg doeg left a comment

Choose a reason for hiding this comment

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

This was fun to read.

@ajm188
Copy link
Contributor Author

ajm188 commented Feb 5, 2021

Oh I forgot! I took a before/after to demonstrate.

On master:

❯ vtctldclient --server "localhost:15999" GetShard doesnotexist/-
Usage:
   GetShard <keyspace/shard> [flags]

Flags:
  -h, --help   help for GetShard

Global Flags:
      --action_timeout duration                timeout for the total command (default 1h0m0s)
      --alsologtostderr                        log to standard error as well as files
      --datadog-agent-host string              host to send spans to. if empty, no tracing will be done
      --datadog-agent-port string              port to send spans to. if empty, no tracing will be done
      --emit_stats                             If set, emit stats to push-based monitoring and stats backends
      --grpc_auth_static_client_creds string   when using grpc_static_auth in the server, this file provides the credentials to use to authenticate with server
      --grpc_compression string                Which protocol to use for compressing gRPC. Default: nothing. Supported: snappy
      --grpc_enable_tracing                    Enable GRPC tracing
      --grpc_initial_conn_window_size int      gRPC initial connection window size
      --grpc_initial_window_size int           gRPC initial window size
      --grpc_keepalive_time duration           After a duration of this time, if the client doesn't see any activity, it pings the server to see if the transport is still alive. (default 10s)
      --grpc_keepalive_timeout duration        After having pinged for keepalive check, the client waits for a duration of Timeout and if no activity is seen even after that the connection is closed. (default 10s)
      --grpc_max_message_size int              Maximum allowed RPC message size. Larger messages will be rejected by gRPC with the error 'exceeding the max size'. (default 16777216)
      --grpc_prometheus                        Enable gRPC monitoring with Prometheus
      --jaeger-agent-host string               host and port to send spans to. if empty, no tracing will be done
      --keep_logs duration                     keep logs for this long (using ctime) (zero to keep forever) (default 0s)
      --keep_logs_by_mtime duration            keep logs for this long (using mtime) (zero to keep forever) (default 0s)
      --log_backtrace_at traceLocation         when logging hits line file:N, emit a stack trace (default :0)
      --log_dir string                         If non-empty, write log files in this directory
      --log_err_stacks                         log stack traces for errors
      --log_rotate_max_size uint               size in bytes at which logs are rotated (glog.MaxSize) (default 1887436800)
      --logtostderr                            log to standard error instead of files
      --purge_logs_interval duration           how often try to remove old logs (default 1h0m0s)
      --remote_operation_timeout duration      time to wait for a remote operation (default 30s)
      --server string                          server to use for connection
      --sql-max-length-errors int              truncate queries in error logs to the given length (default unlimited)
      --sql-max-length-ui int                  truncate queries in debug UIs to the given length (default 512) (default 512)
      --stats_backend string                   The name of the registered push-based monitoring/stats backend to use
      --stats_combine_dimensions string        List of dimensions to be combined into a single "all" value in exported stats vars
      --stats_drop_variables string            Variables to be dropped from the list of exported variables.
      --stats_emit_period duration             Interval between emitting stats to all registered backends (default 1m0s)
      --stderrthreshold severity               logs at or above this threshold go to stderr (default 1)
      --topo_global_root string                the path of the global topology data in the global topology server
      --topo_global_server_address string      the address of the global topology server
      --topo_implementation string             the topology implementation to use
      --tracer string                          tracing service to use (default "noop")
      --tracing-sampling-rate float            sampling rate for the probabilistic jaeger sampler (default 0.1)
  -v, --v Level                                log level for V logs
      --vmodule moduleSpec                     comma-separated list of pattern=N settings for file-filtered logging
      --vtctld_grpc_ca string                  the server ca to use to validate servers when connecting
      --vtctld_grpc_cert string                the cert to use to connect
      --vtctld_grpc_key string                 the key to use to connect
      --vtctld_grpc_server_name string         the server name to use to validate server certificate

E0204 21:18:17.422348   84647 main.go:42] rpc error: code = Unknown desc = node doesn't exist: /vitess/global/keyspaces/doesnotexist/shards/-/Shard

With this change:

❯ vtctldclient --server "localhost:15999" GetShard doesnotexist/-
E0204 21:19:35.818538   84973 main.go:42] rpc error: code = Unknown desc = node doesn't exist: /vitess/global/keyspaces/doesnotexist/shards/-/Shard

However, if I don't actually pass flag parsing, I still get the usage:

❯ vtctldclient --server "localhost:15999" GetShard
Usage:
   GetShard <keyspace/shard> [flags]

Flags:
  -h, --help   help for GetShard

Global Flags:
      --action_timeout duration                timeout for the total command (default 1h0m0s)
      --alsologtostderr                        log to standard error as well as files
      --datadog-agent-host string              host to send spans to. if empty, no tracing will be done
      --datadog-agent-port string              port to send spans to. if empty, no tracing will be done
      --emit_stats                             If set, emit stats to push-based monitoring and stats backends
      --grpc_auth_static_client_creds string   when using grpc_static_auth in the server, this file provides the credentials to use to authenticate with server
      --grpc_compression string                Which protocol to use for compressing gRPC. Default: nothing. Supported: snappy
      --grpc_enable_tracing                    Enable GRPC tracing
      --grpc_initial_conn_window_size int      gRPC initial connection window size
      --grpc_initial_window_size int           gRPC initial window size
      --grpc_keepalive_time duration           After a duration of this time, if the client doesn't see any activity, it pings the server to see if the transport is still alive. (default 10s)
      --grpc_keepalive_timeout duration        After having pinged for keepalive check, the client waits for a duration of Timeout and if no activity is seen even after that the connection is closed. (default 10s)
      --grpc_max_message_size int              Maximum allowed RPC message size. Larger messages will be rejected by gRPC with the error 'exceeding the max size'. (default 16777216)
      --grpc_prometheus                        Enable gRPC monitoring with Prometheus
      --jaeger-agent-host string               host and port to send spans to. if empty, no tracing will be done
      --keep_logs duration                     keep logs for this long (using ctime) (zero to keep forever) (default 0s)
      --keep_logs_by_mtime duration            keep logs for this long (using mtime) (zero to keep forever) (default 0s)
      --log_backtrace_at traceLocation         when logging hits line file:N, emit a stack trace (default :0)
      --log_dir string                         If non-empty, write log files in this directory
      --log_err_stacks                         log stack traces for errors
      --log_rotate_max_size uint               size in bytes at which logs are rotated (glog.MaxSize) (default 1887436800)
      --logtostderr                            log to standard error instead of files
      --purge_logs_interval duration           how often try to remove old logs (default 1h0m0s)
      --remote_operation_timeout duration      time to wait for a remote operation (default 30s)
      --server string                          server to use for connection
      --sql-max-length-errors int              truncate queries in error logs to the given length (default unlimited)
      --sql-max-length-ui int                  truncate queries in debug UIs to the given length (default 512) (default 512)
      --stats_backend string                   The name of the registered push-based monitoring/stats backend to use
      --stats_combine_dimensions string        List of dimensions to be combined into a single "all" value in exported stats vars
      --stats_drop_variables string            Variables to be dropped from the list of exported variables.
      --stats_emit_period duration             Interval between emitting stats to all registered backends (default 1m0s)
      --stderrthreshold severity               logs at or above this threshold go to stderr (default 1)
      --topo_global_root string                the path of the global topology data in the global topology server
      --topo_global_server_address string      the address of the global topology server
      --topo_implementation string             the topology implementation to use
      --tracer string                          tracing service to use (default "noop")
      --tracing-sampling-rate float            sampling rate for the probabilistic jaeger sampler (default 0.1)
  -v, --v Level                                log level for V logs
      --vmodule moduleSpec                     comma-separated list of pattern=N settings for file-filtered logging
      --vtctld_grpc_ca string                  the server ca to use to validate servers when connecting
      --vtctld_grpc_cert string                the cert to use to connect
      --vtctld_grpc_key string                 the key to use to connect
      --vtctld_grpc_server_name string         the server name to use to validate server certificate

E0204 21:19:51.631723   84982 main.go:42] accepts 1 arg(s), received 0

@rohit-nayak-ps rohit-nayak-ps merged commit 0e14662 into vitessio:master Feb 5, 2021
@deepthi
Copy link
Collaborator

deepthi commented Feb 6, 2021

Late to the party, but this is awesome! Love the detailed documentation.

@askdba askdba added this to the v10.0 milestone Feb 8, 2021
ajm188 pushed a commit to tinyspeck/vitess that referenced this pull request Feb 11, 2021
Provide named function for squashing usage errors; start using it
@ajm188 ajm188 deleted the am_vtctldclient_errors branch March 4, 2021 16:32
setassociative pushed a commit to tinyspeck/vitess that referenced this pull request Mar 11, 2021
Provide named function for squashing usage errors; start using it
rafael pushed a commit to tinyspeck/vitess that referenced this pull request Apr 6, 2021
Provide named function for squashing usage errors; start using it

Signed-off-by: Rafael Chacon <rafael@slack-corp.com>
@ajm188 ajm188 added the Type: Enhancement Logical improvement (somewhere between a bug and feature) label May 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Cluster management Type: Enhancement Logical improvement (somewhere between a bug and feature)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants