Conversation
|
🚀 PR Preview for netobserv.io has been successfully deployed! |
leandroberetta
force-pushed
the
network-health
branch
from
89fc45e to
3134685
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds author Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User (Dashboard)
participant Dashboard as NetObserv Dashboard
participant Topology as Topology UI
participant Prom as Prometheus
participant AM as Alertmanager
participant Istio as Istio / Workload
User->>Dashboard: Open Network Health view / select workload
Dashboard->>Prom: Query recording metrics & rule states
Prom-->>Dashboard: Return recorded metrics (e.g., 5xx rate)
Note over Prom,Dashboard: Recording rules continuously compute metrics
Istio->>Prom: Workload emits metrics (e.g., istio_requests_total)
Dashboard->>Topology: Request health panel for selected object
Prom-->>Topology: Indicate active violations for object
alt Violation exists
Topology->>User: Show Health side panel (critical/summary)
else No violation
Topology->>User: No Health panel shown
end
rect rgba(255,200,200,0.5)
Prom->>AM: Alert rules fire (pending/firing)
AM-->>Dashboard: Alerts surfaced in UI
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@content/posts/2026-03-18-network-health/index.md`:
- Line 176: Replace the overly emphatic sentence "This is exactly where
**Network Health** comes into play." with a gentler transition such as "This is
where **Network Health** comes into play." — locate the exact text "This is
exactly where **Network Health** comes into play." in the markdown and remove
"exactly" to soften the tone.
- Line 72: The sentence "Use **recording rules** when you want continuous
visibility into how a metric evolves over time" uses redundant wording; edit
that line to replace "evolves over time" with "evolves" so it reads "Use
**recording rules** when you want continuous visibility into how a metric
evolves."
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 216af9d5-9464-4ebf-a4d4-96d1113f7586
⛔ Files ignored due to path filters (5)
content/posts/2026-03-18-network-health/alert.pngis excluded by!**/*.pngcontent/posts/2026-03-18-network-health/recording-rule.pngis excluded by!**/*.pngcontent/posts/2026-03-18-network-health/reviews-5xx-topology.pngis excluded by!**/*.pngcontent/posts/2026-03-18-network-health/reviews-5xx.pngis excluded by!**/*.pngcontent/posts/2026-03-18-network-health/topology.pngis excluded by!**/*.png
📒 Files selected for processing (2)
content/posts/2026-03-18-network-health/index.mddata/authors.yml
| In practice: | ||
|
|
||
| - Use **alert rules** when you need to be notified of immediate issues | ||
| - Use **recording rules** when you want continuous visibility into how a metric evolves over time |
There was a problem hiding this comment.
Tighten redundant wording.
At Line 72, consider replacing “evolves over time” with just “evolves” for cleaner phrasing.
🧰 Tools
🪛 LanguageTool
[style] ~72-~72: This phrase is redundant. Consider writing “evolves”.
Context: ...continuous visibility into how a metric evolves over time For Network Health, recording rules ...
(EVOLVE_OVER_TIME)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@content/posts/2026-03-18-network-health/index.md` at line 72, The sentence
"Use **recording rules** when you want continuous visibility into how a metric
evolves over time" uses redundant wording; edit that line to replace "evolves
over time" with "evolves" so it reads "Use **recording rules** when you want
continuous visibility into how a metric evolves."
|
|
||
| > *How do you make this visible at a glance for cluster administrators, without digging into Prometheus queries?* | ||
|
|
||
| This is exactly where **Network Health** comes into play. |
There was a problem hiding this comment.
Prefer a less emphatic transition.
At Line 176, replacing “exactly” (e.g., “This is where...”) reads more naturally.
🧰 Tools
🪛 LanguageTool
[style] ~176-~176: Consider an alternative for the overused word “exactly”.
Context: ...ging into Prometheus queries?* This is exactly where Network Health comes into pla...
(EXACTLY_PRECISELY)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@content/posts/2026-03-18-network-health/index.md` at line 176, Replace the
overly emphatic sentence "This is exactly where **Network Health** comes into
play." with a gentler transition such as "This is where **Network Health** comes
into play." — locate the exact text "This is exactly where **Network Health**
comes into play." in the markdown and remove "exactly" to soften the tone.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
content/posts/2026-03-27-network-health/index.md (1)
176-176: Prefer a less repetitive word than “exactly.”At Line 176, consider replacing “exactly” with “precisely” (or removing it) to reduce repetition and keep tone crisp.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@content/posts/2026-03-27-network-health/index.md` at line 176, Replace the word "exactly" in the sentence "This is exactly where **Network Health** comes into play." with a less repetitive alternative (e.g., "precisely") or remove it entirely so the line reads "This is precisely where **Network Health** comes into play." or "This is where **Network Health** comes into play."; update the sentence accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@content/posts/2026-03-27-network-health/index.md`:
- Around line 71-72: Replace the redundant phrase in the recording-rules bullet:
find the line containing "Use **recording rules** when you want continuous
visibility into how a metric evolves over time" and remove "over time" so it
reads "Use **recording rules** when you want continuous visibility into how a
metric evolves." Keep the rest of the formatting and emphasis intact.
- Around line 19-23: The bullet list in
content/posts/2026-03-27-network-health/index.md is inconsistent: the first item
uses title-style capitalization ("DNS errors and NXDOMAIN responses") while the
next four are lowercase; update the bullets "packet drops", "network policy
denials", "latency trends", and "ingress errors" to match the same
capitalization style (e.g., "Packet drops", "Network policy denials", "Latency
trends", "Ingress errors") so all list items are consistent.
---
Nitpick comments:
In `@content/posts/2026-03-27-network-health/index.md`:
- Line 176: Replace the word "exactly" in the sentence "This is exactly where
**Network Health** comes into play." with a less repetitive alternative (e.g.,
"precisely") or remove it entirely so the line reads "This is precisely where
**Network Health** comes into play." or "This is where **Network Health** comes
into play."; update the sentence accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 710eb3d8-1039-4692-b269-471a7a01f84c
⛔ Files ignored due to path filters (5)
content/posts/2026-03-27-network-health/alert.pngis excluded by!**/*.pngcontent/posts/2026-03-27-network-health/recording-rule.pngis excluded by!**/*.pngcontent/posts/2026-03-27-network-health/reviews-5xx-topology.pngis excluded by!**/*.pngcontent/posts/2026-03-27-network-health/reviews-5xx.pngis excluded by!**/*.pngcontent/posts/2026-03-27-network-health/topology.pngis excluded by!**/*.png
📒 Files selected for processing (1)
content/posts/2026-03-27-network-health/index.md
| - **DNS errors and NXDOMAIN responses** | ||
| - **packet drops** | ||
| - **network policy denials** | ||
| - **latency trends** | ||
| - **ingress errors** |
There was a problem hiding this comment.
Normalize bullet capitalization for consistency.
Line 19 starts with title-style capitalization, while Lines 20–23 are lowercase. Consider using one style across all bullets.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@content/posts/2026-03-27-network-health/index.md` around lines 19 - 23, The
bullet list in content/posts/2026-03-27-network-health/index.md is inconsistent:
the first item uses title-style capitalization ("DNS errors and NXDOMAIN
responses") while the next four are lowercase; update the bullets "packet
drops", "network policy denials", "latency trends", and "ingress errors" to
match the same capitalization style (e.g., "Packet drops", "Network policy
denials", "Latency trends", "Ingress errors") so all list items are consistent.
| - Use **alert rules** when you need to be notified of immediate issues | ||
| - Use **recording rules** when you want continuous visibility into how a metric evolves over time |
There was a problem hiding this comment.
Tighten redundant phrasing in the recording-rules bullet.
At Line 72, “evolves over time” is redundant in this context. “evolves” reads cleaner.
🧰 Tools
🪛 LanguageTool
[style] ~72-~72: This phrase is redundant. Consider writing “evolves”.
Context: ...continuous visibility into how a metric evolves over time For Network Health, recording rules ...
(EVOLVE_OVER_TIME)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@content/posts/2026-03-27-network-health/index.md` around lines 71 - 72,
Replace the redundant phrase in the recording-rules bullet: find the line
containing "Use **recording rules** when you want continuous visibility into how
a metric evolves over time" and remove "over time" so it reads "Use **recording
rules** when you want continuous visibility into how a metric evolves." Keep the
rest of the formatting and emphasis intact.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
content/posts/2026-03-26-network-health/index.md (1)
1-322: Excellent documentation—technically sound and well-structured.This is comprehensive, accurate documentation for the Network Health feature. The YAML examples are syntactically correct, the PromQL expressions are valid, and the progressive structure (concepts → configuration → hands-on demo) makes it easy to follow.
The static analysis tool flagged two optional style refinements:
- Line 72: "evolves over time" could be shortened to "evolves"
- Line 176: "exactly" could be removed
Both are purely stylistic—feel free to ignore.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@content/posts/2026-03-26-network-health/index.md` around lines 1 - 322, Summary: two optional stylistic edits requested—shorten "evolves over time" to "evolves" and remove the word "exactly". Fix: in the post content locate the sentence containing "evolves over time" (in the Recording rules paragraph/section) and replace it with "evolves"; also find the sentence containing "exactly" (in the Demo introduction where you say "This is exactly where...") and remove the word, adjusting whitespace/punctuation as needed; ensure both edits preserve surrounding wording and formatting (no code changes required).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@content/posts/2026-03-26-network-health/index.md`:
- Around line 1-322: Summary: two optional stylistic edits requested—shorten
"evolves over time" to "evolves" and remove the word "exactly". Fix: in the post
content locate the sentence containing "evolves over time" (in the Recording
rules paragraph/section) and replace it with "evolves"; also find the sentence
containing "exactly" (in the Demo introduction where you say "This is exactly
where...") and remove the word, adjusting whitespace/punctuation as needed;
ensure both edits preserve surrounding wording and formatting (no code changes
required).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dee0eb52-e462-4c9a-96d0-43359716a1ee
⛔ Files ignored due to path filters (5)
content/posts/2026-03-26-network-health/alert.pngis excluded by!**/*.pngcontent/posts/2026-03-26-network-health/recording-rule.pngis excluded by!**/*.pngcontent/posts/2026-03-26-network-health/reviews-5xx-topology.pngis excluded by!**/*.pngcontent/posts/2026-03-26-network-health/reviews-5xx.pngis excluded by!**/*.pngcontent/posts/2026-03-26-network-health/topology.pngis excluded by!**/*.png
📒 Files selected for processing (1)
content/posts/2026-03-26-network-health/index.md
leandroberetta
force-pushed
the
network-health
branch
from
00cb9c2 to
ba68e9e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
content/posts/2026-03-26-network-health/index.md (1)
72-72: Optional: Style refinements.Static analysis flagged minor style improvements:
- Line 72: "evolves over time" → "evolves" (redundant)
- Line 176: Consider alternative to "exactly" (overused)
Also applies to: 176-176
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@content/posts/2026-03-26-network-health/index.md` at line 72, Remove the redundant phrase "over time" from the sentence "Use **recording rules** when you want continuous visibility into how a metric evolves over time" so it reads "...how a metric evolves" and, in the other sentence containing the word "exactly", replace "exactly" with a less overused synonym suited to context (e.g., "precisely", "strictly", or "specifically") ensuring the sentence flow remains natural.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@content/posts/2026-03-26-network-health/index.md`:
- Around line 132-158: Replace the incorrect annotation key
"netobserv.io/network-health" with the underscore-form
"netobserv_io_network_health" in the metadata.annotations block of every
PrometheusRule example (the three PrometheusRule snippets named/typed as kind:
PrometheusRule), i.e., update the annotations key in each metadata.annotations
section so it reads netobserv_io_network_health: | followed by the existing JSON
payload; leave the netobserv: "true" labels on the PrometheusRule and on
individual rules unchanged.
---
Nitpick comments:
In `@content/posts/2026-03-26-network-health/index.md`:
- Line 72: Remove the redundant phrase "over time" from the sentence "Use
**recording rules** when you want continuous visibility into how a metric
evolves over time" so it reads "...how a metric evolves" and, in the other
sentence containing the word "exactly", replace "exactly" with a less overused
synonym suited to context (e.g., "precisely", "strictly", or "specifically")
ensuring the sentence flow remains natural.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c8628fd4-832f-44e3-9ba6-06af4b1367bc
⛔ Files ignored due to path filters (5)
content/posts/2026-03-26-network-health/alert.pngis excluded by!**/*.pngcontent/posts/2026-03-26-network-health/recording-rule.pngis excluded by!**/*.pngcontent/posts/2026-03-26-network-health/reviews-5xx-topology.pngis excluded by!**/*.pngcontent/posts/2026-03-26-network-health/reviews-5xx.pngis excluded by!**/*.pngcontent/posts/2026-03-26-network-health/topology.pngis excluded by!**/*.png
📒 Files selected for processing (2)
content/posts/2026-03-26-network-health/index.mddata/authors.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- data/authors.yml
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
content/posts/2026-03-26-network-health/index.md (1)
109-112:⚠️ Potential issue | 🔴 CriticalUse the correct Network Health annotation key everywhere.
Line 111 and both YAML examples (Line 141, Line 206) still use
netobserv.io/network-health. It should benetobserv_io_network_healthto match NetObserv parsing expectations.Proposed fix
-`netobserv.io/network-health` +`netobserv_io_network_health`- annotations: - netobserv.io/network-health: | + annotations: + netobserv_io_network_health: |- annotations: - netobserv.io/network-health: | + annotations: + netobserv_io_network_health: |Also applies to: 140-142, 205-207
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@content/posts/2026-03-26-network-health/index.md` around lines 109 - 112, Replace the incorrect annotation key and placement: change every occurrence of the annotation string "netobserv.io/network-health" to "netobserv_io_network_health" and move the annotation from individual recording rule entries to the PrometheusRule metadata block (ensure the single annotation key "netobserv_io_network_health" appears under the PrometheusRule metadata, not per-rule). Update the two YAML examples and the narrative example where "netobserv.io/network-health" is used so they all use "netobserv_io_network_health" and are shown on the PrometheusRule metadata level.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@content/posts/2026-03-26-network-health/index.md`:
- Line 267: The traffic-generation loop currently escapes the brace range so
bash won't perform brace expansion; locate the command line that reads "for i in
\{1..100\}; do curl -s http://<bookinfo-url>/productpage > /dev/null; done" and
remove the backslashes before the braces so the shell sees an unescaped
brace-range expression and the loop runs 100 iterations as intended.
---
Duplicate comments:
In `@content/posts/2026-03-26-network-health/index.md`:
- Around line 109-112: Replace the incorrect annotation key and placement:
change every occurrence of the annotation string "netobserv.io/network-health"
to "netobserv_io_network_health" and move the annotation from individual
recording rule entries to the PrometheusRule metadata block (ensure the single
annotation key "netobserv_io_network_health" appears under the PrometheusRule
metadata, not per-rule). Update the two YAML examples and the narrative example
where "netobserv.io/network-health" is used so they all use
"netobserv_io_network_health" and are shown on the PrometheusRule metadata
level.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9a5667d5-629b-4adf-b7ae-8c5ec507f227
📒 Files selected for processing (1)
content/posts/2026-03-26-network-health/index.md
| To observe the effect, we generate traffic through the application: | ||
|
|
||
| ```bash | ||
| for i in \{1..100\}; do curl -s http://<bookinfo-url>/productpage > /dev/null; done |
There was a problem hiding this comment.
Fix bash loop brace expansion in the traffic-generation command.
Line 267 escapes braces (\{1..100\}), so brace expansion won’t happen in bash. The loop won’t run 100 times as documented.
Proposed fix
-for i in \{1..100\}; do curl -s http://<bookinfo-url>/productpage > /dev/null; done
+for i in {1..100}; do curl -s http://<bookinfo-url>/productpage > /dev/null; done📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for i in \{1..100\}; do curl -s http://<bookinfo-url>/productpage > /dev/null; done | |
| for i in {1..100}; do curl -s http://<bookinfo-url>/productpage > /dev/null; done |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@content/posts/2026-03-26-network-health/index.md` at line 267, The
traffic-generation loop currently escapes the brace range so bash won't perform
brace expansion; locate the command line that reads "for i in \{1..100\}; do
curl -s http://<bookinfo-url>/productpage > /dev/null; done" and remove the
backslashes before the braces so the shell sees an unescaped brace-range
expression and the loop runs 100 iterations as intended.
|
Thanks @leandroberetta , nice post!
|
| The following images describe some health rules in two different scopes: | ||
|
|
||
|  | ||
| *Alert rule showing as pending or firing in the dashboard at the Namespace scope* |
There was a problem hiding this comment.
Will the number of errors, warnings, and info at the top be in sync with the sum of those amounts in the Global, Nodes, Namespaces, and Workloads tabs? Currently, it is not. In this screenshot, the Namespaces tab show 2 errors, yet the total number of errors is shown as 1 at the top section. This could use some explanation.
Also, if a tab, like Namespaces, has multiple types of issues (e.g. an error and a warning), the icon on the tab appears to only show the most severe issue (e.g. error only). It could also explain this.
| - Use **alert rules** when you need to be notified of immediate issues | ||
| - Use **recording rules** when you want continuous visibility into how a metric evolves over time | ||
|
|
||
| For Network Health, recording rules are often a better fit, as they allow you to observe degradation trends before they become critical. |
There was a problem hiding this comment.
This implies that you should use one or the other. Would you ever want to create an alert and a recording rule for the same issue? Why or why not?
There was a problem hiding this comment.
with flowcollector config, it's not possible to have both. You could have Alert mode for a variant and then add your own recording rule with "netobserv" label.
| name: my-recording-rules | ||
| namespace: netobserv | ||
| labels: | ||
| netobserv: "true" |
There was a problem hiding this comment.
Emphasize "true" needs to be quoted if that is the case.
memodi
left a comment
memodi
left a comment
There was a problem hiding this comment.
thanks @leandroberetta
I have similar comments as Joel about blog lacking flowcollector config example and able to override thresholds, besides that it could also mention about the runbooks.
| - Use **alert rules** when you need to be notified of immediate issues | ||
| - Use **recording rules** when you want continuous visibility into how a metric evolves over time | ||
|
|
||
| For Network Health, recording rules are often a better fit, as they allow you to observe degradation trends before they become critical. |
There was a problem hiding this comment.
For Network Health, recording rules are often a better fit, as they allow you to observe degradation trends before they become critical.
isn't having alert vs recording rule about user preference? The metrics will still be there even if they're for alerts
There was a problem hiding this comment.
It's true, but I think it depends on how they are defined, you might want to see some metric but not as an alert, let's say you want to see the metric, but not as critical (before that), you can configure an alert with info or warning, but is still an alert.
4 participants
Summary by CodeRabbit
Documentation
Chores