Skip to content

Add option to lowercase "device" tag #19964

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

Merged
merged 6 commits into from
Apr 1, 2025
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
12 changes: 11 additions & 1 deletion disk/assets/configuration/spec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ files:
Works on Linux only.
value:
example: false
type: boolean
type: boolean
- name: device_tag_re
description: |
Instruct the check to apply additional tags to matching
Expand Down Expand Up @@ -263,3 +263,13 @@ files:
type: string
- name: type
type: string

- name: lowercase_device_tag
description: |
Enable this to lowercase the "device" tags for both partition and disk metrics.
This is useful only in very specific circumstances:
1. Your "device" tag value is uppercase and your host is running on Linux.
2. You cannot use the "device_name" tag.
value:
type: boolean
example: false
1 change: 1 addition & 0 deletions disk/changelog.d/19964.added
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add option to lowercase "device" tag. Useful for those who cannot use the "device_name" tag and need "device" to only be lowercase.
4 changes: 4 additions & 0 deletions disk/datadog_checks/disk/config_models/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ def instance_include_all_devices():
return True


def instance_lowercase_device_tag():
return False


def instance_min_collection_interval():
return 15

Expand Down
1 change: 1 addition & 0 deletions disk/datadog_checks/disk/config_models/instance.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ class InstanceConfig(BaseModel):
file_system_exclude: Optional[tuple[str, ...]] = None
file_system_include: Optional[tuple[str, ...]] = None
include_all_devices: Optional[bool] = None
lowercase_device_tag: Optional[bool] = None
metric_patterns: Optional[MetricPatterns] = None
min_collection_interval: Optional[float] = None
min_disk_size: Optional[float] = None
Expand Down
8 changes: 8 additions & 0 deletions disk/datadog_checks/disk/data/conf.yaml.default
Original file line number Diff line number Diff line change
Expand Up @@ -252,3 +252,11 @@ instances:
# host: nfsserver
# share: /mnt/nfs_share
# type: nfs

## @param lowercase_device_tag - boolean - optional - default: false
## Enable this to lowercase the "device" tags for both partition and disk metrics.
## This is useful only in very specific circumstances:
## 1. Your "device" tag value is uppercase and your host is running on Linux.
## 2. You cannot use the "device_name" tag.
#
# lowercase_device_tag: false
5 changes: 3 additions & 2 deletions disk/datadog_checks/disk/disk.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ def __init__(self, name, init_config, instances):
self._compile_pattern_filters(instance)
self._compile_tag_re()
self._blkid_label_re = re.compile('LABEL=\"(.*?)\"', re.I)
self._lowercase_device_tag = is_affirmative(instance.get('lowercase_device_tag', False))

if self._use_lsblk and self._blkid_cache_file:
raise ConfigurationError("Only one of 'use_lsblk' and 'blkid_cache_file' can be set at the same time.")
Expand Down Expand Up @@ -183,7 +184,7 @@ def _get_tags(self, part):
if Platform.is_win32():
device_name = device_name.strip('\\').lower()

tags.append('device:{}'.format(device_name))
tags.append('device:{}'.format(device_name.lower() if self._lowercase_device_tag else device_name))
tags.append('device_name:{}'.format(_base_device_name(part.device)))
return tags

Expand Down Expand Up @@ -331,7 +332,7 @@ def collect_latency_metrics(self):
device_specific_tags = self._get_device_specific_tags(disk_name)
metric_tags.extend(device_specific_tags)

metric_tags.append('device:{}'.format(disk_name))
metric_tags.append('device:{}'.format(disk_name.lower() if self._lowercase_device_tag else disk_name))
metric_tags.append('device_name:{}'.format(_base_device_name(disk_name)))
if self.devices_label.get(disk_name):
metric_tags.extend(self.devices_label.get(disk_name))
Expand Down
56 changes: 55 additions & 1 deletion disk/tests/test_unit.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

from datadog_checks.base.utils.platform import Platform
from datadog_checks.base.utils.timeout import TimeoutException
from datadog_checks.dev.testing import requires_windows
from datadog_checks.dev.testing import requires_linux, requires_windows
from datadog_checks.dev.utils import ON_WINDOWS, get_metadata_metrics, mock_context_manager
from datadog_checks.disk import Disk
from datadog_checks.disk.disk import IGNORE_CASE
Expand Down Expand Up @@ -223,6 +223,60 @@ def test_use_mount(aggregator, instance_basic_mount, gauge_metrics, rate_metrics
aggregator.assert_metrics_using_metadata(get_metadata_metrics())


@requires_linux # Only linux cares about upper/lower case for paths.
@pytest.mark.parametrize(
'lc_device_tag, expected_dev_path',
[
pytest.param(None, '//CIFS/DEV1', id='Default behavior, keep case'),
pytest.param(False, '//CIFS/DEV1', id='Disabled, keep case'),
pytest.param(True, '//cifs/dev1', id='Enabled, lowercase'),
],
)
def test_lowercase_device_tag(
aggregator,
instance_basic_volume,
gauge_metrics,
rate_metrics,
count_metrics,
dd_run_check,
mocker,
lc_device_tag,
expected_dev_path,
):
"""
If explicitly configured, we will lowercase the "device" tag value.

For metrics with uppercase "device" tag values, our backend introduces a lowercase version of the tag.
Some customers don't like this and cannot use the "device_name" tag instead.
They requested that we add a switch for them to lowercase the "device" tag before we submit it.
This flag MUST be off by default to avoid breaking anyone else.
"""
if lc_device_tag is not None:
instance_basic_volume['lowercase_device_tag'] = lc_device_tag
c = Disk('disk', {}, [instance_basic_volume])

if ON_WINDOWS:
mock_statvfs = mock_context_manager()
else:
mock_statvfs = mock.patch('os.statvfs', return_value=MockInodesMetrics(), __name__='statvfs')

full_dev_path = '//CIFS/DEV1'
mocker.patch(
'psutil.disk_partitions',
return_value=[MockPart(device=full_dev_path, fstype="cifs")],
__name__='disk_partitions',
)
mocker.patch('psutil.disk_usage', return_value=MockDiskMetrics(), __name__='disk_usage')
mocker.patch('psutil.disk_io_counters', return_value=MockDiskIOMetrics(full_dev_path))
with mock_statvfs:
dd_run_check(c)

for name in chain(gauge_metrics, rate_metrics, count_metrics):
aggregator.assert_metric_has_tag(name, f'device:{expected_dev_path}')
# Make sure the "device_name" tag isn't affected.
aggregator.assert_metric_has_tag(name, 'device_name:DEV1')


@pytest.mark.usefixtures('psutil_mocks')
def test_device_tagging(aggregator, gauge_metrics, rate_metrics, count_metrics, dd_run_check):
instance = {
Expand Down
Loading