DBMON-5625 Enable only_custom_queries for sqlserver #21304

edengorevoy · 2025-09-09T18:23:28Z

What does this PR do?

This PR enables only_custom_queries in the sqlserver integration. If enabled, the check will only execute queries defined in the custom_queries config option. (Note: dbm queries are not gated by this flag. It is assumed that if the check instance has only_custom_queries enabled, it will not have DBM enabled. This follows the MySQL implementation.)

This PR:

Splits out basic metric fetching from collect_metrics into its own function, load_basic_metrics, which takes the cursor as a parameter.
Updates collect_metrics to call load_basic_metrics and later submit them only if only_custom_queries is disabled.

Context:

In SQLServer custom_queries are handled by a QueryManager object. This has always been true in sqlserver: pr. For postgres the functionality was migrated to the QueryManager last year: pr.

The QueryManager object is inited with a list of custom queries and the instance config. In its own execute() logic, it flips through the queries that were passed in, checks only_custom_queries, and filter sout non-custom queries assuming they are also handled by the QueryManager. However, since we execute non-custom queries outside of the QueryManager, they are not filtered out. We need to check the config ourselves and gate these metrics manually in metric collection.

Testing

I spun up a local sqlserver instance with the following config:

            {"sqlserver": {
              "instances": [
                {
                  "host": "dbm-orders-local-sqlserver-primary-${USER}",
                  "port": 1433,
                  "username": "datadog",
                  "password": "SomethingElse$",
                  "connector": "odbc",
                  "driver": "ODBC Driver 18 for SQL Server",
                  "connection_string": "TrustServerCertificate=yes",
                  "tags": [
                    "env:local"
                  ],
                  "only_custom_queries": true,
                  "reported_hostname": "dbm-orders-local-sqlserver-primary-${USER}-custom-queries-only",
                  "custom_queries": [
                    {
                      "query": "SELECT 1;",
                      "columns": [
                        {"name": "custom_queries.test", "type": "gauge"}
                      ]
                    }
                  ]
                }
              ]
            }}

and ensured that sqlserver.custom_queries.test was recorded for that host but no other metrics.

Review checklist (to be filled by reviewers)

Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
Add the qa/skip-qa label if the PR doesn't need to be tested during QA.
If you need to backport this PR to another branch, you can add the backport/<branch-name> label to the PR and it will automatically open a backport PR once this one is merged

codecov · 2025-09-10T20:33:18Z

Codecov Report

❌ Patch coverage is 90.90909% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.31%. Comparing base (aaa9182) to head (b099de0).
⚠️ Report is 11 commits behind head on master.

Additional details and impacted files

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

sangeetashivaji · 2025-09-11T13:49:33Z

sqlserver/tests/test_integration.py

@@ -506,6 +506,41 @@ def test_custom_queries(aggregator, dd_run_check, instance_docker, custom_query,
        aggregator.assert_metric(metric_name, **kwargs)


+@pytest.mark.integration


I'm guessing when store procedure checks are enabled and when only_custom_queries are enabled we do not collect stored procedure metrics? Should we test that?
Also what happens when the only_custom_queries is set and no custom_queries are given?

If stored procedure checks are enabled and only_custom_queries is enabled, we would collect stored procedure metrics. The idea is that if a user has a check instance with only_custom_queries enabled they'd be removing the other collection parameters.

I think it would be good to validate these things in the config init though; I can add those situations as warning logs. What do you think?

also, will definitely add tests for that regardless

I think it would be good to validate these things in the config init though; I can add those situations as warning logs. What do you think?

Yeah I think we should definitely do that. Thank you!

Made the config validation change! I added a test for log output given a certain config, rather than adding extra only_custom_queries gates around dbm metrics, sproc metrics. Since we're technically leaking only_custom_queries outside of the QueryManager I figured it'd be best to gate with it only where there's no disabling config option (ie for base metrics only)

edengorevoy · 2025-09-12T13:29:54Z

/merge

dd-devflow-routing-codex · 2025-09-12T13:30:03Z

View all feedbacks in Devflow UI.

2025-09-12 13:30:02 UTC ℹ️ Start processing command /merge

2025-09-12 13:30:18 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

2025-09-12 13:51:57 UTC ⚠️ MergeQueue: This merge request was unqueued

[email protected] unqueued this merge request

eric-weaver

Left a couple of minor comments, I want to sync with @sethsamuel on the config changes before merging

eric-weaver · 2025-09-11T14:46:07Z

sqlserver/changelog.d/21304.fixed

@@ -0,0 +1 @@
+Support `only_custom_queries` configuration option in sqlserver integration.


This is a new config option. We need to change the changelog to added and not fixed

It's technically not a new config option since it exists in the sqlserver conf.yaml already & customers were reporting that it doesnt work as expected. But it's definitely new functionality if it works now so I see how added makes more sense

Ah I see yeah it looks like it was added to a bunch of configs in the past, but I don't see how it was ever actually wired up. I'm fine with fixed in that case

eric-weaver · 2025-09-12T13:21:11Z

sqlserver/datadog_checks/sqlserver/config.py

@@ -271,3 +274,21 @@ def _build_database_metrics_configs(self, instance):
                if value is not None:
                    config[key] = value
        return configurable_metrics
+
+    def _validate_only_custom_queries(self, instance):


Let's wait for @sethsamuel to return from PTO Monday on this one. We're in the process of reworking config handling / validation for surfacing agent health work. Want to make sure this will play nice with the pattern we're working towards

eric-weaver · 2025-09-12T13:31:50Z

sqlserver/tests/test_integration.py

+@pytest.mark.usefixtures('dd_environment')
+def test_only_custom_queries(aggregator, dd_run_check, instance_docker):
+    """Test that only_custom_queries=True skips regular metrics but executes custom queries."""
+    instance = copy(instance_docker)


I'll clean this up in a follow up since it's already done all over but just want to point out that this copy(instance_docker) is technically susceptible to mutation issues between tests because we're doing a shallow copy of a dictionary that we're regularly mutating deeper levels within the tests.

eric-weaver · 2025-09-12T13:35:30Z

sqlserver/tests/test_integration.py

+    # Verify that regular integration metrics are NOT collected
+    # (These would normally be collected by default)
+    aggregator.assert_metric('sqlserver.cache.hit_ratio', count=0)
+    aggregator.assert_metric('sqlserver.broker_activation.tasks_running', count=0)


we should be able to avoid asserting count=0 on these by calling aggregator.assert_all_metrics_covered() after your assertion on custom queries. This will ensure we've checked all metrics emitted explicitly and will fail if anything else goes through. It will be more future proof

edengorevoy · 2025-09-12T13:51:46Z

/merge -c

dd-devflow-routing-codex · 2025-09-12T13:51:50Z

View all feedbacks in Devflow UI.

2025-09-12 13:51:49 UTC ℹ️ Start processing command /merge -c

temporal-github-worker-1 bot added agent/review-requested ecosystems/review-requested product/review-requested labels Sep 9, 2025

datadog-agent-integrations-bot bot added the integration/sqlserver label Sep 9, 2025

edengorevoy force-pushed the eden.gorevoy/enable-only-custom-queries-option branch 3 times, most recently from ba14a59 to 991052a Compare September 10, 2025 19:50

datadog-agent-integrations-bot bot added the base_package label Sep 10, 2025

edengorevoy force-pushed the eden.gorevoy/enable-only-custom-queries-option branch from 8228037 to c36a70c Compare September 10, 2025 19:58

Enable only_custom_queries for sqlserver

302e263

edengorevoy force-pushed the eden.gorevoy/enable-only-custom-queries-option branch from c36a70c to 302e263 Compare September 10, 2025 20:01

Add test

9085766

edengorevoy force-pushed the eden.gorevoy/enable-only-custom-queries-option branch from 1612a16 to 9085766 Compare September 10, 2025 23:07

edengorevoy changed the title ~~Enable only_custom_queries for sqlserver~~ DBMON-5625 Enable only_custom_queries for sqlserver Sep 10, 2025

edengorevoy marked this pull request as ready for review September 10, 2025 23:47

edengorevoy requested review from a team as code owners September 10, 2025 23:47

datadog-agent-integrations-bot bot added team/agent-integrations team/database-monitoring-agent labels Sep 10, 2025

sangeetashivaji reviewed Sep 11, 2025

View reviewed changes

Add validation and validation tests

b099de0

sangeetashivaji approved these changes Sep 12, 2025

View reviewed changes

dd-devflow bot added the mergequeue-status: waiting label Sep 12, 2025

eric-weaver requested changes Sep 12, 2025

View reviewed changes

dd-devflow bot removed the mergequeue-status: waiting label Sep 12, 2025

dd-devflow bot added the mergequeue-status: removed label Sep 12, 2025

		@@ -506,6 +506,41 @@ def test_custom_queries(aggregator, dd_run_check, instance_docker, custom_query,
		aggregator.assert_metric(metric_name, **kwargs)


		@pytest.mark.integration

		@@ -0,0 +1 @@
		Support `only_custom_queries` configuration option in sqlserver integration.

DBMON-5625 Enable only_custom_queries for sqlserver #21304

Are you sure you want to change the base?

DBMON-5625 Enable only_custom_queries for sqlserver #21304

Uh oh!

Conversation

edengorevoy commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Context:

Testing

Review checklist (to be filled by reviewers)

Uh oh!

codecov bot commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sangeetashivaji Sep 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

edengorevoy commented Sep 12, 2025

Uh oh!

dd-devflow-routing-codex bot commented Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eric-weaver left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

edengorevoy commented Sep 12, 2025

Uh oh!

dd-devflow-routing-codex bot commented Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

edengorevoy commented Sep 9, 2025 •

edited

Loading

codecov bot commented Sep 10, 2025 •

edited

Loading

sangeetashivaji Sep 11, 2025 •

edited

Loading

dd-devflow-routing-codex bot commented Sep 12, 2025 •

edited

Loading

dd-devflow-routing-codex bot commented Sep 12, 2025 •

edited

Loading