Skip to content
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

Capture and parse the cmdstat timings from INFO ALL command #5926

Merged
merged 2 commits into from
Aug 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions plugins/inputs/redis/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,13 @@ Additionally the plugin also calculates the hit/miss ratio (keyspace\_hitrate) a
- expires(int, number)
- avg_ttl(int, number)

- redis_cmdstat
Every Redis used command will have 3 new fields:
- calls(int, number)
- usec(int, mircoseconds)
- usec_per_call(float, microseconds)


### Tags:

- All measurements have the following tags:
Expand All @@ -130,6 +137,9 @@ Additionally the plugin also calculates the hit/miss ratio (keyspace\_hitrate) a
- The redis_keyspace measurement has an additional database tag:
- database

- The redis_cmdstat measurement has an additional tag:
- command

### Example Output:

Using this configuration:
Expand Down Expand Up @@ -161,3 +171,8 @@ redis_keyspace:
```
> redis_keyspace,database=db1,host=host,server=localhost,port=6379,replication_role=master keys=1i,expires=0i,avg_ttl=0i 1493101350000000000
```

redis_command:
```
> redis_cmdstat,command=publish,host=host,port=6379,replication_role=master,server=localhost calls=68113i,usec=325146i,usec_per_call=4.77 1559227136000000000
```
52 changes: 51 additions & 1 deletion plugins/inputs/redis/redis.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ type RedisClient struct {
}

func (r *RedisClient) Info() *redis.StringCmd {
return r.client.Info()
return r.client.Info("ALL")
}

func (r *RedisClient) BaseTags() map[string]string {
Expand Down Expand Up @@ -248,6 +248,11 @@ func gatherInfoOutput(
gatherKeyspaceLine(name, kline, acc, tags)
continue
}
if section == "Commandstats" {
kline := strings.TrimSpace(parts[1])
gatherCommandstateLine(name, kline, acc, tags)
continue
}
metric = name
}

Expand Down Expand Up @@ -321,6 +326,51 @@ func gatherKeyspaceLine(
}
}

// Parse the special cmdstat lines.
// Example:
// cmdstat_publish:calls=33791,usec=208789,usec_per_call=6.18
// Tag: cmdstat=publish; Fields: calls=33791i,usec=208789i,usec_per_call=6.18
func gatherCommandstateLine(
name string,
line string,
acc telegraf.Accumulator,
global_tags map[string]string,
) {
if !strings.HasPrefix(name, "cmdstat") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick, but I would add an underscore to match the TrimPrefix laster on:

- if !strings.HasPrefix(name, "cmdstat") {
+ if !strings.HasPrefix(name, "cmdstat_") {

return
}

fields := make(map[string]interface{})
tags := make(map[string]string)
for k, v := range global_tags {
tags[k] = v
}
tags["command"] = strings.TrimPrefix(name, "cmdstat_")
parts := strings.Split(line, ",")
for _, part := range parts {
kv := strings.Split(part, "=")
if len(kv) != 2 {
continue
}

switch kv[0] {
case "calls":
fallthrough
case "usec":
ival, err := strconv.ParseInt(kv[1], 10, 64)
if err == nil {
fields[kv[0]] = ival
}
case "usec_per_call":
fval, err := strconv.ParseFloat(kv[1], 64)
if err == nil {
fields[kv[0]] = fval
}
}
}
acc.AddFields("redis_cmdstat", fields, tags)
}

func init() {
inputs.Add("redis", func() telegraf.Input {
return &Redis{}
Expand Down
20 changes: 20 additions & 0 deletions plugins/inputs/redis/redis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,22 @@ func TestRedis_ParseMetrics(t *testing.T) {
}
acc.AssertContainsTaggedFields(t, "redis", fields, tags)
acc.AssertContainsTaggedFields(t, "redis_keyspace", keyspaceFields, keyspaceTags)

cmdstatSetTags := map[string]string{"host": "redis.net", "replication_role": "master", "command": "set"}
cmdstatSetFields := map[string]interface{}{
"calls": int64(261265),
"usec": int64(1634157),
"usec_per_call": float64(6.25),
}
acc.AssertContainsTaggedFields(t, "redis_cmdstat", cmdstatSetFields, cmdstatSetTags)

cmdstatCommandTags := map[string]string{"host": "redis.net", "replication_role": "master", "command": "command"}
cmdstatCommandFields := map[string]interface{}{
"calls": int64(1),
"usec": int64(990),
"usec_per_call": float64(990.0),
}
acc.AssertContainsTaggedFields(t, "redis_cmdstat", cmdstatCommandFields, cmdstatCommandTags)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we needs to add test in case that usec and usec_per_call are not number?

Copy link
Contributor

@glinton glinton Aug 5, 2019

Choose a reason for hiding this comment

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

If this is something that may be returned, let's test for and handle it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a pattern from testutil I could use?

FWIW, I don't expect the type to change given the age of this commit :)
redis/redis@d7ed7fd

}

const testOutput = `# Server
Expand Down Expand Up @@ -205,6 +221,10 @@ used_cpu_user:0.05
used_cpu_sys_children:0.00
used_cpu_user_children:0.00

# Commandstats
cmdstat_set:calls=261265,usec=1634157,usec_per_call=6.25
cmdstat_command:calls=1,usec=990,usec_per_call=990.00

# Keyspace
db0:keys=2,expires=0,avg_ttl=0

Expand Down