Skip to content

Commit 5d5cef7

Browse files
avara1986avara1986
authored
chore(rcm/di): disable multiple target files (#6035)
Remote Config detects if there is new information to poll from subscribers by validating a checksum of the Remote Config payloads. When we remove a Dynamic Instrumentation Probe from the UI, the payload is set to 'False'. The problem occurs when we remove two probes. In both cases, the "config" is set to False, they have the same checksum, and the second probe is not removed correctly. No change log is needed because this error was introduced in version 1.15.0. ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines) are followed. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment.
1 parent f67b694 commit 5d5cef7

File tree

3 files changed

+143
-9
lines changed

3 files changed

+143
-9
lines changed

‎ddtrace/internal/remoteconfig/_connectors.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,9 @@ def __init__(self):
4747
self.shared_data_counter = 0
4848

4949
@staticmethod
50-
def _hash_config(config_raw):
51-
# type: (SharedDataType) -> int
52-
return hash(str(config_raw))
50+
def _hash_config(config_raw, metadata_raw):
51+
# type: (Any, Any) -> int
52+
return hash(str(config_raw) + str(metadata_raw))
5353

5454
def read(self):
5555
# type: () -> SharedDataType
@@ -64,7 +64,7 @@ def read(self):
6464

6565
def write(self, metadata, config_raw):
6666
# type: (Any, Any) -> None
67-
last_checksum = self._hash_config(config_raw)
67+
last_checksum = self._hash_config(config_raw, metadata)
6868
if last_checksum != self.checksum:
6969
data_len = len(self.data.value)
7070
if data_len >= (SHARED_MEMORY_SIZE - 1000):

‎tests/debugging/probe/test_remoteconfig.py‎

Lines changed: 138 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,14 +106,148 @@ def callback(e, ps, *args, **kwargs):
106106
]
107107
)
108108

109-
adapter = ProbeRCAdapter("", callback)
109+
adapter = ProbeRCAdapter(None, callback)
110110
remoteconfig_poller.register("TEST", adapter)
111111
adapter.append_and_publish({"test": random.randint(0, 11111111)}, "", config_metadata())
112112
remoteconfig_poller._poll_data()
113113

114114
assert set(_.probe_id for _ in probes) == expected
115115

116116

117+
def test_poller_remove_probe():
118+
events = set()
119+
120+
def cb(e, ps):
121+
events.add((e, frozenset({p.probe_id if isinstance(p, Probe) else p for p in ps})))
122+
123+
def validate_events(expected):
124+
assert events == expected
125+
events.clear()
126+
127+
old_interval = config.diagnostics_interval
128+
config.diagnostics_interval = 0.5
129+
try:
130+
adapter = ProbeRCAdapter(None, cb)
131+
# Wait to allow the next call to the adapter to generate a status event
132+
remoteconfig_poller.register("TEST", adapter, skip_enabled=True)
133+
adapter.append_and_publish(
134+
{
135+
"id": "probe1",
136+
"version": 0,
137+
"type": ProbeType.SPAN_PROBE,
138+
"active": True,
139+
"tags": ["foo:bar"],
140+
"where": {"type": "Stuff", "method": "foo"},
141+
"resource": "resourceX",
142+
},
143+
"",
144+
config_metadata("probe1"),
145+
)
146+
remoteconfig_poller._poll_data()
147+
148+
validate_events(
149+
{
150+
(ProbePollerEvent.NEW_PROBES, frozenset({"probe1"})),
151+
}
152+
)
153+
154+
adapter.append_and_publish(
155+
False,
156+
"",
157+
config_metadata("probe1"),
158+
)
159+
remoteconfig_poller._poll_data()
160+
161+
validate_events(
162+
{
163+
(ProbePollerEvent.DELETED_PROBES, frozenset({"probe1"})),
164+
}
165+
)
166+
167+
finally:
168+
config.diagnostics_interval = old_interval
169+
170+
171+
def test_poller_remove_multiple_probe():
172+
events = set()
173+
174+
def cb(e, ps):
175+
events.add((e, frozenset({p.probe_id if isinstance(p, Probe) else p for p in ps})))
176+
177+
def validate_events(expected):
178+
assert events == expected
179+
events.clear()
180+
181+
old_interval = config.diagnostics_interval
182+
config.diagnostics_interval = 0.5
183+
try:
184+
adapter = ProbeRCAdapter(None, cb)
185+
# Wait to allow the next call to the adapter to generate a status event
186+
remoteconfig_poller.register("TEST", adapter, skip_enabled=True)
187+
adapter.append(
188+
{
189+
"id": "probe1",
190+
"version": 0,
191+
"type": ProbeType.SPAN_PROBE,
192+
"active": True,
193+
"tags": ["foo:bar"],
194+
"where": {"type": "Stuff", "method": "foo"},
195+
"resource": "resourceX",
196+
},
197+
"",
198+
config_metadata("probe1"),
199+
)
200+
adapter.append(
201+
{
202+
"id": "probe2",
203+
"version": 0,
204+
"type": ProbeType.SPAN_PROBE,
205+
"active": True,
206+
"tags": ["foo:bar"],
207+
"where": {"type": "Stuff", "method": "foo"},
208+
"resource": "resourceX",
209+
},
210+
"",
211+
config_metadata("probe2"),
212+
)
213+
adapter.publish()
214+
remoteconfig_poller._poll_data()
215+
216+
validate_events(
217+
{
218+
(ProbePollerEvent.NEW_PROBES, frozenset({"probe2"})),
219+
(ProbePollerEvent.NEW_PROBES, frozenset({"probe1"})),
220+
}
221+
)
222+
223+
adapter.append_and_publish(
224+
False,
225+
"",
226+
config_metadata("probe1"),
227+
)
228+
remoteconfig_poller._poll_data()
229+
230+
validate_events(
231+
{
232+
(ProbePollerEvent.DELETED_PROBES, frozenset({"probe1"})),
233+
}
234+
)
235+
adapter.append_and_publish(
236+
False,
237+
"",
238+
config_metadata("probe2"),
239+
)
240+
remoteconfig_poller._poll_data()
241+
242+
validate_events(
243+
{
244+
(ProbePollerEvent.DELETED_PROBES, frozenset({"probe2"})),
245+
}
246+
)
247+
finally:
248+
config.diagnostics_interval = old_interval
249+
250+
117251
def test_poller_events(remote_config_worker, mock_config):
118252
events = set()
119253

@@ -153,7 +287,7 @@ def callback(e, ps, *args, **kwargs):
153287
old_interval = config.diagnostics_interval
154288
config.diagnostics_interval = 0.5
155289
try:
156-
adapter = ProbeRCAdapter("", callback)
290+
adapter = ProbeRCAdapter(None, callback)
157291
remoteconfig_poller.register("TEST2", adapter, skip_enabled=True)
158292
adapter.append_and_publish({"test": 2}, "", metadata)
159293
remoteconfig_poller._poll_data()
@@ -207,7 +341,7 @@ def validate_events(expected):
207341
old_interval = config.diagnostics_interval
208342
config.diagnostics_interval = 0.5
209343
try:
210-
adapter = ProbeRCAdapter("", cb)
344+
adapter = ProbeRCAdapter(None, cb)
211345
# Wait to allow the next call to the adapter to generate a status event
212346
remoteconfig_poller.register("TEST", adapter, skip_enabled=True)
213347
adapter.append_and_publish(
@@ -403,7 +537,7 @@ def cb(e, ps):
403537
old_interval = config.diagnostics_interval
404538
config.diagnostics_interval = 0.5
405539
try:
406-
adapter = ProbeRCAdapter("", cb)
540+
adapter = ProbeRCAdapter(None, cb)
407541
# Wait to allow the next call to the adapter to generate a status event
408542
remoteconfig_poller.register("TEST", adapter)
409543
sleep(0.5)

‎tests/internal/remoteconfig/test_remoteconfig_connector.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
)
1717
def test_hash(data):
1818
connector = PublisherSubscriberConnector()
19-
assert type(connector._hash_config(data)) is int
19+
assert type(connector._hash_config(data, None)) is int
2020

2121

2222
def test_connector():

0 commit comments

Comments
 (0)