Skip to content

Commit e689136

Browse files
gabedosvitkyrka
andauthored
[autodiscovery] Advanced Discovery Probe Workqueue (#51890)
### What does this PR do? Wires Configuration Discovery into Autodiscovery behind a `ConfigDiscoverer` interface. Templates carrying `discovery: {}` route through a workqueue-driven worker pool instead of the synchronous `configresolver.Resolve` path in cfgmgr. The feature is dormant pending the Python-backed bridge from #51501. ### Motivation Foundation for Configuration Discovery for Agent Integrations (both for Containers + Processes). This delegate config creation to the Integration itself. The Integration can decided which ports telemetry exists on to be monitored. ### Describe how you validated your changes Unit Tests. There is currently no e2e functionality, however, the unit tests show how this will be hooked in once the real implementation for DiscoveryConfig from the Python Integrations is connected. ### Additional Notes Thoughts: `onDiscoveryResult` sends the changes through a channel `cm.discoveredCh <- changes` to be picked up by AutoConfig to actually apply the changes onto the Scheduler. During this gap, there could in theory be a race if the service gets deleted but the Discovery result could still apply. To make this atomic, we would need to push down into cfgmgr a callback for autoconfig to apply the changes there too or maybe share the same channel for processing this. Might do this in a separate PR to keep this one simpler. Co-authored-by: vitkyrka <vincent.whitchurch@datadoghq.com>
1 parent 6f88da9 commit e689136

16 files changed

Lines changed: 1775 additions & 11 deletions
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
load("@rules_go//go:def.bzl", "go_library", "go_test")
2+
3+
go_library(
4+
name = "discoverer",
5+
srcs = [
6+
"discovery_json.go",
7+
"python_bridge.go", # //go:build python
8+
"python_bridge_nopython.go", # //go:build !python
9+
"types.go",
10+
"worker.go",
11+
],
12+
importpath = "github.com/DataDog/datadog-agent/comp/core/autodiscovery/discoverer",
13+
tags = ["manual"],
14+
visibility = ["//visibility:public"],
15+
deps = [
16+
"//comp/core/autodiscovery/integration",
17+
"//comp/core/workloadmeta/def",
18+
"//pkg/util/log",
19+
"@//pkg/collector/python",
20+
"@io_k8s_apimachinery//pkg/util/wait",
21+
"@io_k8s_client_go//util/workqueue",
22+
],
23+
)
24+
25+
go_test(
26+
name = "discoverer_test",
27+
srcs = [
28+
"discovery_json_test.go",
29+
"fake_service_test.go",
30+
"worker_test.go",
31+
],
32+
embed = [":discoverer"],
33+
gotags = ["test"],
34+
tags = ["manual"],
35+
deps = [
36+
"//comp/core/autodiscovery/integration",
37+
"//comp/core/workloadmeta/def",
38+
"@com_github_stretchr_testify//assert",
39+
"@com_github_stretchr_testify//require",
40+
"@org_uber_go_atomic//:atomic",
41+
],
42+
)
Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
// Unless explicitly stated otherwise all files in this repository are licensed
2+
// under the Apache License Version 2.0.
3+
// This product includes software developed at Datadog (https://www.datadoghq.com/).
4+
// Copyright 2016-present Datadog, Inc.
5+
6+
package discoverer
7+
8+
import (
9+
"encoding/json"
10+
"fmt"
11+
12+
"github.com/DataDog/datadog-agent/comp/core/autodiscovery/integration"
13+
)
14+
15+
// discoveryService is the JSON payload sent to the integration when asking it
16+
// to discover its config for a given service.
17+
type discoveryService struct {
18+
ID string `json:"id"`
19+
Host string `json:"host"`
20+
Ports []discoveryPort `json:"ports"`
21+
}
22+
23+
type discoveryPort struct {
24+
Number int `json:"number"`
25+
Name string `json:"name"`
26+
}
27+
28+
// discoveredConfig is the JSON shape returned by the integration.
29+
type discoveredConfig struct {
30+
Instances []json.RawMessage `json:"instances"`
31+
InitConfig json.RawMessage `json:"init_config"`
32+
MetricConfig json.RawMessage `json:"metric_config"`
33+
LogsConfig json.RawMessage `json:"logs"`
34+
IgnoreAutodiscoveryTags bool `json:"ignore_autodiscovery_tags"`
35+
CheckTagCardinality string `json:"check_tag_cardinality"`
36+
}
37+
38+
// marshalService builds the JSON payload sent to the integration for the
39+
// given live service. Returns ("", false, nil) when the service has no
40+
// usable host yet — typical during container startup, treated by callers as
41+
// a transient failure that warrants a retry.
42+
func marshalService(svc ServiceInfo) (string, bool, error) {
43+
hosts, err := svc.GetHosts()
44+
if err != nil {
45+
return "", false, nil
46+
}
47+
host, ok := pickHost(hosts)
48+
if !ok {
49+
return "", false, nil
50+
}
51+
exposed, err := svc.GetPorts()
52+
if err != nil {
53+
return "", false, fmt.Errorf("GetPorts: %w", err)
54+
}
55+
payload := discoveryService{
56+
ID: svc.GetServiceID(),
57+
Host: host,
58+
Ports: make([]discoveryPort, 0, len(exposed)),
59+
}
60+
for _, p := range exposed {
61+
payload.Ports = append(payload.Ports, discoveryPort{Number: p.Port, Name: p.Name})
62+
}
63+
b, err := json.Marshal(payload)
64+
if err != nil {
65+
return "", false, fmt.Errorf("marshal: %w", err)
66+
}
67+
return string(b), true, nil
68+
}
69+
70+
// pickHost applies the same fallback policy as %%host%%: single network →
71+
// use it; multiple networks with a "bridge" → use bridge; otherwise no
72+
// deterministic choice is possible and we return false.
73+
func pickHost(hosts map[string]string) (string, bool) {
74+
if len(hosts) == 0 {
75+
return "", false
76+
}
77+
if len(hosts) == 1 {
78+
for _, ip := range hosts {
79+
return ip, true
80+
}
81+
}
82+
if ip, ok := hosts["bridge"]; ok {
83+
return ip, true
84+
}
85+
return "", false
86+
}
87+
88+
// parseDiscoveryResult turns the raw JSON returned by ConfigDiscoverer into a
89+
// slice of integration.Config. The configs returned here are not yet resolved
90+
// through configresolver — the caller is expected to run them through the
91+
// normal substitution + secret-decryption path before scheduling.
92+
func parseDiscoveryResult(integrationName, resultJSON string) ([]integration.Config, error) {
93+
var raws []discoveredConfig
94+
if err := json.Unmarshal([]byte(resultJSON), &raws); err != nil {
95+
return nil, fmt.Errorf("decode discovery payload for %s: %w", integrationName, err)
96+
}
97+
if len(raws) == 0 {
98+
return nil, nil
99+
}
100+
configs := make([]integration.Config, 0, len(raws))
101+
for _, raw := range raws {
102+
initConfig := raw.InitConfig
103+
if len(initConfig) == 0 {
104+
initConfig = json.RawMessage("{}")
105+
}
106+
cfg := integration.Config{
107+
Name: integrationName,
108+
InitConfig: integration.Data(initConfig),
109+
MetricConfig: integration.Data(raw.MetricConfig),
110+
LogsConfig: integration.Data(raw.LogsConfig),
111+
IgnoreAutodiscoveryTags: raw.IgnoreAutodiscoveryTags,
112+
CheckTagCardinality: raw.CheckTagCardinality,
113+
}
114+
for _, inst := range raw.Instances {
115+
cfg.Instances = append(cfg.Instances, integration.Data(inst))
116+
}
117+
configs = append(configs, cfg)
118+
}
119+
return configs, nil
120+
}
Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
// Unless explicitly stated otherwise all files in this repository are licensed
2+
// under the Apache License Version 2.0.
3+
// This product includes software developed at Datadog (https://www.datadoghq.com/).
4+
// Copyright 2016-present Datadog, Inc.
5+
6+
package discoverer
7+
8+
import (
9+
"encoding/json"
10+
"reflect"
11+
"testing"
12+
13+
"github.com/stretchr/testify/assert"
14+
"github.com/stretchr/testify/require"
15+
)
16+
17+
func TestParseDiscoveryResult(t *testing.T) {
18+
tests := []struct {
19+
name string
20+
integration string
21+
payload string
22+
wantErr bool
23+
wantNil bool
24+
wantLen int
25+
wantNames []string
26+
wantInitConfigs []string // per-config; "" means default "{}"
27+
wantInstLens []int
28+
}{
29+
{
30+
name: "two configs use integration name",
31+
integration: "redis",
32+
payload: `[
33+
{"instances": [{"host": "10.0.0.1"}], "init_config": {"foo": 1}},
34+
{"instances": [{"host": "10.0.0.2"}]}
35+
]`,
36+
wantLen: 2,
37+
wantNames: []string{"redis", "redis"},
38+
wantInitConfigs: []string{`{"foo":1}`, `{}`},
39+
wantInstLens: []int{1, 1},
40+
},
41+
{
42+
name: "integration name used when no name field",
43+
integration: "krakend",
44+
payload: `[{"instances":[{"host":"x"}]}]`,
45+
wantLen: 1,
46+
wantNames: []string{"krakend"},
47+
wantInitConfigs: []string{`{}`},
48+
wantInstLens: []int{1},
49+
},
50+
{
51+
name: "empty array returns nil",
52+
integration: "redis",
53+
payload: `[]`,
54+
wantNil: true,
55+
},
56+
{
57+
name: "invalid JSON returns error",
58+
integration: "redis",
59+
payload: `not-json`,
60+
wantErr: true,
61+
},
62+
}
63+
64+
for _, tc := range tests {
65+
t.Run(tc.name, func(t *testing.T) {
66+
configs, err := parseDiscoveryResult(tc.integration, tc.payload)
67+
if tc.wantErr {
68+
require.Error(t, err)
69+
return
70+
}
71+
require.NoError(t, err)
72+
if tc.wantNil {
73+
assert.Nil(t, configs)
74+
return
75+
}
76+
require.Len(t, configs, tc.wantLen)
77+
for i, cfg := range configs {
78+
assert.Equal(t, tc.wantNames[i], cfg.Name)
79+
assert.JSONEq(t, tc.wantInitConfigs[i], string(cfg.InitConfig))
80+
assert.Len(t, cfg.Instances, tc.wantInstLens[i])
81+
}
82+
})
83+
}
84+
}
85+
86+
// TestMarshalService_PrefersBridgeOverOtherNetworks: with multiple networks
87+
// the bridge IP wins (matches %%host%%'s getFallbackHost).
88+
func TestMarshalService_PrefersBridgeOverOtherNetworks(t *testing.T) {
89+
svc := &fakeService{
90+
id: "docker://abc",
91+
hosts: map[string]string{"main": "1.2.3.4", "bridge": "10.0.0.1"},
92+
ports: nil,
93+
}
94+
jsonStr, ok, err := marshalService(svc)
95+
require.NoError(t, err)
96+
require.True(t, ok)
97+
var got discoveryService
98+
require.NoError(t, json.Unmarshal([]byte(jsonStr), &got))
99+
assert.Equal(t, "docker://abc", got.ID)
100+
assert.Equal(t, "10.0.0.1", got.Host)
101+
// Empty port list still serializes as an empty array, not null, so the
102+
// Python side gets a stable shape.
103+
assert.NotNil(t, got.Ports)
104+
assert.Empty(t, got.Ports)
105+
}
106+
107+
// TestMarshalService_SingleNetworkUsesIt: a service with exactly one network
108+
// uses that network's IP, matching the %%host%% single-network fallback.
109+
func TestMarshalService_SingleNetworkUsesIt(t *testing.T) {
110+
svc := &fakeService{
111+
id: "docker://abc",
112+
hosts: map[string]string{"main": "1.2.3.4"},
113+
ports: nil,
114+
}
115+
jsonStr, ok, err := marshalService(svc)
116+
require.NoError(t, err)
117+
require.True(t, ok)
118+
var got discoveryService
119+
require.NoError(t, json.Unmarshal([]byte(jsonStr), &got))
120+
assert.Equal(t, "1.2.3.4", got.Host)
121+
}
122+
123+
// TestMarshalService_MultiNetworkWithoutBridge_NotOK: %%host%%'s policy
124+
// refuses to guess between equally-valid IPs when no bridge is present, so
125+
// we treat it as a transient failure (returns ok=false, no error).
126+
func TestMarshalService_MultiNetworkWithoutBridge_NotOK(t *testing.T) {
127+
svc := &fakeService{
128+
id: "docker://abc",
129+
hosts: map[string]string{"netA": "1.2.3.4", "netB": "5.6.7.8"},
130+
}
131+
_, ok, err := marshalService(svc)
132+
require.NoError(t, err)
133+
assert.False(t, ok)
134+
}
135+
136+
func TestMarshalService_NoHostReturnsNotOK(t *testing.T) {
137+
svc := &fakeService{id: "docker://abc", hosts: map[string]string{}}
138+
_, ok, err := marshalService(svc)
139+
require.NoError(t, err)
140+
assert.False(t, ok)
141+
}
142+
143+
func TestMarshalService_PortsRoundTrip(t *testing.T) {
144+
svc := &fakeService{
145+
id: "docker://abc",
146+
hosts: map[string]string{"main": "1.2.3.4"},
147+
ports: []servicePort{{Name: "http", Port: 8080}, {Name: "metrics", Port: 9090}},
148+
}
149+
jsonStr, ok, err := marshalService(svc)
150+
require.NoError(t, err)
151+
require.True(t, ok)
152+
var got discoveryService
153+
require.NoError(t, json.Unmarshal([]byte(jsonStr), &got))
154+
expected := []discoveryPort{{Number: 8080, Name: "http"}, {Number: 9090, Name: "metrics"}}
155+
assert.True(t, reflect.DeepEqual(expected, got.Ports))
156+
}
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
// Unless explicitly stated otherwise all files in this repository are licensed
2+
// under the Apache License Version 2.0.
3+
// This product includes software developed at Datadog (https://www.datadoghq.com/).
4+
// Copyright 2016-present Datadog, Inc.
5+
6+
package discoverer
7+
8+
import (
9+
"sync"
10+
11+
workloadmeta "github.com/DataDog/datadog-agent/comp/core/workloadmeta/def"
12+
)
13+
14+
// servicePort is a tiny port struct used by fakeService so tests don't depend
15+
// on the workloadmeta package internals beyond the public ContainerPort.
16+
type servicePort struct {
17+
Name string
18+
Port int
19+
}
20+
21+
// fakeService is a minimal ServiceInfo implementation for the
22+
// discoverer package's unit tests.
23+
type fakeService struct {
24+
id string
25+
hosts map[string]string
26+
hostsErr error
27+
ports []servicePort
28+
portsErr error
29+
}
30+
31+
var _ ServiceInfo = (*fakeService)(nil)
32+
33+
func (f *fakeService) GetServiceID() string { return f.id }
34+
func (f *fakeService) GetHosts() (map[string]string, error) {
35+
return f.hosts, f.hostsErr
36+
}
37+
func (f *fakeService) GetPorts() ([]workloadmeta.ContainerPort, error) {
38+
if f.portsErr != nil {
39+
return nil, f.portsErr
40+
}
41+
out := make([]workloadmeta.ContainerPort, 0, len(f.ports))
42+
for _, p := range f.ports {
43+
out = append(out, workloadmeta.ContainerPort{Name: p.Name, Port: p.Port})
44+
}
45+
return out, nil
46+
}
47+
48+
// fixedLookup is a ServiceLookup mock implementation.
49+
type fixedLookup struct {
50+
mu sync.Mutex
51+
services map[string]ServiceInfo
52+
}
53+
54+
func (l *fixedLookup) LookupService(svcID string) (ServiceInfo, bool) {
55+
l.mu.Lock()
56+
defer l.mu.Unlock()
57+
if l.services == nil {
58+
return nil, false
59+
}
60+
svc, ok := l.services[svcID]
61+
return svc, ok
62+
}
63+
64+
// remove drops the given svcID from the lookup, simulating a service
65+
// deletion observed by the worker on its next pop.
66+
func (l *fixedLookup) remove(svcID string) {
67+
l.mu.Lock()
68+
defer l.mu.Unlock()
69+
delete(l.services, svcID)
70+
}

0 commit comments

Comments
 (0)