Feature: Add redis search module metrics #953

nantiferov · 2024-09-10T20:11:21Z

fixes: #942

Found that redis returns all related information to modules with info modules and added collection of all metrics that looks useful so far.

Parsing of module:name=RedisCompat,ver=1,api=1,filters=0,usedby=[],using=[],options=[] is not ideal, but works fine with current Redis 7.4 / Redis search module 2.10

exporter/exporter.go

codecov · 2024-09-11T11:07:16Z

Codecov Report

Attention: Patch coverage is 84.78261% with 7 lines in your changes missing coverage. Please review.

Project coverage is 80.35%. Comparing base (827e6b5) to head (9e7822e).
Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
exporter/modules.go	82.75%	3 Missing and 2 partials ⚠️
main.go	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #953      +/-   ##
==========================================
+ Coverage   80.25%   80.35%   +0.10%     
==========================================
  Files          17       18       +1     
  Lines        1980     2026      +46     
==========================================
+ Hits         1589     1628      +39     
- Misses        306      311       +5     
- Partials       85       87       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

coveralls · 2024-09-11T11:07:30Z

Pull Request Test Coverage Report for Build 11101178781

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

48 of 54 (88.89%) changed or added relevant lines in 3 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.1%) to 84.219%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
main.go	0	2	0.0%
exporter/modules.go	29	33	87.88%

Totals
Change from base Build 10969721321:	0.1%
Covered Lines:	2060
Relevant Lines:	2446

💛 - Coveralls

oliver006 · 2024-09-19T13:38:53Z

Sorry about the delay - looking at the PR now.

nantiferov · 2024-09-19T13:43:37Z

No problem, thank you!

oliver006

General approach looks good - just a couple of minor questions

oliver006 · 2024-09-19T14:00:36Z

exporter/exporter.go

+			// Redis Modules metrics
+			// RediSearch module
+			"search_number_of_indexes":   "search_number_of_indexes",
+			"search_used_memory_indexes": "search_used_memory_indexes",


search_used_memory_indexes

I think this could use the unit this is in. Add _bytes ?

Thanks, fixed

oliver006 · 2024-09-19T14:05:26Z

exporter/exporter.go

+		"stream_radix_tree_nodes":                            {txt: `Radix tree nodes count`, lbls: []string{"db", "stream"}},
+		"up":                                                 {txt: "Information about the Redis instance"},
+		"module_info":                                        {txt: "Information about loaded Redis module", lbls: []string{"name", "ver", "api", "filters", "usedby", "using"}},
+		"search_version":                                     {txt: "Information about the RediSearch module", lbls: []string{"version"}},


can we bake this into the module_info metric? Add a version label? seems tightly related to the module but I don't know if they keep different versions for the two.

It's already exists there, just in different format. Here it's search_version:2.10.5, but there it's module:name=search,ver=21005,api=1,filters=0,usedby=[],using=[ReJSON],options=[handle-io-errors], i.e. ver=21005

In theory we can drop this metric and keep only module_info, but it looks nicer with dots as separators here.

I added also now continue after registering metrics to avoid unneeded parsing of metrics after registerConstMetricGauge

In theory we can drop this metric and keep only module_info, but it looks nicer with dots as separators here. - in that case, if the versions match, then I'd prefer to just capture one metric - the formatting is just eye candy.
But if the module version could differ from the search engine version then it'd make sense to capture both.

But if the module version could differ from the search engine version then it'd make sense to capture both.

Nope, that afaik shouldn't happen, removed search_version and let's keep version in module_info as is.

exporter/http_test.go

fixes: oliver006#942

nantiferov · 2024-09-19T14:25:05Z

docker-compose.yml

@@ -83,3 +83,8 @@ services:
    image: tile38/tile38:latest
    ports:
      - "19851:9851"
+
+  redis-stack:
+    image: redis/redis-stack-server:7.4.0-v0


For context, 7.4.0-v0 is current version of redis-stack-server and it includes redis 7.4 + modules with recent versions.

exporter/exporter.go

oliver006 · 2024-09-21T04:44:32Z

exporter/exporter.go

+		"stream_radix_tree_nodes":                            {txt: `Radix tree nodes count`, lbls: []string{"db", "stream"}},
+		"up":                                                 {txt: "Information about the Redis instance"},
+		"module_info":                                        {txt: "Information about loaded Redis module", lbls: []string{"name", "ver", "api", "filters", "usedby", "using"}},
+		"search_version":                                     {txt: "Information about the RediSearch module", lbls: []string{"version"}},


In theory we can drop this metric and keep only module_info, but it looks nicer with dots as separators here. - in that case, if the versions match, then I'd prefer to just capture one metric - the formatting is just eye candy.
But if the module version could differ from the search engine version then it'd make sense to capture both.

exporter/modules.go

oliver006

Nice work, looks good!

Will merge later today or tomorrow and cut a new release.

exporter/exporter.go

oliver006 · 2024-09-28T03:59:08Z

Apologies for the delay - I was busy this past week. I'll get back to this in the next few days.

exporter/modules.go

oliver006

Nice work - thanks for your patience and for pushing this all the way to completion!

nantiferov · 2024-10-07T06:57:06Z

Thank you for review.
Would be great to release new version this week if you have time.

oliver006 · 2024-10-08T03:46:52Z

Yeah, let me cut a release and get this out, sorry for the delay.

nantiferov · 2024-10-08T09:07:16Z

exporter/exporter.go

+			// RediSearch module
+			"search_number_of_indexes":   "search_number_of_indexes",
+			"search_used_memory_indexes": "search_used_memory_indexes_bytes",
+			"search_total_indexing_time": "search_total_indexing_time_ms",


search_total_indexing_time is apparently a counter, will prepare PR with renaming it to search_indexing_time_ms_total

Initially added in oliver006#953 as gauges, but usage shows that they're counters. Related: oliver006#942

#962) Initially added in #953 as gauges, but usage shows that they're counters. Related: #942

nantiferov commented Sep 10, 2024

View reviewed changes

exporter/exporter.go Outdated Show resolved Hide resolved

nantiferov force-pushed the Feature--Add-redis-search-module-metrics branch 3 times, most recently from b6959b0 to 3f02f94 Compare September 11, 2024 11:03

nantiferov force-pushed the Feature--Add-redis-search-module-metrics branch 2 times, most recently from 402bacd to 7c51e0c Compare September 11, 2024 15:01

nantiferov marked this pull request as ready for review September 11, 2024 15:01

nantiferov mentioned this pull request Sep 11, 2024

RediSearch module metrics collection #942

Closed

2 tasks

nantiferov force-pushed the Feature--Add-redis-search-module-metrics branch from 7c51e0c to 081482b Compare September 11, 2024 15:13

oliver006 reviewed Sep 19, 2024

View reviewed changes

nantiferov added 6 commits September 19, 2024 16:22

Feature: Add redis search module metrics

113340b

fixes: oliver006#942

Fix redis-search port for tests

31ff648

Switch to universal data collection for modules

4dffe83

Add more module metrics collection

9ff9169

Add continue after registerConstMetricGauge in module switch

d271734

Add _bytes suffix to search_used_memory_indexes

d97d711

nantiferov force-pushed the Feature--Add-redis-search-module-metrics branch from 17b1ee1 to d97d711 Compare September 19, 2024 14:22

nantiferov commented Sep 19, 2024

View reviewed changes

oliver006 force-pushed the master branch from 03452e3 to 827e6b5 Compare September 21, 2024 04:37

oliver006 reviewed Sep 21, 2024

View reviewed changes

nantiferov and others added 3 commits September 21, 2024 23:33

Merge branch 'master' into Feature--Add-redis-search-module-metrics

d4bd8dd

Remove search_version metric

8d36dd5

Add checks for correctlness of data after split

d125fa4

oliver006 approved these changes Sep 22, 2024

View reviewed changes

Set units for 3 more metrics

7baa35f

nantiferov commented Sep 22, 2024

View reviewed changes

exporter/exporter.go Show resolved Hide resolved

oliver006 reviewed Sep 29, 2024

View reviewed changes

exporter/modules.go Show resolved Hide resolved

Clarify comment about module format

9e7822e

oliver006 approved these changes Oct 5, 2024

View reviewed changes

oliver006 merged commit 1f06f2f into oliver006:master Oct 5, 2024
10 checks passed

nantiferov deleted the Feature--Add-redis-search-module-metrics branch October 7, 2024 06:55

nantiferov commented Oct 8, 2024

View reviewed changes

nantiferov added a commit to nantiferov/redis_exporter that referenced this pull request Oct 29, 2024

Hotfix: Change type of search_total_cycles/search_total_ms_run metrics

c9ea61f

Initially added in oliver006#953 as gauges, but usage shows that they're counters. Related: oliver006#942

nantiferov mentioned this pull request Oct 29, 2024

Hotfix: Change type of search_total_cycles/search_total_ms_run metrics #962

Merged

oliver006 pushed a commit that referenced this pull request Oct 31, 2024

Hotfix: Change type of search_total_cycles/search_total_ms_run metrics (

d9ccc72

#962) Initially added in #953 as gauges, but usage shows that they're counters. Related: #942

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Add redis search module metrics #953

Feature: Add redis search module metrics #953

nantiferov commented Sep 10, 2024 •

edited

Loading

codecov bot commented Sep 11, 2024 •

edited

Loading

coveralls commented Sep 11, 2024 •

edited

Loading

oliver006 commented Sep 19, 2024

nantiferov commented Sep 19, 2024

oliver006 left a comment

oliver006 Sep 19, 2024

nantiferov Sep 19, 2024

oliver006 Sep 19, 2024

nantiferov Sep 19, 2024

nantiferov Sep 19, 2024

oliver006 Sep 21, 2024

nantiferov Sep 21, 2024

oliver006 Sep 22, 2024

nantiferov Sep 19, 2024 •

edited

Loading

oliver006 Sep 21, 2024

oliver006 left a comment

oliver006 commented Sep 28, 2024

oliver006 left a comment

nantiferov commented Oct 7, 2024

oliver006 commented Oct 8, 2024

nantiferov Oct 8, 2024

Feature: Add redis search module metrics #953

Feature: Add redis search module metrics #953

Conversation

nantiferov commented Sep 10, 2024 • edited Loading

codecov bot commented Sep 11, 2024 • edited Loading

Codecov Report

coveralls commented Sep 11, 2024 • edited Loading

Pull Request Test Coverage Report for Build 11101178781

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

oliver006 commented Sep 19, 2024

nantiferov commented Sep 19, 2024

oliver006 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nantiferov Sep 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliver006 left a comment

Choose a reason for hiding this comment

oliver006 commented Sep 28, 2024

oliver006 left a comment

Choose a reason for hiding this comment

nantiferov commented Oct 7, 2024

oliver006 commented Oct 8, 2024

Choose a reason for hiding this comment

nantiferov commented Sep 10, 2024 •

edited

Loading

codecov bot commented Sep 11, 2024 •

edited

Loading

coveralls commented Sep 11, 2024 •

edited

Loading

nantiferov Sep 19, 2024 •

edited

Loading