-
Notifications
You must be signed in to change notification settings - Fork 321
ROB-887 Cache node IP lookups for prometheus alerts #2154
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,11 @@ | ||
| import logging | ||
| import time | ||
| from typing import Any, Dict, List, NamedTuple, Optional, Type, Union | ||
|
|
||
| from hikaru.model.rel_1_26 import DaemonSet, HorizontalPodAutoscaler, Job, Node, NodeList, StatefulSet | ||
| from pydantic.main import BaseModel | ||
|
|
||
| from robusta.core.model.env_vars import NODE_IP_CACHE_TTL_SEC | ||
| from robusta.core.model.events import ExecutionBaseEvent | ||
| from robusta.core.playbooks.base_trigger import BaseTrigger, TriggerEvent | ||
| from robusta.core.reporting.base import Finding | ||
|
|
@@ -130,15 +132,24 @@ class PrometheusAlertTriggers(BaseModel): | |
|
|
||
|
|
||
| class AlertEventBuilder: | ||
| _node_name_by_ip: Dict[str, str] = {} | ||
| _node_ip_cache_time: float = 0 | ||
|
|
||
| @classmethod | ||
| def __find_node_by_ip(cls, ip) -> Optional[Node]: | ||
| def __refresh_node_ip_cache(cls): | ||
| nodes: NodeList = NodeList.listNode().obj | ||
| for node in nodes.items: | ||
| addresses = [a.address for a in node.status.addresses] | ||
| logging.info(f"node {node.metadata.name} has addresses {addresses}") | ||
| if ip in addresses: | ||
| return node | ||
| return None | ||
| cls._node_name_by_ip = { | ||
| address.address: node.metadata.name for node in nodes.items for address in node.status.addresses | ||
| } | ||
| cls._node_ip_cache_time = time.time() | ||
|
|
||
| @classmethod | ||
| def __find_node_by_ip(cls, ip) -> Optional[Node]: | ||
| cache_expired = time.time() - cls._node_ip_cache_time > NODE_IP_CACHE_TTL_SEC | ||
|
Comment on lines
+146
to
+150
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Expect: both TTL operations use time.monotonic().
rg -n -C 2 \
'time\.(time|monotonic)|_node_ip_cache_time|cache_expired' \
src/robusta/integrations/prometheus/trigger.pyRepository: robusta-dev/robusta Length of output: 814 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- trigger.py structure ---'
ast-grep outline src/robusta/integrations/prometheus/trigger.py
printf '%s\n' '--- relevant implementation ---'
sed -n '1,190p' src/robusta/integrations/prometheus/trigger.py
printf '%s\n' '--- cache and builder usages ---'
rg -n -C 3 \
'_node_name_by_ip|_node_ip_cache_time|__find_node_by_ip|__refresh_node_ip_cache|AlertEventBuilder' \
src tests 2>/dev/null || trueRepository: robusta-dev/robusta Length of output: 13487 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '1,190p' src/robusta/integrations/prometheus/trigger.py
rg -n -C 3 '_node_name_by_ip|_node_ip_cache_time|__find_node_by_ip|__refresh_node_ip_cache|AlertEventBuilder' src tests 2>/dev/null || trueRepository: robusta-dev/robusta Length of output: 12452 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
TTL = 900.0
cache_wall = 1_000_000.0
cache_mono = 10_000.0
cases = [
("15 min elapsed, wall clock unchanged", 1_000_900.0, 10_900.0),
("15 min elapsed, wall clock moved backward 1 h", 996_300.0, 10_900.0),
("5 min elapsed, wall clock moved forward 1 h", 1_003_300.0, 10_300.0),
]
for name, now_wall, now_mono in cases:
wall_expired = now_wall - cache_wall > TTL
monotonic_expired = now_mono - cache_mono > TTL
print(f"{name}: wall_clock={wall_expired}, monotonic_clock={monotonic_expired}")
PYRepository: robusta-dev/robusta Length of output: 406 Use a monotonic clock for cache expiration. Replace both 🤖 Prompt for AI Agents |
||
| if cache_expired or ip not in cls._node_name_by_ip: | ||
| cls.__refresh_node_ip_cache() | ||
|
Comment on lines
+149
to
+152
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Expect: identify whether alert handling can invoke cache refresh concurrently.
rg -n -C 10 \
'alerts_queue|add_task|Thread|worker|concurrent|__find_node_by_ip|__refresh_node_ip_cache' \
src/robusta/runner src/robusta/integrations/prometheusRepository: robusta-dev/robusta Length of output: 13898 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- queue implementation and worker configuration ---'
rg -n -C 14 \
'class TaskQueue|def __init__|num_workers|ThreadPoolExecutor|Thread\(|NUM_EVENT_THREADS' \
src/robusta/utils src/robusta/runner src/robusta/core
printf '%s\n' '--- relevant web initialization and alert dispatch ---'
sed -n '1,120p' src/robusta/runner/web.pyRepository: robusta-dev/robusta Length of output: 50376 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- NUM_EVENT_THREADS definition and usage ---'
rg -n -C 5 \
'NUM_EVENT_THREADS\s*=|NUM_EVENT_THREADS' \
src/robusta
printf '%s\n' '--- exact queue worker code ---'
sed -n '37,75p' src/robusta/utils/task_queue.py
printf '%s\n' '--- exact alert dispatch code ---'
sed -n '88,110p' src/robusta/runner/web.pyRepository: robusta-dev/robusta Length of output: 6487 Synchronize node-cache refreshes across alert workers.
🤖 Prompt for AI Agents |
||
| node_name = cls._node_name_by_ip.get(ip) | ||
| return Node().read(name=node_name) if node_name else None | ||
|
|
||
| @classmethod | ||
| def __load_node(cls, alert: PrometheusAlert, node_name: str) -> Optional[Node]: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: robusta-dev/robusta
Length of output: 4124
🏁 Script executed:
Repository: robusta-dev/robusta
Length of output: 3726
Validate
NODE_IP_CACHE_TTL_SEC. Negative values always expire the cache. A value of0also refreshes the cache on each__find_node_by_ipcall, causing repeatedNodeList.listNode()calls. Reject negative values and define the intended behavior for0.🤖 Prompt for AI Agents