Skip to content

Commit b58097f

Browse files
authored
fix(redisreceiver): Collect redis keyspace metrics even if dbs are nonsequential (open-telemetry#38210)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description As described in the issue, if a redis DB is inactive it will not report keyspace metrics, and if a user has nonsequential dbs in use the metrics will not all be captured. This PR resolves the issue by continuing on a missing DB instead of breaking. The maximum number of dbs is 15, so a few extra loop iterations shouldn't be a significant performance affect. <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue Fixes open-telemetry#38135 <!--Describe what testing was performed and which tests were added.--> #### Testing Added case to the unit tests. <!--Please delete paragraphs that you did not use before submitting.-->
1 parent f609c28 commit b58097f

File tree

5 files changed

+35
-5
lines changed

5 files changed

+35
-5
lines changed
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: bug_fix
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: redisreceiver
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Collect keyspace metrics even if reported dbs are nonsequential
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [38135]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext: |
19+
If a redis instance has no activity on a db, the db number is not reported in the keyspace metrics.
20+
This change ensures that the keyspace metrics are collected even if the reported dbs have gaps.
21+
22+
# If your change doesn't affect end users or the exported elements of any package,
23+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
24+
# Optional: The change log or logs in which this entry should be included.
25+
# e.g. '[user]' or '[user, api]'
26+
# Include 'user' if the change is relevant to end users.
27+
# Include 'api' if there is a change to a library API.
28+
# Default: '[user]'
29+
change_logs: []

receiver/redisreceiver/redis_scraper.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,11 +137,11 @@ func (rs *redisScraper) recordCommonMetrics(ts pcommon.Timestamp, inf info) {
137137
// recordKeyspaceMetrics records metrics from 'keyspace' Redis info key-value pairs,
138138
// e.g. "db0: keys=1,expires=2,avg_ttl=3".
139139
func (rs *redisScraper) recordKeyspaceMetrics(ts pcommon.Timestamp, inf info) {
140-
for db := 0; db < redisMaxDbs; db++ {
140+
for db := range redisMaxDbs {
141141
key := "db" + strconv.Itoa(db)
142142
str, ok := inf[key]
143143
if !ok {
144-
break
144+
continue
145145
}
146146
keyspace, parsingError := parseKeyspaceString(db, str)
147147
if parsingError != nil {

receiver/redisreceiver/redis_scraper_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@ func TestRedisRunnable(t *testing.T) {
2727
require.NoError(t, err)
2828
md, err := runner.ScrapeMetrics(context.Background())
2929
require.NoError(t, err)
30-
// + 6 because there are two keyspace entries each of which has three metrics
30+
// + 9 because there are three keyspace entries each of which has three metrics
3131
// -2 because maxmemory and slave_repl_offset is by default disabled, so recorder is there, but there won't be data point
32-
assert.Equal(t, len(rs.dataPointRecorders())+6-2, md.DataPointCount())
32+
assert.Equal(t, len(rs.dataPointRecorders())+9-2, md.DataPointCount())
3333
rm := md.ResourceMetrics().At(0)
3434
ilm := rm.ScopeMetrics().At(0)
3535
il := ilm.Scope()

receiver/redisreceiver/redis_svc_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,6 @@ func TestParser(t *testing.T) {
1717
s := newFakeAPIParser()
1818
info, err := s.info()
1919
require.NoError(t, err)
20-
require.Len(t, info, 130)
20+
require.Len(t, info, 131)
2121
require.Equal(t, "1.24", info["allocator_frag_ratio"]) // spot check
2222
}

receiver/redisreceiver/testdata/info.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ cluster_enabled:0
140140
# Keyspace
141141
db0:keys=1,expires=2,avg_ttl=3
142142
db1:keys=4,expires=5,avg_ttl=6
143+
db3:keys=5,expires=10,avg_ttl=85
143144

144145
# Commandstats
145146
cmdstat_ssubscribe:calls=0,usec=0,usec_per_call=0.00,rejected_calls=3,failed_calls=0

0 commit comments

Comments
 (0)