Skip to content

Commit 5195ba9

Browse files
authored
[DOIO-36] Extend queryactions component to support SQL Server (#52372)
## Summary This PR adds SQL Server support to the Data Observability queryactions component. It schedules RC query payloads on the matching `sqlserver` check without changing the `DO_QUERY_ACTIONS` payload schema. It also makes shared instance selection and remainder reconciliation safer: - finds PostgreSQL, SAP HANA and SQL Server configs through one supported-integration path - keeps the matched integration name when it builds the scheduled check - rejects an empty `db_identifier.host` - rejects payloads that match more than one enabled local instance instead of selecting the first - stores the matched base-config instance ordinal, so reconciliation removes only that exact instance and keeps bundled siblings ## SQL Server matching Non-Azure SQL Server instances continue to match by host. Azure SQL Database instances match when: - `azure.deployment_type` is `sql_database` - the top-level instance `database` matches every query `dbname`, case-insensitively This prevents a host-shared payload from being scheduled against another Azure database. It also rejects payloads that contain mixed databases. ## Test plan - [x] `bazel test //comp/dataobs/queryactions/impl:impl_test_base` - [x] SQL Server check scheduling and disable or restore - [x] non-Azure host matching - [x] Azure shared-host database selection, case-insensitive matching and cross-database rejection - [x] same-host different-port and duplicate-instance ambiguity rejection - [x] exact ordinal remainder handling - [x] PostgreSQL and SAP HANA remainder regression coverage Closes https://linear.app/datadog/issue/DOIO-36 Co-authored-by: maciej.obuchowski <maciej.obuchowski@datadoghq.com>
1 parent ea2f403 commit 5195ba9

7 files changed

Lines changed: 490 additions & 75 deletions

File tree

comp/dataobs/queryactions/def/component.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,6 @@ package queryactions
1111
// Component is the Data Observability query actions component interface.
1212
// This component subscribes to RC DO_QUERY_ACTIONS product to receive declarative query configs,
1313
// each containing the full set of active monitor queries for a DB instance.
14-
// It injects data_observability config into matching postgres check instances.
15-
// Activates when a postgres instance with data_observability.enabled: true is detected.
14+
// It injects data_observability config into matching supported database check instances.
15+
// Activates when a supported integration instance with data_observability.enabled: true is detected.
1616
type Component interface{}

comp/dataobs/queryactions/impl/handler.go

Lines changed: 125 additions & 64 deletions
Large diffs are not rendered by default.

comp/dataobs/queryactions/impl/handler_test.go

Lines changed: 319 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -779,7 +779,7 @@ func TestFindMatchingConfig_TemplatedIdentifiersDistinguishPorts(t *testing.T) {
779779
Type: "self-hosted",
780780
Host: fmt.Sprintf("do-test-postgres-staging:%d", port),
781781
AgentHostname: "do-test-postgres-staging",
782-
})
782+
}, nil)
783783
require.NoError(t, err)
784784
assert.Equal(t, port, instance["port"])
785785
})
@@ -1041,6 +1041,67 @@ func TestOnRCUpdate_PortlessMatchedInstance_KeepsPortedSiblingInRemainder(t *tes
10411041
assert.Equal(t, siblingPort, remainderInstance["port"], "remainder must keep the ported sibling, not drop it via the portless matched identity")
10421042
}
10431043

1044+
// TestOnRCUpdate_SameHostDifferentPorts_RejectsAmbiguousMatch verifies that a literal SQL Server
1045+
// host cannot silently select the first of two enabled local instances that differ only by port.
1046+
func TestOnRCUpdate_SameHostDifferentPorts_RejectsAmbiguousMatch(t *testing.T) {
1047+
const sharedHost = "sqlserver.internal"
1048+
sqlserverCfg := integration.Config{
1049+
Name: "sqlserver",
1050+
Provider: "file",
1051+
Instances: []integration.Data{
1052+
integration.Data("host: " + sharedHost + "\nport: 1433\ndata_observability:\n enabled: true\n"),
1053+
integration.Data("host: " + sharedHost + "\nport: 1434\ndata_observability:\n enabled: true\n"),
1054+
},
1055+
}
1056+
c := newTestComponentWithAC(t, []integration.Config{sqlserverCfg})
1057+
1058+
payloadJSON, err := json.Marshal(DOQueryPayload{
1059+
ConfigID: "cfg-same-host",
1060+
DBIdentifier: DBIdentifier{Type: "self-hosted", Host: sharedHost},
1061+
Queries: []QuerySpec{{DBName: "master", Type: "run_query", Query: "SELECT 1", IntervalSeconds: 60, TimeoutSeconds: 10}},
1062+
})
1063+
require.NoError(t, err)
1064+
1065+
statuses, changes := collectStatuses(c, map[string]state.RawConfig{
1066+
"path/cfg-same-host": {Config: payloadJSON},
1067+
})
1068+
1069+
require.Equal(t, state.ApplyStateError, statuses["path/cfg-same-host"].State)
1070+
assert.Contains(t, statuses["path/cfg-same-host"].Error, "ambiguous SQL Server instance match")
1071+
assert.Empty(t, changes.Schedule)
1072+
assert.Empty(t, changes.Unschedule)
1073+
assert.Empty(t, c.activeConfigs)
1074+
}
1075+
1076+
// TestOnRCUpdate_DuplicateLocalInstances_RejectsAmbiguousMatch verifies that duplicate enabled
1077+
// local instances are rejected instead of resolving to whichever one appears first.
1078+
func TestOnRCUpdate_DuplicateLocalInstances_RejectsAmbiguousMatch(t *testing.T) {
1079+
const instanceYAML = "host: duplicate.example.com\nport: 1433\ndata_observability:\n enabled: true\n"
1080+
sqlserverCfg := integration.Config{
1081+
Name: "sqlserver",
1082+
Provider: "file",
1083+
Instances: []integration.Data{integration.Data(instanceYAML), integration.Data(instanceYAML)},
1084+
}
1085+
c := newTestComponentWithAC(t, []integration.Config{sqlserverCfg})
1086+
1087+
payloadJSON, err := json.Marshal(DOQueryPayload{
1088+
ConfigID: "cfg-duplicate",
1089+
DBIdentifier: DBIdentifier{Type: "self-hosted", Host: "duplicate.example.com"},
1090+
Queries: []QuerySpec{{DBName: "master", Type: "run_query", Query: "SELECT 1", IntervalSeconds: 60, TimeoutSeconds: 10}},
1091+
})
1092+
require.NoError(t, err)
1093+
1094+
statuses, changes := collectStatuses(c, map[string]state.RawConfig{
1095+
"path/cfg-duplicate": {Config: payloadJSON},
1096+
})
1097+
1098+
require.Equal(t, state.ApplyStateError, statuses["path/cfg-duplicate"].State)
1099+
assert.Contains(t, statuses["path/cfg-duplicate"].Error, "ambiguous SQL Server instance match")
1100+
assert.Empty(t, changes.Schedule)
1101+
assert.Empty(t, changes.Unschedule)
1102+
assert.Empty(t, c.activeConfigs)
1103+
}
1104+
10441105
// TestOnRCUpdate_MultipleDOConfigsSameBase verifies that two DO configs targeting two different
10451106
// instances of the same base config never leave an instance both in the remainder and as a DO
10461107
// check (which would double-run it). With both instances targeted, no remainder is scheduled.
@@ -1203,8 +1264,8 @@ func TestBuildCheckConfig_PerQueryDBName(t *testing.T) {
12031264
}
12041265

12051266
// TestOnRCUpdate_MalformedPostgresYAML_SurfacesParseError verifies that when a postgres
1206-
// instance's YAML is malformed, the error message from findPostgresConfig mentions the
1207-
// parse failure, not just "identifier not found".
1267+
// instance's YAML is malformed, the error message from findPostgresConfig mentions
1268+
// the parse failure, not just "identifier not found".
12081269
func TestOnRCUpdate_MalformedPostgresYAML_SurfacesParseError(t *testing.T) {
12091270
postgresCfg := integration.Config{
12101271
Name: "postgres",
@@ -1516,3 +1577,258 @@ func TestOnRCUpdate_SecondUpdateReusesStoredBase(t *testing.T) {
15161577
assert.NotEmpty(t, queries, "scheduled config has no DO queries — looks like the wrongly-restored original")
15171578
}
15181579
}
1580+
1581+
// --- SQL Server tests ---
1582+
1583+
// TestOnRCUpdate_SQLServer_SchedulesCheck verifies that a sqlserver integration config
1584+
// is matched and scheduled using the correct integration name (not "postgres").
1585+
func TestOnRCUpdate_SQLServer_SchedulesCheck(t *testing.T) {
1586+
sqlserverCfg := integration.Config{
1587+
Name: "sqlserver",
1588+
Provider: "file",
1589+
NodeName: "node1",
1590+
Instances: []integration.Data{
1591+
integration.Data("host: sqlserver.example.com\nport: 1433\nusername: datadog\ndata_observability:\n enabled: true\n"),
1592+
},
1593+
}
1594+
c := newTestComponentWithAC(t, []integration.Config{sqlserverCfg})
1595+
1596+
payload := DOQueryPayload{
1597+
ConfigID: "cfg-sqlserver",
1598+
DBIdentifier: DBIdentifier{Type: "self-hosted", Host: "sqlserver.example.com"},
1599+
Queries: []QuerySpec{
1600+
{
1601+
MonitorID: 10,
1602+
Type: "run_query",
1603+
Query: "SELECT count(*) FROM sys.tables",
1604+
IntervalSeconds: 60,
1605+
TimeoutSeconds: 10,
1606+
Entity: EntityMetadata{Platform: "sqlserver", Database: "master", Table: "sys.tables"},
1607+
},
1608+
},
1609+
}
1610+
payloadJSON, err := json.Marshal(payload)
1611+
require.NoError(t, err)
1612+
1613+
statuses, changes := collectStatuses(c, map[string]state.RawConfig{
1614+
"path/cfg-sqlserver": {Config: payloadJSON, Metadata: state.Metadata{ID: "rc-sqlserver"}},
1615+
})
1616+
1617+
require.Equal(t, state.ApplyStateAcknowledged, statuses["path/cfg-sqlserver"].State)
1618+
require.Len(t, changes.Schedule, 1)
1619+
assert.Equal(t, "sqlserver", changes.Schedule[0].Name, "scheduled check must use sqlserver integration name")
1620+
assert.Equal(t, "file", changes.Schedule[0].Provider)
1621+
assert.Equal(t, "node1", changes.Schedule[0].NodeName)
1622+
require.Contains(t, c.activeConfigs, "cfg-sqlserver")
1623+
}
1624+
1625+
// TestMatchesIdentifier_SQLServer_HostOnly verifies that a self-hosted or Azure VM SQL Server
1626+
// instance matches by host only (no Azure SQL DB deployment_type present).
1627+
func TestMatchesIdentifier_SQLServer_HostOnly(t *testing.T) {
1628+
instance := map[string]any{
1629+
"host": "sqlserver.internal",
1630+
"port": 1433,
1631+
}
1632+
1633+
t.Run("matching host with different query databases", func(t *testing.T) {
1634+
dbID := &DBIdentifier{Type: "self-hosted", Host: "sqlserver.internal"}
1635+
queries := []QuerySpec{{DBName: "master"}, {DBName: "msdb"}}
1636+
assert.True(t, evaluateInstanceIdentifier(instance, *dbID, "sqlserver", queries).matched)
1637+
})
1638+
1639+
t.Run("mismatching host", func(t *testing.T) {
1640+
dbID := &DBIdentifier{Type: "self-hosted", Host: "other.internal"}
1641+
assert.False(t, evaluateInstanceIdentifier(instance, *dbID, "sqlserver", nil).matched)
1642+
})
1643+
}
1644+
1645+
// TestMatchesIdentifier_AzureSQLDB_HostAndDatabase verifies that Azure SQL Database instances
1646+
// require host equality and case-insensitive database equality for every query.
1647+
func TestMatchesIdentifier_AzureSQLDB_HostAndDatabase(t *testing.T) {
1648+
instance := map[string]any{
1649+
"host": "myserver.database.windows.net",
1650+
"database": "MyDB",
1651+
"azure": map[string]any{
1652+
"deployment_type": "sql_database",
1653+
},
1654+
}
1655+
dbID := &DBIdentifier{Type: "self-hosted", Host: "myserver.database.windows.net"}
1656+
1657+
t.Run("host and same-case query databases match", func(t *testing.T) {
1658+
queries := []QuerySpec{{DBName: "MyDB"}, {DBName: "MyDB"}}
1659+
assert.True(t, evaluateInstanceIdentifier(instance, *dbID, "sqlserver", queries).matched)
1660+
})
1661+
1662+
t.Run("host matches but query database does not", func(t *testing.T) {
1663+
queries := []QuerySpec{{DBName: "OtherDB"}}
1664+
assert.False(t, evaluateInstanceIdentifier(instance, *dbID, "sqlserver", queries).matched)
1665+
})
1666+
1667+
t.Run("database matching is case-insensitive", func(t *testing.T) {
1668+
queries := []QuerySpec{{DBName: "mydb"}}
1669+
assert.True(t, evaluateInstanceIdentifier(instance, *dbID, "sqlserver", queries).matched, "MyDB must match mydb")
1670+
})
1671+
1672+
t.Run("mixed query databases do not match", func(t *testing.T) {
1673+
queries := []QuerySpec{{DBName: "MyDB"}, {DBName: "OtherDB"}}
1674+
assert.False(t, evaluateInstanceIdentifier(instance, *dbID, "sqlserver", queries).matched)
1675+
1676+
otherInstance := map[string]any{
1677+
"host": "myserver.database.windows.net",
1678+
"database": "OtherDB",
1679+
"azure": map[string]any{
1680+
"deployment_type": "sql_database",
1681+
},
1682+
}
1683+
assert.False(t, evaluateInstanceIdentifier(otherInstance, *dbID, "sqlserver", queries).matched)
1684+
})
1685+
1686+
t.Run("empty queries do not match", func(t *testing.T) {
1687+
assert.False(t, evaluateInstanceIdentifier(instance, *dbID, "sqlserver", nil).matched)
1688+
})
1689+
1690+
t.Run("host does not match", func(t *testing.T) {
1691+
otherDBID := &DBIdentifier{Type: "self-hosted", Host: "otherserver.database.windows.net"}
1692+
assert.False(t, evaluateInstanceIdentifier(instance, *otherDBID, "sqlserver", []QuerySpec{{DBName: "MyDB"}}).matched)
1693+
})
1694+
}
1695+
1696+
// TestOnRCUpdate_EmptyIdentifierHost verifies that an empty db_identifier.host is rejected before
1697+
// any local instance can be selected.
1698+
func TestOnRCUpdate_EmptyIdentifierHost(t *testing.T) {
1699+
sqlserverCfg := integration.Config{
1700+
Name: "sqlserver",
1701+
Provider: "file",
1702+
Instances: []integration.Data{integration.Data("host: sqlserver.example.com\ndata_observability:\n enabled: true\n")},
1703+
}
1704+
c := newTestComponentWithAC(t, []integration.Config{sqlserverCfg})
1705+
1706+
payloadJSON, err := json.Marshal(DOQueryPayload{
1707+
ConfigID: "cfg-empty-host",
1708+
DBIdentifier: DBIdentifier{Type: "self-hosted"},
1709+
Queries: []QuerySpec{{DBName: "master", Type: "run_query", Query: "SELECT 1", IntervalSeconds: 60, TimeoutSeconds: 10}},
1710+
})
1711+
require.NoError(t, err)
1712+
1713+
statuses, changes := collectStatuses(c, map[string]state.RawConfig{
1714+
"path/cfg-empty-host": {Config: payloadJSON},
1715+
})
1716+
1717+
require.Equal(t, state.ApplyStateError, statuses["path/cfg-empty-host"].State)
1718+
assert.Equal(t, "empty db_identifier.host", statuses["path/cfg-empty-host"].Error)
1719+
assert.Empty(t, changes.Schedule)
1720+
assert.Empty(t, changes.Unschedule)
1721+
}
1722+
1723+
// TestOnRCUpdate_AzureSQLDB_RejectsCrossDBPayload verifies that an RC payload targeting
1724+
// another database cannot be injected into a MyDB instance sharing the same Azure SQL Server host.
1725+
func TestOnRCUpdate_AzureSQLDB_RejectsCrossDBPayload(t *testing.T) {
1726+
sqlserverCfg := integration.Config{
1727+
Name: "sqlserver",
1728+
Provider: "file",
1729+
Instances: []integration.Data{
1730+
integration.Data("host: myserver.database.windows.net\ndatabase: MyDB\nazure:\n deployment_type: sql_database\ndata_observability:\n enabled: true\n"),
1731+
},
1732+
}
1733+
c := newTestComponentWithAC(t, []integration.Config{sqlserverCfg})
1734+
1735+
payload := DOQueryPayload{
1736+
ConfigID: "cfg-wrongdb",
1737+
DBIdentifier: DBIdentifier{Type: "self-hosted", Host: "myserver.database.windows.net"},
1738+
Queries: []QuerySpec{{DBName: "OtherDB", Type: "run_query", Query: "SELECT 1", IntervalSeconds: 60, TimeoutSeconds: 10}},
1739+
}
1740+
payloadJSON, err := json.Marshal(payload)
1741+
require.NoError(t, err)
1742+
1743+
statuses, changes := collectStatuses(c, map[string]state.RawConfig{
1744+
"path/cfg-wrongdb": {Config: payloadJSON},
1745+
})
1746+
1747+
assert.Equal(t, state.ApplyStateError, statuses["path/cfg-wrongdb"].State)
1748+
assert.Empty(t, changes.Schedule)
1749+
assert.Empty(t, c.activeConfigs)
1750+
}
1751+
1752+
// TestOnRCUpdate_AzureSQLDB_PreservesSiblingDatabase checks that targeting one Azure SQL
1753+
// database does not remove another database that shares the server host.
1754+
func TestOnRCUpdate_AzureSQLDB_PreservesSiblingDatabase(t *testing.T) {
1755+
const host = "myserver.database.windows.net"
1756+
sqlserverCfg := integration.Config{
1757+
Name: "sqlserver",
1758+
Provider: "file",
1759+
Instances: []integration.Data{
1760+
integration.Data("host: " + host + "\ndatabase: MyDB\nazure:\n deployment_type: sql_database\ndata_observability:\n enabled: true\n"),
1761+
integration.Data("host: " + host + "\ndatabase: OtherDB\nazure:\n deployment_type: sql_database\ndata_observability:\n enabled: true\n"),
1762+
},
1763+
}
1764+
c := newTestComponentWithAC(t, []integration.Config{sqlserverCfg})
1765+
1766+
payload := DOQueryPayload{
1767+
ConfigID: "cfg-my-db",
1768+
DBIdentifier: DBIdentifier{Type: "self-hosted", Host: host},
1769+
Queries: []QuerySpec{{DBName: "mydb", Type: "run_query", Query: "SELECT 1", IntervalSeconds: 60, TimeoutSeconds: 10}},
1770+
}
1771+
payloadJSON, err := json.Marshal(payload)
1772+
require.NoError(t, err)
1773+
1774+
statuses, changes := collectStatuses(c, map[string]state.RawConfig{
1775+
"path/cfg-my-db": {Config: payloadJSON},
1776+
})
1777+
1778+
require.Equal(t, state.ApplyStateAcknowledged, statuses["path/cfg-my-db"].State)
1779+
require.Len(t, changes.Unschedule, 1, "should unschedule the original two-instance config")
1780+
require.Len(t, changes.Schedule, 2, "should schedule the targeted check and the sibling remainder")
1781+
1782+
databasesWithQueries := make(map[string]bool)
1783+
for _, cfg := range changes.Schedule {
1784+
for _, instanceData := range cfg.Instances {
1785+
var instance map[string]any
1786+
require.NoError(t, yaml.Unmarshal(instanceData, &instance))
1787+
database, _ := instance["database"].(string)
1788+
dataObservability, ok := instance["data_observability"].(map[string]any)
1789+
require.True(t, ok)
1790+
_, hasQueries := dataObservability["queries"]
1791+
databasesWithQueries[database] = hasQueries
1792+
}
1793+
}
1794+
1795+
assert.True(t, databasesWithQueries["MyDB"], "the targeted database must receive queries")
1796+
assert.False(t, databasesWithQueries["OtherDB"], "the sibling must remain unchanged")
1797+
}
1798+
1799+
// TestOnRCUpdate_SQLServer_DisableRestoresOriginalConfig verifies that sending an empty
1800+
// queries list for a previously active SQL Server config re-schedules the original config.
1801+
func TestOnRCUpdate_SQLServer_DisableRestoresOriginalConfig(t *testing.T) {
1802+
sqlserverCfg := integration.Config{
1803+
Name: "sqlserver",
1804+
Provider: "file",
1805+
NodeName: "node1",
1806+
Instances: []integration.Data{
1807+
integration.Data("host: sqlserver.example.com\nport: 1433\ndata_observability:\n enabled: true\n"),
1808+
},
1809+
}
1810+
c := newTestComponentWithAC(t, []integration.Config{sqlserverCfg})
1811+
1812+
// First: schedule a DO config so the base config becomes managed.
1813+
enable := DOQueryPayload{
1814+
ConfigID: "cfg-sqlserver-disable",
1815+
DBIdentifier: DBIdentifier{Type: "self-hosted", Host: "sqlserver.example.com"},
1816+
Queries: []QuerySpec{{Type: "run_query", Query: "SELECT 1", IntervalSeconds: 60, TimeoutSeconds: 10}},
1817+
}
1818+
enableJSON, err := json.Marshal(enable)
1819+
require.NoError(t, err)
1820+
collectStatuses(c, map[string]state.RawConfig{"path/config": {Config: enableJSON}})
1821+
require.Contains(t, c.activeConfigs, "cfg-sqlserver-disable")
1822+
1823+
// Now: empty queries disables the DO config and restores the original sqlserver config.
1824+
statuses, changes := collectStatuses(c, map[string]state.RawConfig{
1825+
"path/config": {Config: []byte(`{"config_id": "cfg-sqlserver-disable", "queries": []}`)},
1826+
})
1827+
1828+
assert.Equal(t, state.ApplyStateAcknowledged, statuses["path/config"].State)
1829+
assert.Empty(t, c.activeConfigs)
1830+
require.Len(t, changes.Unschedule, 1, "should unschedule the DO sqlserver check")
1831+
require.Len(t, changes.Schedule, 1, "should re-schedule the original base sqlserver config")
1832+
assert.Equal(t, sqlserverCfg, changes.Schedule[0])
1833+
assert.Equal(t, "sqlserver", changes.Schedule[0].Name)
1834+
}

comp/dataobs/queryactions/impl/queryactions.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ type component struct {
4444
rcclient rcclient.Component
4545
// activeConfigs maps a DO config_id to the DO check config currently scheduled for it.
4646
activeConfigs map[string]activeConfigEntry
47-
// managedBases maps a base postgres config Digest to the bookkeeping needed to restore it.
47+
// managedBases maps a base integration config Digest to the bookkeeping needed to restore it.
4848
// A base config has an entry here while at least one DO config targets one of its instances;
4949
// the entry records the original config (for restoration) and the remainder config currently
5050
// scheduled in its place. See reconcileBases.
@@ -184,6 +184,9 @@ func (c *component) Stream(ctx context.Context) <-chan integration.ConfigChanges
184184
// data_observability.enabled: true is configured in autodiscovery.
185185
func (c *component) hasSupportedIntegration() bool {
186186
for _, cfg := range c.ac.GetUnresolvedConfigs() {
187+
if !isSupportedIntegration(cfg.Name) {
188+
continue
189+
}
187190
for _, instanceData := range cfg.Instances {
188191
var instance map[string]any
189192
if err := yaml.Unmarshal(instanceData, &instance); err != nil {

0 commit comments

Comments
 (0)