Skip to content
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
1 change: 1 addition & 0 deletions postgres/changelog.d/21884.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed a bug where excluding "postgres" database when configuring autodiscovery would still fallback to "postgres" if `dbname` wasn't also configured
36 changes: 32 additions & 4 deletions postgres/datadog_checks/postgres/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,15 @@ def build_config(check: PostgreSql) -> Tuple[InstanceConfig, ValidationResult]:
if f in instance:
args[f] = instance[f]

# Build database_autodiscovery config first, as we may need it for dbname defaulting
database_autodiscovery_config = {
**dict_defaults.instance_database_autodiscovery().model_dump(),
**(instance.get('database_autodiscovery', {})),
}

if 'dbname' not in instance and database_autodiscovery_config.get('enabled'):
args['dbname'] = database_autodiscovery_config.get('global_view_db')

# Set values for args that have deprecated fallbacks, are not supported by the spec model
# or have other complexities
# If you change a literal value here, make sure to update spec.yaml
Expand All @@ -119,10 +128,7 @@ def build_config(check: PostgreSql) -> Tuple[InstanceConfig, ValidationResult]:
**dict_defaults.instance_database_identifier().model_dump(),
**(instance.get('database_identifier', {})),
},
"database_autodiscovery": {
**dict_defaults.instance_database_autodiscovery().model_dump(),
**(instance.get('database_autodiscovery', {})),
},
"database_autodiscovery": database_autodiscovery_config,
"query_metrics": {
**dict_defaults.instance_query_metrics().model_dump(),
**{
Expand Down Expand Up @@ -386,6 +392,28 @@ def validate_config(config: InstanceConfig, instance: dict, validation_result: V
'Use the `exclude_hostname` option instead.'
)

# Validate dbname is not excluded when using autodiscovery
# Only validate when dbname was NOT explicitly set by the user (auto-defaulted)
# If user explicitly set dbname, they may intentionally want to connect to an excluded database
# for global operations while excluding it from per-database metric collection
if config.database_autodiscovery.enabled and config.dbname and 'dbname' not in instance:
import re

# Guard against None - exclude could be explicitly set to null to remove default exclusions
for exclude_pattern in config.database_autodiscovery.exclude or ():
try:
# Note: Match is case-sensitive to match the behavior of the base Discovery Filter class
if re.search(exclude_pattern, config.dbname):
# Auto-defaulted dbname conflicts - suggest setting global_view_db
validation_result.add_error(
f'The default dbname "{config.dbname}" is excluded by autodiscovery pattern '
f'"{exclude_pattern}". Set database_autodiscovery.global_view_db to a '
f'non-excluded database.'
)
break
except re.error:
validation_result.add_warning(f'Invalid regex pattern in autodiscovery exclude: {exclude_pattern}')

# If the user provided config explicitly enables these features, we add a warning if dbm is not enabled
dbm_required = ['query_activity', 'query_samples', 'query_metrics', 'collect_settings', 'collect_schemas']
for feature in dbm_required:
Expand Down
165 changes: 165 additions & 0 deletions postgres/tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -381,3 +381,168 @@ def test_relations_validation_fails_if_schemas_is_wrong_type():
def test_relations_validation_fails_if_relkind_is_wrong_type():
with pytest.raises(ConfigurationError):
RelationsManager.validate_relations_config([{"relation_name": "person", "relkind": "foo"}])


def test_autodiscovery_dbname_defaults_to_global_view_db(mock_check):
# When autodiscovery is enabled and dbname is not explicitly set,
# dbname should default to global_view_db
instance = {
'host': 'localhost',
'port': 5432,
'username': 'testuser',
'password': 'testpass',
'database_autodiscovery': {
'enabled': True,
'global_view_db': 'main',
},
}
mock_check.instance = instance
mock_check.init_config = {}
config, result = build_config(check=mock_check)
assert result.valid
assert config.dbname == 'main'


def test_autodiscovery_dbname_respects_explicit_value(mock_check):
# When autodiscovery is enabled and dbname IS explicitly set,
# the explicit value should be used, even if it's in the exclude list
# This allows users to connect to a database for global operations
# while excluding it from per-database metric collection
instance = {
'host': 'localhost',
'port': 5432,
'username': 'testuser',
'password': 'testpass',
'dbname': 'postgres',
'database_autodiscovery': {
'enabled': True,
'global_view_db': 'postgres',
'exclude': ['postgres'],
},
}
mock_check.instance = instance
mock_check.init_config = {}
config, result = build_config(check=mock_check)
# Should be valid - user explicitly set dbname, so they know what they're doing
assert result.valid
assert config.dbname == 'postgres'


def test_autodiscovery_excluded_default_dbname_fails(mock_check):
# When autodiscovery is enabled, default dbname postgres is used,
# and postgres is in the exclude list, should fail validation
instance = {
'host': 'localhost',
'port': 5432,
'username': 'testuser',
'password': 'testpass',
'database_autodiscovery': {
'enabled': True,
'exclude': ['postgres'],
},
}
mock_check.instance = instance
mock_check.init_config = {}
config, result = build_config(check=mock_check)
assert not result.valid
assert any('is excluded by autodiscovery pattern' in str(e) for e in result.errors)
assert any('global_view_db' in str(e) for e in result.errors)


def test_autodiscovery_with_regex_exclude_pattern(mock_check):
# Test that regex patterns work correctly in exclude validation
instance = {
'host': 'localhost',
'port': 5432,
'username': 'testuser',
'password': 'testpass',
'database_autodiscovery': {
'enabled': True,
'global_view_db': 'test_main',
'exclude': ['test_.*'],
},
}
mock_check.instance = instance
mock_check.init_config = {}
config, result = build_config(check=mock_check)
assert not result.valid
assert any('is excluded by autodiscovery pattern' in str(e) for e in result.errors)


def test_autodiscovery_with_non_excluded_global_view_db_succeeds(mock_check):
# When autodiscovery is enabled with exclude patterns,
# but global_view_db doesn't match any of them, should succeed
instance = {
'host': 'localhost',
'port': 5432,
'username': 'testuser',
'password': 'testpass',
'database_autodiscovery': {
'enabled': True,
'global_view_db': 'production',
'exclude': ['postgres', 'test_.*'],
},
}
mock_check.instance = instance
mock_check.init_config = {}
config, result = build_config(check=mock_check)
assert result.valid
assert config.dbname == 'production'


def test_autodiscovery_case_sensitive_exclude_matching(mock_check):
instance = {
'host': 'localhost',
'port': 5432,
'username': 'testuser',
'password': 'testpass',
'database_autodiscovery': {
'enabled': True,
'global_view_db': 'POSTGRES',
'exclude': ['postgres'],
},
}
mock_check.instance = instance
mock_check.init_config = {}
config, result = build_config(check=mock_check)
# Should be valid - case-sensitive matching means POSTGRES != postgres
assert result.valid
assert config.dbname == 'POSTGRES'


def test_autodiscovery_invalid_regex_pattern_warns(mock_check):
# Invalid regex patterns should generate warnings
instance = {
'host': 'localhost',
'port': 5432,
'username': 'testuser',
'password': 'testpass',
'database_autodiscovery': {
'enabled': True,
'global_view_db': 'main',
'exclude': ['[invalid'], # Invalid regex
},
}
mock_check.instance = instance
mock_check.init_config = {}
config, result = build_config(check=mock_check)
# Should have a warning about invalid regex
assert any('Invalid regex pattern' in w for w in result.warnings)


def test_autodiscovery_exclude_none_does_not_error(mock_check):
instance = {
'host': 'localhost',
'port': 5432,
'username': 'testuser',
'password': 'testpass',
'database_autodiscovery': {
'enabled': True,
'global_view_db': 'main',
},
}
mock_check.instance = instance
mock_check.init_config = {}
config, result = build_config(check=mock_check)
assert result.valid
assert config.dbname == 'main'
Loading