Skip to content

Conversation

@Avi-Robusta
Copy link
Contributor

For example adding this config

global_config:
...
  prometheus_rule_group_name: testing-123
  prometheus_rule_custom_labels:
    label1: value1
    label2: value2

created this prometheus rule from the UI

apiVersion: monitoring.coreos.com/v1
kind: PrometheusRule
metadata:
  labels:
    label1: value1
    label2: value2
    release: robusta
    release.app: robusta-resource-management
    role: alert-rules
  name: robusta-prometheus.rules--1
spec:
  groups:
    - name: testing-123
      rules:
        - alert: AlertmanagerClusterCrashlooping
        ...

@coderabbitai
Copy link

coderabbitai bot commented Nov 19, 2025

Warning

Rate limit exceeded

@Avi-Robusta has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 41 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between a487c62 and 28bb475.

📒 Files selected for processing (1)
  • src/robusta/core/sinks/robusta/rrm/prometheus_alert_resource_manager.py (4 hunks)

Walkthrough

Adds Helm settings for PrometheusRule group name and custom labels, threads them from registry → RRM → PrometheusAlertResourceHandler, updates handler to merge/apply labels and group name on created/updated PrometheusRule CRs, and inserts documentation describing these settings (duplicated in two places).

Changes

Cohort / File(s) Summary
Documentation
docs/setup-robusta/alertsui.rst
Added a new section "Customizing PrometheusRule Labels and Group Name" with instructions to set prometheus_rule_group_name and prometheus_rule_custom_labels under globalConfig in generated_values.yaml. The section was inserted in two locations (after Helm upgrade flow and inside External Prometheus tab).
Helm configuration
helm/robusta/values.yaml
Added globalConfig.prometheus_rule_group_name (default "kubernetes-apps") and globalConfig.prometheus_rule_custom_labels (empty mapping).
Sink / RRM wiring
src/robusta/core/sinks/robusta/robusta_sink.py , src/robusta/core/sinks/robusta/rrm/rrm.py
RRM.__init__ now accepts a registry parameter; robusta_sink.py passes registry into RRM. rrm.py reads globalConfig from registry and extracts custom_labels and group_name, then passes them when creating PrometheusAlertResourceHandler.
PrometheusRule resource handler
src/robusta/core/sinks/robusta/rrm/prometheus_alert_resource_manager.py
PrometheusAlertResourceHandler.__init__ gained custom_labels and optional group_name. Snapshot creation and CR update logic now merges base labels with custom_labels, applies/updates the PrometheusRule group name, and preserves/merges existing metadata.labels on updates.

Sequence Diagram(s)

sequenceDiagram
    participant Sink as RobustaSink
    participant RRM as RRM
    participant Registry as Registry\n(global_config)
    participant Handler as PrometheusAlertResourceHandler

    Sink->>RRM: __init__(dal, cluster, account_id, registry)
    RRM->>Registry: read global_config
    Registry-->>RRM: {prometheus_rule_group_name, prometheus_rule_custom_labels}
    RRM->>Handler: __init__(..., custom_labels, group_name)
    Handler->>Handler: store __custom_labels & __group_name

    Note over Handler: On PrometheusRule create/update
    Handler->>Handler: __get_snapshot_body()
    Handler->>Handler: build base labels (release, role, id)
    Handler->>Handler: merge __custom_labels into labels
    Handler->>Handler: set group name on PrometheusRule metadata
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20-30 minutes

  • Review points:
    • Verify globalConfig extraction from registry in rrm.py.
    • Confirm PrometheusAlertResourceHandler merges custom labels without overwriting unrelated existing metadata.labels.
    • Ensure robusta_sink.py supplies registry in all instantiation paths.
    • Check documentation placement (duplicate insertion) is intentional.

Suggested reviewers

  • moshemorad
  • arikalon1
  • pavangudiwada

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title is terse but directly related to the main changeset—adding custom labels for PrometheusRules. However, it lacks clarity about what 'custom rule label' entails and could be more descriptive.
Description check ✅ Passed The description clearly relates to the changeset by showing a concrete example of how the new configuration options (prometheus_rule_group_name and prometheus_rule_custom_labels) work in practice with actual YAML config and output.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
src/robusta/core/sinks/robusta/robusta_sink.py (1)

95-99: RRM now correctly receives registry; optionally use self.registry for clarity

Passing registry into RRM matches the updated constructor and lets it read global_config for PrometheusRule settings. For consistency and readability you might consider passing self.registry instead of the raw parameter, since SinkBase already stores it.

src/robusta/core/sinks/robusta/rrm/rrm.py (1)

16-31: RRM initialization correctly pulls PrometheusRule config; consider cleaning up unused account_id

Using registry.get_global_config() here to derive custom_labels and group_name and passing them into PrometheusAlertResourceHandler is a clean way to thread the new settings without tying RRM to RobustaSink.

Two minor points:

  • These values are read once at RRM construction. If you ever want live updates on config_reload, you’d need to re-read global_config or recreate the handler; currently a Helm upgrade/restart is required, which matches the docs.
  • Ruff correctly reports account_id as unused in the constructor. When it’s safe to touch the API, consider removing it or storing/using it (e.g., for logging/metrics) to avoid drift between signature and implementation.
src/robusta/core/sinks/robusta/rrm/prometheus_alert_resource_manager.py (1)

18-37: Constructor wiring for custom_labels and group_name looks good; consider updating group name on existing CRs

The new constructor parameters and defaults (group_name or "kubernetes-apps", custom_labels or {}) are consistent with the Helm defaults and RRM usage.

One behavioral nuance: in __create_cr_rules, when existing_cr_obj is present, you only update groups[0]["rules"] and leave the existing group name as-is. That means changing prometheus_rule_group_name in globalConfig will not update the spec.groups[0].name of already-created PrometheusRule CRDs; only CRDs created after the change will use the new group name.

If you want the configuration to apply uniformly after a Helm upgrade, you could also set the group name when updating existing objects, e.g.:

             if existing_cr_obj:
                 if not existing_cr_obj.get("spec"):
                     existing_cr_obj["spec"] = {}
 
                 if not existing_cr_obj["spec"].get("groups"):
                     existing_cr_obj["spec"]["groups"] = [{}]
 
-                existing_cr_obj["spec"]["groups"][0]["rules"] = rules
+                existing_cr_obj["spec"]["groups"][0]["rules"] = rules
+                existing_cr_obj["spec"]["groups"][0]["name"] = self.__group_name

This keeps behavior aligned with the new configuration knobs after upgrades.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 621ad02 and 2a3e7c1.

📒 Files selected for processing (5)
  • docs/setup-robusta/alertsui.rst (1 hunks)
  • helm/robusta/values.yaml (1 hunks)
  • src/robusta/core/sinks/robusta/robusta_sink.py (1 hunks)
  • src/robusta/core/sinks/robusta/rrm/prometheus_alert_resource_manager.py (3 hunks)
  • src/robusta/core/sinks/robusta/rrm/rrm.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/robusta/core/sinks/robusta/rrm/rrm.py (3)
src/robusta/core/sinks/robusta/rrm/account_resource_fetcher.py (1)
  • AccountResourceFetcher (9-22)
src/robusta/core/sinks/robusta/robusta_sink.py (1)
  • get_global_config (720-721)
src/robusta/core/sinks/robusta/rrm/prometheus_alert_resource_manager.py (1)
  • PrometheusAlertResourceHandler (17-207)
src/robusta/core/sinks/robusta/robusta_sink.py (1)
src/robusta/core/sinks/robusta/rrm/rrm.py (1)
  • RRM (15-90)
🪛 Ruff (0.14.5)
src/robusta/core/sinks/robusta/rrm/rrm.py

16-16: Unused method argument: account_id

(ARG002)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: run_tests
  • GitHub Check: run_tests
  • GitHub Check: Deploy docs
🔇 Additional comments (2)
helm/robusta/values.yaml (1)

37-41: New PrometheusRule config options align with handler behavior

Defining prometheus_rule_group_name (defaulting to "kubernetes-apps") and prometheus_rule_custom_labels under globalConfig matches how RRM and PrometheusAlertResourceHandler consume these values. The defaults are sensible and preserve existing behavior when unset.

docs/setup-robusta/alertsui.rst (1)

69-91: Docs accurately describe new PrometheusRule customization knobs

The new section clearly explains how to set prometheus_rule_group_name and prometheus_rule_custom_labels under globalConfig, and correctly scopes them to PrometheusRules created from the Alerts UI. This matches the Helm values and handler implementation.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
src/robusta/core/sinks/robusta/rrm/prometheus_alert_resource_manager.py (1)

78-84: Avoid letting custom labels break the internal release.app selector.

The labels.update(self.__custom_labels) on line 84 allows users to override the internal identification label (release.app). If overridden, the __list_rules_objects method won't discover CRDs created with the custom value, leading to sync conflicts.

This issue was already identified in a previous review. Please refer to the earlier comment on lines 78-92 for the recommended fix.

🧹 Nitpick comments (1)
src/robusta/core/sinks/robusta/rrm/prometheus_alert_resource_manager.py (1)

22-22: Make custom_labels optional with a default value for defensive coding.

The custom_labels parameter is currently required. If a caller doesn't provide it, a TypeError will be raised. Consider making it optional to improve robustness.

Apply this diff to make the parameter optional:

     def __init__(
         self,
         cluster: str,
         resource_kind: ResourceKind,
-        custom_labels: Dict[str, str],
+        custom_labels: Optional[Dict[str, str]] = None,
         group_name: Optional[str] = None,
     ):

Then update line 36 to handle the None case:

         self.__group_name = group_name or "kubernetes-apps"
-        self.__custom_labels = custom_labels
+        self.__custom_labels = custom_labels or {}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2a3e7c1 and 62dd930.

📒 Files selected for processing (1)
  • src/robusta/core/sinks/robusta/rrm/prometheus_alert_resource_manager.py (4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: run_tests
  • GitHub Check: run_tests

Copy link
Contributor

@arikalon1 arikalon1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice work!

@arikalon1 arikalon1 merged commit 910f863 into master Nov 19, 2025
8 checks passed
@arikalon1 arikalon1 deleted the ROB-1901-custom-rule-label branch November 19, 2025 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants