[RedisEnterprise] Enable use_type flag for node and proxy datastreams by ritalwar · Pull Request #8213 · elastic/integrations

ritalwar · 2023-10-16T13:12:13Z

Proposed commit message

This PR enables use_type flag for node and proxy datastreams of RedisEnterprise. This is required to correctly specify the metric_type mapping, as use_type enables a different layout for metrics storage, leveraging Elasticsearch types.

Checklist

I have reviewed tips for building integrations and this pull request is aligned with them.
I have verified that all data streams collect metrics or logs.
I have added an entry to my package's changelog.yml file.
I have verified that Kibana version constraints are current according to guidelines.

Related issues

Closes Enabling 'use_type' Flag for Redis Enterprise #8149

Screenshots

elasticmachine · 2023-10-16T13:26:59Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2023-11-15T05:37:35.182+0000
Duration: 16 min 30 sec

Test stats 🧪

Test	Results
Failed	0
Passed	5
Skipped	0
Total	5

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.

elasticmachine · 2023-10-16T13:27:09Z

🌐 Coverage report

Name	Metrics % (`covered/total`)	Diff
Packages	100.0% (`0/0`)	💚
Files	100.0% (`0/0`)	💚 3.832
Classes	100.0% (`0/0`)	💚 3.832
Methods	50.0% (`4/8`)	👎 -42.257
Lines	100.0% (`0/0`)	💚 11.395
Conditionals	100.0% (`0/0`)	💚

lalit-satapathy · 2023-11-02T08:27:22Z

                "node": "1",
                "instance": "host.docker.internal:8070",
                "job": "prometheus"
+            },


is this a coincidence that all node* metrics are gauge/value?

I've already done the metric_type PR for this datastream and verified then there that all the node datastream metrics are gauges. Also please refer.

lalit-satapathy · 2023-11-02T08:27:40Z

-                "listener_total_started_res": 0,
-                "listener_cmd_set_max": 0,
-                "listener_acc_latency_max": 0
+            "listener_resp2_clients_max": {


Same here, is this a coincidence that all proxy* metrics are counters?

I will verify this again.

Any changes here?

No, I verified all of them are counter fields, and this has been verified from the Prometheus endpoint where they are collected.

lalit-satapathy · 2023-11-02T08:28:33Z

-      type: object
-      object_type: keyword
-      description: Label fields
+- name: redisenterprise.node.*.value


Assuming the metric_type mapping will be taken in separate PR.

Yes, those will be raised separately as part of the TSDB migration.

Assuming the metric type mappsing is: redisenterprise.node.*.value will directly map to gauge and redisenterprise.proxy.*.counter to counter.

Yes, we do need to explicitly specify the metric_type, and I will address this in the metric_type PR.

lalit-satapathy

LGTM, Need to follow-up and ensure the TSDB mappings are updated accordingly.

lalit-satapathy · 2023-11-06T08:36:46Z

+  description: >
+    Redisenterprise node gauge metric
+
+- name: redisenterprise.node.*.counter


Is this entry needed, since node is all gauge? Or is it a fallback if any nodes have counter in the future?

I believe it's a better idea to remove it. Instead of retaining a fallback, we can update it when a new field is introduced in a new version. I will also make sure to update the tested version for consistency.

lalit-satapathy · 2023-11-06T08:37:53Z

-      type: object
-      object_type: keyword
-      description: Label fields
+- name: redisenterprise.node.*.value


Assuming the metric type mappsing is: redisenterprise.node.*.value will directly map to gauge and redisenterprise.proxy.*.counter to counter.

lalit-satapathy · 2023-11-06T08:38:07Z

-                "listener_total_started_res": 0,
-                "listener_cmd_set_max": 0,
-                "listener_acc_latency_max": 0
+            "listener_resp2_clients_max": {


Any changes here?

agithomas

Please verify if a change in the snapshot image is needed as part of the dashboard change?

agithomas · 2023-11-10T09:07:57Z

I suppose there is SVR00002 exclusion (Mandatory filters in dashboard) in the package configuration. Would you be able to overturn this with the dashboard changes ?

agithomas · 2023-11-10T09:10:10Z

 Redis Enterprise integration provides monitoring of [redis](https://redis.com/) cluster. Monitoring is done via prometheus exported port of redis enterprise cluster. Once a redis enterprise [cluster](https://redis.com/redis-enterprise/technology/redis-enterprise-cluster-architecture/) is installed, prometheus port is available for monitoring. The url of the host:port(8070) needs to be passed to the hosts in the settings.

-Redis Enterpise integration is tested with redislabs/redis:5.2.2-24 version.
+Redis Enterpise integration is tested with redislabs/redis:7.2.4-72 version.


Please add a title ## Compatibility above this line.

ritalwar · 2023-11-10T10:31:31Z

Please verify if a change in the snapshot image is needed as part of the dashboard change?

There aren't any major changes, just some adjustments to field names that aren't visible in the snapshot.

ritalwar · 2023-11-10T10:33:35Z

I suppose there is SVR00002 exclusion (Mandatory filters in dashboard) in the package configuration. Would you be able to overturn this with the dashboard changes ?

Sorry, I don't understand why we need to overturn it.

agithomas · 2023-11-10T10:44:22Z

Sorry, I don't understand why we need to overturn it.

Ignore it, it can be taken up later in a separate PR and it is a part of package spec V3 issue addressal.

shmsr · 2023-11-10T16:43:00Z

Left one more important comment: #8213 (comment)

agithomas

LGTM!

elasticmachine · 2023-11-15T07:09:08Z

Package redisenterprise - 0.7.0 containing this change is available at https://epr.elastic.co/search?package=redisenterprise

[RedisEnterprise] Enable use_type flag for node and proxy datastreams.

2be582a

ritalwar requested a review from a team as a code owner October 16, 2023 13:12

Update changelog.yml

3d188f3

ritalwar and others added 7 commits October 18, 2023 12:18

Update Dashboards

a61b7c1

Update Dashboard title

c9bb735

Update README.md

cbc22d1

Merge branch 'main' into redisenterprise_usetype_8149

e0f92de

Add object_type

85cd600

Update sample_event.json

df0b19f

Update fields mappings for node and proxy datastreams

d559c67

ritalwar requested review from agithomas and lalit-satapathy October 27, 2023 09:26

ritalwar added the enhancement New feature or request label Nov 1, 2023

lalit-satapathy reviewed Nov 2, 2023

View reviewed changes

Update changelog.yml

e1d8909

lalit-satapathy approved these changes Nov 6, 2023

View reviewed changes

Address review comments

2553c00

shmsr changed the title ~~[RedisEnterprise] Enable use_type flag for node and proxy datastreams.~~ Nov 9, 2023

agithomas reviewed Nov 10, 2023

View reviewed changes

Comment thread packages/redisenterprise/data_stream/node/fields/fields.yml Outdated

agithomas reviewed Nov 10, 2023

View reviewed changes

Comment thread packages/redisenterprise/data_stream/node/fields/fields.yml

agithomas reviewed Nov 10, 2023

View reviewed changes

ritalwar requested a review from agithomas November 10, 2023 10:44

shmsr requested changes Nov 10, 2023

View reviewed changes

Comment thread packages/redisenterprise/data_stream/proxy/fields/fields.yml Outdated

Comment thread packages/redisenterprise/_dev/build/docs/README.md Outdated

ritalwar requested a review from shmsr November 10, 2023 11:12

Address comments

0d60e7c

Update fields.yml

1d5961d

agithomas approved these changes Nov 15, 2023

View reviewed changes

shmsr approved these changes Nov 15, 2023

View reviewed changes

ritalwar merged commit d31f40f into elastic:main Nov 15, 2023

andrewkroh added the Integration:redisenterprise Redis Enterprise label Jul 22, 2024

Uh oh!

Conversation

ritalwar commented Oct 16, 2023 • edited by shmsr Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed commit message

Checklist

Related issues

Screenshots

elasticmachine commented Oct 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

Test stats 🧪

🤖 GitHub comments

elasticmachine commented Oct 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🌐 Coverage report

Uh oh!

Uh oh!

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

ritalwar Nov 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lalit-satapathy 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

Uh oh!

Uh oh!

agithomas left a comment

Choose a reason for hiding this comment

agithomas commented Nov 10, 2023

Choose a reason for hiding this comment

ritalwar commented Nov 10, 2023

ritalwar commented Nov 10, 2023

agithomas commented Nov 10, 2023

Uh oh!

Uh oh!

shmsr commented Nov 10, 2023

agithomas left a comment

Choose a reason for hiding this comment

elasticmachine commented Nov 15, 2023

Labels

6 participants

ritalwar commented Oct 16, 2023 •

edited by shmsr

Loading

elasticmachine commented Oct 16, 2023 •

edited

Loading

elasticmachine commented Oct 16, 2023 •

edited

Loading

ritalwar Nov 2, 2023 •

edited

Loading