Skip to content

Conversation

@Avi-Robusta
Copy link
Contributor

No description provided.

@coderabbitai
Copy link

coderabbitai bot commented Jan 14, 2026

Walkthrough

New documentation adding a Google Managed Alertmanager integration guide and updating related Prometheus/metric-provider docs to reference it, including sample YAMLs, OperatorConfig guidance, and verification steps for webhook delivery.

Changes

Cohort / File(s) Summary
New Google Managed Alertmanager Integration Guide
docs/configuration/alertmanager-integration/google-managed-alertmanager.rst
Added a new guide detailing requirements (Robusta account_id, signing_key), Secret and OperatorConfig examples, requirement that alerts include cluster_name label, sample Rules resource, and curl-based verification steps for webhook delivery.
Cross-reference and navigation updates
docs/configuration/alertmanager-integration/google-managed-prometheus.rst, docs/configuration/index.rst, docs/configuration/metric-providers-google.rst
Inserted a note linking to the new Google Managed Alertmanager guide, added a navigation grid item, and updated Next Steps to include the new guide alongside existing Google Managed Prometheus references.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • RoiGlinik
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive No pull request description was provided, making it impossible to assess whether it relates to the changeset. Add a description explaining the changes, such as documenting the new Google Managed Alertmanager integration and related documentation updates.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: adding Alertmanager support for GCP, which aligns with the new Google Managed Alertmanager documentation and integration guide.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch gcp-alertmanager


📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e7f910c and 0b1906c.

📒 Files selected for processing (1)
  • docs/configuration/alertmanager-integration/google-managed-alertmanager.rst
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/configuration/alertmanager-integration/google-managed-alertmanager.rst
⏰ 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

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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.

@github-actions
Copy link

github-actions bot commented Jan 14, 2026

Docker image ready for 629dd5c (built in 37s)

⚠️ Warning: does not support ARM (ARM images are built on release only - not on every PR)

Use this tag to pull the image for testing.

📋 Copy commands

⚠️ Temporary images are deleted after 30 days. Copy to a permanent registry before using them:

gcloud auth configure-docker us-central1-docker.pkg.dev
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/robusta-runner:629dd5c
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/robusta-runner:629dd5c me-west1-docker.pkg.dev/robusta-development/development/robusta-runner-dev:629dd5c
docker push me-west1-docker.pkg.dev/robusta-development/development/robusta-runner-dev:629dd5c

Patch Helm values in one line:

helm upgrade --install robusta robusta/robusta \
  --reuse-values \
  --set runner.image=me-west1-docker.pkg.dev/robusta-development/development/robusta-runner-dev:629dd5c

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

🤖 Fix all issues with AI agents
In `@docs/configuration/alertmanager-integration/google-managed-alertmanager.rst`:
- Around line 106-108: The Alertmanager integration docs show a curl example
using "Authorization: Bearer <ACCOUNT_ID> <SIGNING_KEY>" but the Alertmanager
YAML snippet uses authorization.credentials without the "Bearer" prefix; update
the documentation to either (a) change the Alertmanager secret/config example so
authorization.credentials explicitly includes the Bearer prefix (e.g.,
credentials: 'Bearer <ACCOUNT_ID> <SIGNING_KEY>') or (b) add a clarifying
sentence next to http_config.authorization / authorization.credentials stating
that the full header must include the "Bearer " prefix to match the curl
example, ensuring consistency between the YAML example and the curl validation
snippet.
- Around line 36-40: The authorization block is missing the Bearer token type
which causes the Authorization header to omit the required "Bearer" prefix;
update the authorization mapping by either adding a new key "type: Bearer"
alongside the existing "credentials" field or by prefixing the credentials value
with "Bearer " (i.e., change credentials to "Bearer <ACCOUNT_ID>
<SIGNING_KEY>"), ensuring the "authorization" block contains either type: Bearer
+ credentials or credentials starting with "Bearer ".
🧹 Nitpick comments (2)
docs/configuration/alertmanager-integration/google-managed-alertmanager.rst (2)

62-71: Clarify which externalLabels are optional.

The example includes location and project_id in addition to the required cluster_name label. Consider adding a comment in the YAML or a note indicating which fields are optional versus required, to help users customize the configuration appropriately.

📝 Suggested documentation enhancement
 collection:
   externalLabels:
-    cluster_name: gmp-demo-cluster   # Match your Robusta cluster_name, or use "external"
-    location: us-central1-c
-    project_id: my-gcp-project
+    cluster_name: gmp-demo-cluster   # Required: Match your Robusta cluster_name, or use "external"
+    location: us-central1-c          # Optional: GCP location for organizational purposes
+    project_id: my-gcp-project       # Optional: GCP project identifier
 rules:
   externalLabels:
-    cluster_name: gmp-demo-cluster
-    location: us-central1-c
-    project_id: my-gcp-project
+    cluster_name: gmp-demo-cluster   # Required: Must match the collection cluster_name
+    location: us-central1-c          # Optional: Should match collection if specified
+    project_id: my-gcp-project       # Optional: Should match collection if specified

128-135: Consider clarifying the purpose of duplicate cluster labels.

The test payload includes both cluster_name: "external" and cluster: "external" labels. If both are necessary for compatibility or different purposes, consider adding a comment explaining why both are included.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a005f90 and e7f910c.

📒 Files selected for processing (4)
  • docs/configuration/alertmanager-integration/google-managed-alertmanager.rst
  • docs/configuration/alertmanager-integration/google-managed-prometheus.rst
  • docs/configuration/index.rst
  • docs/configuration/metric-providers-google.rst
⏰ 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). (5)
  • GitHub Check: run_tests
  • GitHub Check: run_tests
  • GitHub Check: build
  • GitHub Check: Deploy docs
  • GitHub Check: Deploy docs
🔇 Additional comments (5)
docs/configuration/alertmanager-integration/google-managed-alertmanager.rst (2)

73-97: Well-structured demo alert.

The test alert using vector(1) will fire immediately and continuously, making it ideal for verifying the webhook integration. The annotations clearly explain its purpose and remind users to clean it up.


8-8: Verify the "external" cluster_name value is documented and supported.

The documentation references using "external" as a cluster_name value for alerts that don't belong to a specific runner. Please confirm that this is an officially supported/reserved value in Robusta's codebase (check cluster_name validation logic, configuration schemas, and any relevant code handling) and ensure this usage is clearly documented with examples so users understand when to use "external" versus a custom cluster name.

docs/configuration/index.rst (1)

49-52: LGTM - Navigation link added correctly.

The new grid item for Google Managed Alertmanager follows the established pattern and correctly links to the new documentation page.

docs/configuration/alertmanager-integration/google-managed-prometheus.rst (1)

12-14: Good cross-reference to the new guide.

The note appropriately directs users to the dedicated Google Managed Alertmanager guide, helping them find the correct documentation for their use case.

docs/configuration/metric-providers-google.rst (1)

96-97: Clear guidance between recommended and legacy options.

Good job distinguishing the recommended Google Managed Alertmanager integration from the legacy guide. This helps users make the right choice for their setup.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Sheeproid
Sheeproid previously approved these changes Jan 14, 2026
@arikalon1 arikalon1 merged commit ccf0065 into master Jan 15, 2026
5 checks passed
@arikalon1 arikalon1 deleted the gcp-alertmanager branch January 15, 2026 17:16
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.

4 participants