-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(metrics): Update metrics for aliased commands #4819
base: main
Are you sure you want to change the base?
Conversation
src/server/main_service.cc
Outdated
@@ -1234,7 +1234,7 @@ void Service::DispatchCommand(ArgSlice args, SinkReplyBuilder* builder, | |||
|
|||
dfly_cntx->cid = cid; | |||
|
|||
if (!InvokeCmd(cid, args_no_cmd, builder, dfly_cntx)) { | |||
if (!InvokeCmd(cid, args_no_cmd, builder, dfly_cntx, cmd)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this might break dual word registered commands like ACL ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed so that only aliases are handled specially when counting stats, all other commands use cmd.name
, by checking if the passed in command was part of alias map
62c9812
to
0099f42
Compare
When a call is made to server, its call count and call time is counted against the string it was called with, if the user supplied command was an alias to some other command. Signed-off-by: Abhijat Malviya <[email protected]>
0099f42
to
f5b7aea
Compare
std::optional<std::string_view> orig_cmd_name = std::nullopt; | ||
if (registry_.IsAlias(cmd)) { | ||
orig_cmd_name = cmd; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another alternative to this lookup would be to return more info from FindExtended
, maybe a struct with some flags like whether the input was alias or not.
This current approach seems to be simple, although it comes at the expense of an extra hashmap lookup in the command execution path.
The alias map would ideally be small though so this lookup shouldn't be too bad. I would be interested to know reviewers thoughts on this though.
When a call is made to server, its call count and call time is counted against the string it was called with, if the command was found to be an alias.
Fixes #4068
Follow up PR to #4782