-
Notifications
You must be signed in to change notification settings - Fork 260
Backport MongoDB hosts array support to 3.x #879
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
base: 3.x
Are you sure you want to change the base?
Conversation
Backport MongoDB integration fix from main branch to resolve broken connection strings when using modern configuration format with hosts array instead of legacy host/port parameters. The current 3.x template generates malformed MongoDB URIs (mongodb://user:pass@:/admin with missing host:port) when users provide modern 'hosts' array parameters. This occurs because the template expects old-style 'host' and 'port' keys but receives 'hosts' array from modern configurations, causing nil values in the connection string. Changes: - Add dual-path template logic supporting both formats - Modern configs with 'hosts' array use hosts format (works correctly) - Legacy configs with 'host'/'port' use connection string (backward compatible) - Add support for new MongoDB monitoring parameters: - dbm: Database Monitoring feature - database_autodiscovery: Auto-discovery of databases - reported_database_hostname: Custom hostname for metrics - Update documentation to deprecate host/port in favor of hosts - Add RSpec tests for modern hosts array format - Update CHANGELOG for unreleased changes Template changes: - Check if server['hosts'] exists and has elements - If yes: render modern hosts array format with fields - If no: fall back to legacy connection string format - Add dbm, database_autodiscovery, reported_database_hostname rendering Documentation changes: - Move hosts, dbm, database_autodiscovery, reported_database_hostname to top - Mark host and port as deprecated with recommendation to use hosts - Update examples to show modern hosts array format - Add link to official Datadog MongoDB integration docs Test changes: - Add test context for hosts array configuration - Verify hosts array renders correctly - Verify all new parameters render correctly - Maintain backward compatibility with existing tests Tested: - pdk validate: PASSED - Full test suite: 4895 examples, 0 failures, 63 pending - Syntax validation: PASSED Refs: 9472fc7 Refs: DataDog#838 Signed-off-by: Jaremy Hatler <j.hatler@xsolla.com>
khewonc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The puppet 4 tests consistently fail with the error Comparing Symbols to non-Symbol values is deprecated. Could you look into a fix for that?
|
|
||
| # Unreleased | ||
|
|
||
| * [BUGFIX] Backport MongoDB hosts array support from main to fix broken connection strings with modern configuration format |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll add a changelog entry upon release, so this can be removed
| 'hosts' => ['localhost:27017'], | ||
| 'username' => 'user', | ||
| 'password' => 'pass', | ||
| 'database' => 'admin', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this database admin required for mongo? I noticed it was different from the original PR that was linked, but I'm not familiar enough with mongo to know what implications this has
| it { is_expected.to contain_file(conf_file).with_content(%r{- hosts:\s+- localhost:27017}) } | ||
| it { is_expected.to contain_file(conf_file).with_content(%r{username: user}) } | ||
| it { is_expected.to contain_file(conf_file).with_content(%r{password: pass}) } | ||
| it { is_expected.to contain_file(conf_file).with_content(%r{database: admin}) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same idea here
What does this PR do?
Backports the MongoDB
hostsarray support frommainbranch (PR #838, commit 9472fc7) to the3.xbranch to fix broken connection strings when using modern MongoDB configuration format.This resolves the issue where users providing modern
hostsarray parameters get malformed MongoDB URIs likemongodb://user:pass@:/admin(missing host:port).Motivation
Problem:
Organizations running the 3.x branch experience broken MongoDB monitoring when using modern configuration format. The template expects legacy
host/portparameters but receiveshostsarray, causing nil values that produce malformed connection strings.Impact:
host/portformat (inconsistent with current Datadog documentation)Why backport:
mainbranch (merged Feb 26, 2025)Additional Notes
Changes included in this backport:
Template (
templates/agent-conf.d/mongo.yaml.erb):hostsparameter existsdbm,database_autodiscovery,reported_database_hostnameManifest (
manifests/integrations/mongo.pp):hosts,dbm,database_autodiscovery,reported_database_hostnamehostandportparameters in favor ofhostsarrayTests (
spec/classes/datadog_agent_integrations_mongo_spec.rb):CHANGELOG (
CHANGELOG.md):Backward Compatibility:
✅ Fully backward compatible - legacy
host/portconfigurations continue to work unchangedOriginal implementation:
Describe your test plan
Automated Testing:
pdk validate --parallel: PASSED (only expected warnings for undocumented classes)Test scenarios covered: