Skip to content

Commit c08cefb

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 9084ca8 commit c08cefb

5 files changed

Lines changed: 339 additions & 49 deletions

File tree

src/instana/agent/host.py

Lines changed: 57 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -99,13 +99,24 @@ 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.
106+
107+
Note: We intentionally read the FSM state directly instead of calling
108+
can_send(), because can_send() has a fork-detection side-effect that
109+
triggers handle_fork() → reset() and would cause a spurious restart
110+
loop on frameworks (e.g. Twisted) that spawn threads with different
111+
perceived PIDs. total_seconds() is used instead of .seconds to
112+
correctly handle gaps longer than 24 hours.
104113
@return: Boolean
105114
"""
106-
if self.last_seen and self.can_send:
115+
if self.last_seen and self.machine.fsm.current in ["wait4init", "good2go"]:
116+
poll_rate = getattr(getattr(self, "options", None), "poll_rate", 1)
117+
timeout_threshold = max(60, poll_rate * 2)
107118
diff = datetime.now() - self.last_seen
108-
if diff.seconds > 60:
119+
if diff.total_seconds() > timeout_threshold:
109120
return True
110121
return False
111122

@@ -276,31 +287,40 @@ def report_data_payload(
276287
) -> Optional[Response]:
277288
"""
278289
Used to report collection payload to the host agent. This can be metrics, spans and snapshot data.
290+
When there is nothing to send (no spans, profiles, or metrics), a lightweight HEAD heartbeat
291+
is sent instead so that the host-agent timeout detection continues to work correctly even
292+
when poll_rate is larger than the 60-second timeout window.
279293
"""
280294
response = None
295+
data_was_sent = False
281296
try:
282297
# Report spans (if any)
283298
response = self.report_spans(payload)
284-
285299
if response is not None and 200 <= response.status_code <= 204:
286300
self.last_seen = datetime.now()
301+
data_was_sent = True
287302

288303
# Report profiles (if any)
289304
response = self.report_profiles(payload)
290-
291305
if response is not None and 200 <= response.status_code <= 204:
292306
self.last_seen = datetime.now()
307+
data_was_sent = True
293308

294309
# Report metrics
295310
response = self.report_metrics(payload)
296-
297311
if response is not None and 200 <= response.status_code <= 204:
298312
self.last_seen = datetime.now()
313+
data_was_sent = True
299314

300315
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.
316+
# The host agent returned something indicating that it has a request for us
317+
# that we need to process.
303318
self.handle_agent_tasks(json.loads(response.content)[0])
319+
320+
# Nothing was sent this cycle — send a heartbeat HEAD request so that
321+
# is_timed_out() keeps working correctly at high poll_rate values.
322+
if not data_was_sent:
323+
self._send_heartbeat()
304324
except requests.exceptions.ConnectionError:
305325
pass
306326
except urllib3.exceptions.MaxRetryError:
@@ -312,6 +332,34 @@ def report_data_payload(
312332
)
313333
return response
314334

335+
def _send_heartbeat(self) -> None:
336+
"""
337+
Updates last_seen via a HEAD request when no payload was sent this cycle.
338+
339+
Critical when metric collection is disabled (INSTANA_DISABLE_METRICS_COLLECTION)
340+
and no spans arrive — last_seen would never be updated, causing is_timed_out()
341+
to fire spuriously. Only runs in "good2go" state; wait4init already polls via
342+
is_agent_ready().
343+
"""
344+
try:
345+
announce_data = self.announce_data # local copy — avoids race with reset()
346+
if announce_data is None:
347+
return
348+
if self.machine.fsm.current != "good2go":
349+
return
350+
response = self.client.head(self.__data_url(), timeout=0.8)
351+
if response is not None and 200 <= response.status_code <= 204:
352+
self.last_seen = datetime.now()
353+
except requests.exceptions.ConnectionError:
354+
pass
355+
except urllib3.exceptions.MaxRetryError:
356+
pass
357+
except Exception as exc:
358+
logger.debug(
359+
f"_send_heartbeat: connection error ({type(exc)})",
360+
exc_info=True,
361+
)
362+
315363
def report_metrics(self, payload: dict[str, Any]) -> Optional[Response]:
316364
metrics = payload.get("metrics", [])
317365
if len(metrics) > 0 and len(metrics.get("plugins", [])) > 0:

src/instana/collector/helpers/runtime.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ def _collect_runtime_metrics(
8383
return
8484

8585
""" Collect up and return the runtime metrics """
86+
rusage = self.previous_rusage
8687
try:
8788
rusage = get_resource_usage()
8889
if gc.isenabled():

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.machine.fsm, "current", "good2go"):
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: 66 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# (c) Copyright IBM Corp. 2024
22

3-
from typing import Generator
3+
from collections.abc import Generator
44
from unittest.mock import patch
55

66
import pytest
@@ -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()
@@ -68,8 +68,71 @@ def test_collect_runtime_snapshot_webhook(self) -> None:
6868
def test_collect_gc_metrics(self) -> None:
6969
plugin_data = self.helper.collect_metrics()
7070

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

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

0 commit comments

Comments
 (0)