Skip to content

Commit

Permalink
Correctly recode client infomation to the slowlog when runing script (#…
Browse files Browse the repository at this point in the history
…805)

Currently when we are runing a script, we will passing a fake client.
So if the command executed in the script is recoded in the slowlog,
the client's ip:port and name information will be empty.

before:
```
127.0.0.1:6379> client setname myclient
OK
127.0.0.1:6379> config set slowlog-log-slower-than 0
OK
127.0.0.1:6379> eval "redis.call('ping')" 0
(nil)
127.0.0.1:6379> slowlog get 2
1) 1) (integer) 2
   2) (integer) 1721314289
   3) (integer) 96
   4) 1) "eval"
      2) "redis.call('ping')"
      3) "0"
   5) "127.0.0.1:61106"
   6) "myclient"
2) 1) (integer) 1
   2) (integer) 1721314289
   3) (integer) 4
   4) 1) "ping"
   5) ""
   6) ""
```

after:
```
127.0.0.1:6379> client setname myclient
OK
127.0.0.1:6379> config set slowlog-log-slower-than 0
OK
127.0.0.1:6379> eval "redis.call('ping')" 0
(nil)
127.0.0.1:6379> slowlog get 2
1) 1) (integer) 2
   2) (integer) 1721314371
   3) (integer) 53
   4) 1) "eval"
      2) "redis.call('ping')"
      3) "0"
   5) "127.0.0.1:51983"
   6) "myclient"
2) 1) (integer) 1
   2) (integer) 1721314371
   3) (integer) 1
   4) 1) "ping"
   5) "127.0.0.1:51983"
   6) "myclient"
```

Signed-off-by: Binbin <[email protected]>
  • Loading branch information
enjoy-binbin authored and madolson committed Sep 2, 2024
1 parent 3df3998 commit 6d93a31
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 0 deletions.
7 changes: 7 additions & 0 deletions src/server.c
Original file line number Diff line number Diff line change
Expand Up @@ -3275,6 +3275,13 @@ void slowlogPushCurrentCommand(client *c, struct serverCommand *cmd, ustime_t du
* arguments. */
robj **argv = c->original_argv ? c->original_argv : c->argv;
int argc = c->original_argv ? c->original_argc : c->argc;

/* If a script is currently running, the client passed in is a
* fake client. Or the client passed in is the original client
* if this is a EVAL or alike, doesn't matter. In this case,
* use the original client to get the client information. */
c = scriptIsRunning() ? scriptGetCaller() : c;

slowlogPushEntryIfNeeded(c, argv, argc, duration);
}

Expand Down
38 changes: 38 additions & 0 deletions tests/unit/slowlog.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -248,4 +248,42 @@ start_server {tags {"slowlog"} overrides {slowlog-log-slower-than 1000000}} {

$rd close
}

foreach is_eval {0 1} {
test "SLOWLOG - the commands in script are recorded normally - is_eval: $is_eval" {
if {$is_eval == 0} {
r function load replace "#!lua name=mylib \n redis.register_function('myfunc', function(KEYS, ARGS) server.call('ping') end)"
}

r client setname test-client
r config set slowlog-log-slower-than 0
r slowlog reset

if {$is_eval} {
r eval "server.call('ping')" 0
} else {
r fcall myfunc 0
}
set slowlog_resp [r slowlog get 2]
assert_equal 2 [llength $slowlog_resp]

# The first one is the script command, and the second one is the ping command executed in the script
# Each slowlog contains: id, timestamp, execution time, command array, ip:port, client name
set script_cmd [lindex $slowlog_resp 0]
set ping_cmd [lindex $slowlog_resp 1]

# Make sure the command are logged.
if {$is_eval} {
assert_equal {eval server.call('ping') 0} [lindex $script_cmd 3]
} else {
assert_equal {fcall myfunc 0} [lindex $script_cmd 3]
}
assert_equal {ping} [lindex $ping_cmd 3]

# Make sure the client info are the logged.
assert_equal [lindex $script_cmd 4] [lindex $ping_cmd 4]
assert_equal {test-client} [lindex $script_cmd 5]
assert_equal {test-client} [lindex $ping_cmd 5]
}
}
}

0 comments on commit 6d93a31

Please sign in to comment.