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

Add detailed_index_stats parameter to pull index-level metrics #10766

Merged
merged 12 commits into from
Dec 7, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
9 changes: 9 additions & 0 deletions elastic/assets/configuration/spec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,15 @@ files:
value:
type: boolean
example: false
- name: detailed_index_stats
description: |
This flag is to be used with `cluster_stats` and `pshard_stats` set to true and if
you want to obtain index specific stats.
missingcharacter marked this conversation as resolved.
Show resolved Hide resolved
Without this flag you only get stats from `_all`.
Do not use it if you are pointing to localhost.
value:
type: boolean
example: false
- name: index_stats
description: Set "index_stats" to true to collect metrics for individual indices.
value:
Expand Down
3 changes: 3 additions & 0 deletions elastic/datadog_checks/elastic/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
'pshard_graceful_to',
'node_name_as_host',
'cluster_stats',
'detailed_index_stats',
'slm_stats',
'index_stats',
'service_check_tags',
Expand All @@ -39,6 +40,7 @@ def from_instance(instance):
node_name_as_host = is_affirmative(instance.get('node_name_as_host', False))
index_stats = is_affirmative(instance.get('index_stats', False))
cluster_stats = is_affirmative(instance.get('cluster_stats', False))
detailed_index_stats = is_affirmative(instance.get('detailed_index_stats', False))
slm_stats = is_affirmative(instance.get('slm_stats', False))
if 'is_external' in instance:
cluster_stats = is_affirmative(instance.get('is_external', False))
Expand Down Expand Up @@ -69,6 +71,7 @@ def from_instance(instance):
pshard_graceful_to=pshard_graceful_to,
node_name_as_host=node_name_as_host,
cluster_stats=cluster_stats,
detailed_index_stats=detailed_index_stats,
slm_stats=slm_stats,
index_stats=index_stats,
service_check_tags=service_check_tags,
Expand Down
4 changes: 4 additions & 0 deletions elastic/datadog_checks/elastic/config_models/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ def instance_connect_timeout(field, value):
return get_default_field_value(field, value)


def instance_detailed_index_stats(field, value):
return False


def instance_disable_generic_tags(field, value):
return False

Expand Down
1 change: 1 addition & 0 deletions elastic/datadog_checks/elastic/config_models/instance.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class Config:
cat_allocation_stats: Optional[bool]
cluster_stats: Optional[bool]
connect_timeout: Optional[float]
detailed_index_stats: Optional[bool]
disable_generic_tags: Optional[bool]
disable_legacy_cluster_tag: Optional[bool]
empty_default_hostname: Optional[bool]
Expand Down
8 changes: 8 additions & 0 deletions elastic/datadog_checks/elastic/data/conf.yaml.example
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,14 @@ instances:
#
# cluster_stats: false

## @param detailed_index_stats - boolean - optional - default: false
## This flag is to be used with `cluster_stats` and `pshard_stats` set to true and if
## you want to obtain index specific stats.
## Without this flag you only get stats from `_all`.
## Do not use it if you are pointing to localhost.
#
# detailed_index_stats: false

## @param index_stats - boolean - optional - default: false
## Set "index_stats" to true to collect metrics for individual indices.
#
Expand Down
23 changes: 20 additions & 3 deletions elastic/datadog_checks/elastic/elastic.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# (C) Datadog, Inc. 2018-present
# All rights reserved
# Licensed under Simplified BSD License (see LICENSE)
import re
import time
from collections import defaultdict

Expand Down Expand Up @@ -307,7 +308,23 @@ def _process_stats_data(self, data, stats_metrics, base_tags):

def _process_pshard_stats_data(self, data, pshard_stats_metrics, base_tags):
for metric, desc in iteritems(pshard_stats_metrics):
self._process_metric(data, metric, *desc, tags=base_tags)
pshard_tags = base_tags
if desc[1].startswith('_all.'):
pshard_tags = pshard_tags + ['index:_all']
self._process_metric(data, metric, *desc, tags=pshard_tags)
# process index-level metrics
if self._config.cluster_stats and self._config.detailed_index_stats:
for metric, desc in iteritems(pshard_stats_metrics):
if desc[1].startswith('_all.'):
for index in data['indices']:
self.log.debug("Processing index %s", index)
escaped_index = index.replace('.', '\.') # noqa: W605
index_desc = (
desc[0],
'indices.' + escaped_index + '.' + desc[1].replace('_all.', ''),
desc[2] if 2 < len(desc) else None,
)
self._process_metric(data, metric, *index_desc, tags=base_tags + ['index:' + index])

def _process_metric(self, data, metric, xtype, path, xform=None, tags=None, hostname=None):
"""
Expand All @@ -319,9 +336,9 @@ def _process_metric(self, data, metric, xtype, path, xform=None, tags=None, host
value = data

# Traverse the nested dictionaries
for key in path.split('.'):
for key in re.split(r'(?<!\\)\.', path):
if value is not None:
value = value.get(key)
value = value.get(key.replace('\\', ''))
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add some unit tests for process_metric since the behavior is changing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the creation of an index that starts with a dot "." and added a check for this new index to test_index_metrics and the new test I created called test_detailed_index_stats in commit 0ffcd75

else:
break

Expand Down
10 changes: 10 additions & 0 deletions elastic/tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ def test_from_instance_defaults():
assert c.pshard_stats is False
assert c.pshard_graceful_to is False
assert c.cluster_stats is False
assert c.detailed_index_stats is False
assert c.index_stats is False
assert c.service_check_tags == ['host:example.com', 'port:None']
assert c.tags == ['url:http://example.com']
Expand All @@ -38,18 +39,26 @@ def test_from_instance_cluster_stats():
assert c.cluster_stats is True


@pytest.mark.unit
def test_from_instance_detailed_index_stats():
c = from_instance({'url': 'http://example.com', 'detailed_index_stats': True})
assert c.detailed_index_stats is True


Copy link
Contributor

Choose a reason for hiding this comment

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

Can you also add a test for running the check when this parameter is enabled, and assert the expected tags?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sarah-witt thanks and I addressed this comment in commit 0ffcd75

@pytest.mark.unit
def test_from_instance():
instance = {
"username": "user",
"password": "pass",
"is_external": "yes",
"detailed_index_stats": "yes",
"url": "http://foo.bar",
"tags": ["a", "b:c"],
}
c = from_instance(instance)
assert c.admin_forwarder is False
assert c.cluster_stats is True
assert c.detailed_index_stats is True
assert c.url == "http://foo.bar"
assert c.tags == ["url:http://foo.bar", "a", "b:c"]
assert c.service_check_tags == ["host:foo.bar", "port:None", "a", "b:c"]
Expand All @@ -73,6 +82,7 @@ def test_from_instance():
c = from_instance(instance)
assert c.admin_forwarder is True
assert c.cluster_stats is False
assert c.detailed_index_stats is False
assert c.url == "https://foo.bar:9200"
assert c.tags == ["url:https://foo.bar:9200"]
assert c.service_check_tags == ["host:foo.bar", "port:9200"]