Skip to content

Commit 07c716c

Browse files
committed
feat(system-metrics): Change gc.get_count with gc.get_stats with corresponding metrics.
Signed-off-by: Cagri Yonca <cagri@ibm.com>
1 parent df574aa commit 07c716c

5 files changed

Lines changed: 361 additions & 98 deletions

File tree

src/instana/agent/host.py

Lines changed: 53 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -99,13 +99,17 @@ def reset(self) -> None:
9999

100100
def is_timed_out(self) -> bool:
101101
"""
102-
If we haven't heard from the Instana host agent in 60 seconds, this
103-
method will return True.
102+
If we haven't heard from the Instana host agent within the timeout
103+
window, this method will return True. The window is the larger of
104+
60 seconds or twice the configured poll_rate so that high poll_rate
105+
values (e.g. 120 s) do not cause spurious resets.
104106
@return: Boolean
105107
"""
106-
if self.last_seen and self.can_send:
108+
if self.last_seen and self.can_send():
109+
poll_rate = getattr(getattr(self, "options", None), "poll_rate", 1)
110+
timeout_threshold = max(60, poll_rate * 2)
107111
diff = datetime.now() - self.last_seen
108-
if diff.seconds > 60:
112+
if diff.seconds > timeout_threshold:
109113
return True
110114
return False
111115

@@ -276,31 +280,40 @@ def report_data_payload(
276280
) -> Optional[Response]:
277281
"""
278282
Used to report collection payload to the host agent. This can be metrics, spans and snapshot data.
283+
When there is nothing to send (no spans, profiles, or metrics), a lightweight HEAD heartbeat
284+
is sent instead so that the host-agent timeout detection continues to work correctly even
285+
when poll_rate is larger than the 60-second timeout window.
279286
"""
280287
response = None
288+
data_was_sent = False
281289
try:
282290
# Report spans (if any)
283291
response = self.report_spans(payload)
284-
285292
if response is not None and 200 <= response.status_code <= 204:
286293
self.last_seen = datetime.now()
294+
data_was_sent = True
287295

288296
# Report profiles (if any)
289297
response = self.report_profiles(payload)
290-
291298
if response is not None and 200 <= response.status_code <= 204:
292299
self.last_seen = datetime.now()
300+
data_was_sent = True
293301

294302
# Report metrics
295303
response = self.report_metrics(payload)
296-
297304
if response is not None and 200 <= response.status_code <= 204:
298305
self.last_seen = datetime.now()
306+
data_was_sent = True
299307

300308
if response.status_code == 200 and len(response.content) > 2:
301-
# The host agent returned something indicating that is has a request for us that we
302-
# need to process.
309+
# The host agent returned something indicating that it has a request for us
310+
# that we need to process.
303311
self.handle_agent_tasks(json.loads(response.content)[0])
312+
313+
# Nothing was sent this cycle — send a heartbeat HEAD request so that
314+
# is_timed_out() keeps working correctly at high poll_rate values.
315+
if not data_was_sent:
316+
self._send_heartbeat()
304317
except requests.exceptions.ConnectionError:
305318
pass
306319
except urllib3.exceptions.MaxRetryError:
@@ -312,6 +325,37 @@ def report_data_payload(
312325
)
313326
return response
314327

328+
def _send_heartbeat(self) -> None:
329+
"""
330+
Sends a lightweight HEAD request to the host agent data endpoint to confirm
331+
connectivity and update last_seen when no metrics, spans or profiles were sent.
332+
333+
Guards:
334+
- Only runs in "good2go" state — during wait4init the FSM polling already
335+
performs HEAD checks via is_agent_ready(), so a second HEAD is unnecessary.
336+
- announce_data must be set (skipped silently during pre-announce).
337+
- A local copy of announce_data is taken before the request to avoid a race
338+
condition where reset() sets announce_data=None mid-flight.
339+
"""
340+
try:
341+
announce_data = self.announce_data # local copy — avoids race with reset()
342+
if announce_data is None:
343+
return
344+
if self.machine.fsm.current != "good2go":
345+
return
346+
response = self.client.head(self.__data_url(), timeout=0.8)
347+
if response is not None and 200 <= response.status_code <= 204:
348+
self.last_seen = datetime.now()
349+
except requests.exceptions.ConnectionError:
350+
pass
351+
except urllib3.exceptions.MaxRetryError:
352+
pass
353+
except Exception as exc:
354+
logger.debug(
355+
f"_send_heartbeat: connection error ({type(exc)})",
356+
exc_info=True,
357+
)
358+
315359
def report_metrics(self, payload: dict[str, Any]) -> Optional[Response]:
316360
metrics = payload.get("metrics", [])
317361
if len(metrics) > 0 and len(metrics.get("plugins", [])) > 0:

src/instana/collector/helpers/runtime.py

Lines changed: 27 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,11 @@ def __init__(
4444
self.previous = DictionaryOfStan()
4545
self.previous_rusage = get_resource_usage()
4646

47-
if gc.isenabled():
48-
self.previous_gc_count = gc.get_count()
49-
else:
50-
self.previous_gc_count = None
47+
# Initialise to None so the first collect_metrics call establishes the
48+
# baseline snapshot and reports all-zero deltas. Any GC activity that
49+
# occurs between process start and the first collection is intentionally
50+
# excluded — we only report incremental deltas from the first poll onward.
51+
self.previous_gc_stats = None
5152

5253
def collect_metrics(self, **kwargs: Dict[str, Any]) -> List[Dict[str, Any]]:
5354
plugin_data = dict()
@@ -82,6 +83,7 @@ def _collect_runtime_metrics(
8283
return
8384

8485
""" Collect up and return the runtime metrics """
86+
rusage = self.previous_rusage
8587
try:
8688
rusage = get_resource_usage()
8789
if gc.isenabled():
@@ -232,52 +234,27 @@ def _collect_runtime_metrics(
232234

233235
def _collect_gc_metrics(self, plugin_data, with_snapshot):
234236
try:
235-
gc_count = gc.get_count()
236-
gc_threshold = gc.get_threshold()
237-
238-
self.apply_delta(
239-
gc_count[0],
240-
self.previous["data"]["metrics"]["gc"],
241-
plugin_data["data"]["metrics"]["gc"],
242-
"collect0",
243-
with_snapshot,
244-
)
245-
self.apply_delta(
246-
gc_count[1],
247-
self.previous["data"]["metrics"]["gc"],
248-
plugin_data["data"]["metrics"]["gc"],
249-
"collect1",
250-
with_snapshot,
251-
)
252-
self.apply_delta(
253-
gc_count[2],
254-
self.previous["data"]["metrics"]["gc"],
255-
plugin_data["data"]["metrics"]["gc"],
256-
"collect2",
257-
with_snapshot,
258-
)
259-
260-
self.apply_delta(
261-
gc_threshold[0],
262-
self.previous["data"]["metrics"]["gc"],
263-
plugin_data["data"]["metrics"]["gc"],
264-
"threshold0",
265-
with_snapshot,
266-
)
267-
self.apply_delta(
268-
gc_threshold[1],
269-
self.previous["data"]["metrics"]["gc"],
270-
plugin_data["data"]["metrics"]["gc"],
271-
"threshold1",
272-
with_snapshot,
273-
)
274-
self.apply_delta(
275-
gc_threshold[2],
276-
self.previous["data"]["metrics"]["gc"],
277-
plugin_data["data"]["metrics"]["gc"],
278-
"threshold2",
279-
with_snapshot,
280-
)
237+
gc_stats = gc.get_stats()
238+
if self.previous_gc_stats is None:
239+
# First call: establish baseline, report all-zero deltas so the
240+
# snapshot payload carries zeros rather than the cumulative counts
241+
# that accumulated since process start (which are not meaningful
242+
# as deltas).
243+
self.previous_gc_stats = gc_stats
244+
return
245+
246+
# Use a plain dict as the staging target so that accessing it never
247+
# auto-creates keys in plugin_data (DictionaryOfStan creates keys on
248+
# read, which would leave an empty "gc": {} even when nothing changed).
249+
staging: dict = {}
250+
prev_gc = self.previous["data"]["metrics"]["gc"]
251+
for i, (stat, prev_stat) in enumerate(zip(gc_stats, self.previous_gc_stats)):
252+
for key in ("collections", "collected", "uncollectable"):
253+
delta = stat[key] - prev_stat.get(key, 0)
254+
self.apply_delta(delta, prev_gc, staging, f"{key}{i}", with_snapshot)
255+
if staging:
256+
plugin_data["data"]["metrics"]["gc"].update(staging)
257+
self.previous_gc_stats = gc_stats
281258
except Exception:
282259
logger.debug("_collect_gc_metrics", exc_info=True)
283260

tests/agent/test_host.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -334,8 +334,8 @@ def test_is_timed_out(
334334
assert not agent.is_timed_out()
335335

336336
agent.last_seen = datetime.datetime.now() - datetime.timedelta(minutes=5)
337-
agent.can_send = True
338-
assert agent.is_timed_out()
337+
with patch.object(agent, "can_send", return_value=True):
338+
assert agent.is_timed_out()
339339

340340
def test_can_send_test_env(
341341
self,

tests/collector/helpers/test_collector_runtime.py

Lines changed: 67 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ def test_default_while_gc_disabled(self) -> None:
2727

2828
gc.disable()
2929
helper = RuntimeHelper(collector=HostCollector(HostAgent()))
30-
assert helper.previous_gc_count is None
30+
assert helper.previous_gc_stats is None
3131

3232
def test_collect_metrics(self) -> None:
3333
response = self.helper.collect_metrics()
@@ -65,8 +65,73 @@ def test_collect_runtime_snapshot_webhook(self) -> None:
6565
def test_collect_gc_metrics(self) -> None:
6666
plugin_data = self.helper.collect_metrics()
6767

68+
# First call establishes the baseline (previous_gc_stats was None); no
69+
# data is written yet.
6870
self.helper._collect_gc_metrics(plugin_data[0], True)
69-
assert len(self.helper.previous["data"]["metrics"]["gc"]) == 6
71+
assert self.helper.previous_gc_stats is not None
72+
73+
# Second call computes deltas from the baseline and writes them.
74+
self.helper._collect_gc_metrics(plugin_data[0], True)
75+
gc_metrics = self.helper.previous["data"]["metrics"]["gc"]
76+
for i in range(3):
77+
for key in ("collections", "collected", "uncollectable"):
78+
assert f"{key}{i}" in gc_metrics
79+
80+
def test_collect_gc_metrics_reports_delta_between_polls(self) -> None:
81+
"""GC metrics must be deltas between successive polls, not kumulatif values."""
82+
# Simulate first poll: previous_gc_stats set to known baseline
83+
baseline = [
84+
{"collections": 100, "collected": 200, "uncollectable": 0},
85+
{"collections": 10, "collected": 50, "uncollectable": 0},
86+
{"collections": 1, "collected": 5, "uncollectable": 0},
87+
]
88+
self.helper.previous_gc_stats = baseline
89+
90+
# Simulate gc.get_stats() returning incremented counts
91+
after = [
92+
{"collections": 103, "collected": 206, "uncollectable": 0},
93+
{"collections": 11, "collected": 53, "uncollectable": 0},
94+
{"collections": 1, "collected": 5, "uncollectable": 0},
95+
]
96+
97+
plugin_data = [{"data": {"metrics": {"gc": {}}}}]
98+
99+
import unittest.mock as mock
100+
with mock.patch("gc.get_stats", return_value=after):
101+
self.helper._collect_gc_metrics(plugin_data[0], True)
102+
103+
gc_metrics = plugin_data[0]["data"]["metrics"]["gc"]
104+
# Gen 0: collections delta = 3, collected delta = 6
105+
assert gc_metrics["collections0"] == 3
106+
assert gc_metrics["collected0"] == 6
107+
assert gc_metrics["uncollectable0"] == 0
108+
# Gen 1: collections delta = 1, collected delta = 3
109+
assert gc_metrics["collections1"] == 1
110+
assert gc_metrics["collected1"] == 3
111+
# Gen 2: no change — delta = 0, still reported because with_snapshot=True
112+
assert gc_metrics["collections2"] == 0
113+
assert gc_metrics["collected2"] == 0
114+
115+
# previous_gc_stats must be updated to the latest snapshot
116+
assert self.helper.previous_gc_stats == after
117+
118+
def test_collect_gc_metrics_no_change_not_sent_without_snapshot(self) -> None:
119+
"""When nothing changed and with_snapshot=False, gc metrics must be empty."""
120+
same = [
121+
{"collections": 50, "collected": 100, "uncollectable": 0},
122+
{"collections": 5, "collected": 20, "uncollectable": 0},
123+
{"collections": 0, "collected": 0, "uncollectable": 0},
124+
]
125+
self.helper.previous_gc_stats = same
126+
127+
plugin_data = [{"data": {"metrics": {"gc": {}}}}]
128+
129+
import unittest.mock as mock
130+
with mock.patch("gc.get_stats", return_value=same):
131+
self.helper._collect_gc_metrics(plugin_data[0], False)
132+
133+
# All deltas are 0 and with_snapshot=False → nothing written
134+
assert plugin_data[0]["data"]["metrics"]["gc"] == {}
70135

71136
def test_collect_runtime_metrics(self) -> None:
72137
"""Test that _collect_runtime_metrics properly collects metrics"""

0 commit comments

Comments
 (0)