Fix Slack notification grouping with label-based channel override#2030
Fix Slack notification grouping with label-based channel override#2030
Conversation
When channel_override uses labels/annotations (e.g. robusta-clusterrel-labels.severity), grouped messages were broken in two ways: 1. The group key only used group_by attributes (e.g. cluster), so findings destined for different channels (sev1, sev2, sev3) were incorrectly merged into a single group. 2. The summary message resolved the channel with empty labels/annotations, falling back to the default channel instead of the override channel. Thread replies then went to a different channel than the summary, breaking threading. Fix: resolve the channel per-finding and include it in the group key when channel_override is configured, and pass the pre-resolved channel to the summary message sender. https://claude.ai/code/session_01L3rB5LufiC9W9hGDgYMTXP
WalkthroughThis change implements channel-aware message grouping in the Slack sink. A resolved channel is determined via ChannelTransformer based on various sources (channel_override, slack_channel, cluster, and finding labels/annotations), then used to separate message groups and passed to the message sender, which conditionally accepts a channel parameter for backward compatibility. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip Migrating from UI to YAML configuration.Use the |
|
✅ Docker image ready for
Use this tag to pull the image for testing. 📋 Copy commandsgcloud auth configure-docker us-central1-docker.pkg.dev
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/robusta-runner:74a4338
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/robusta-runner:74a4338 me-west1-docker.pkg.dev/robusta-development/development/robusta-runner-dev:74a4338
docker push me-west1-docker.pkg.dev/robusta-development/development/robusta-runner-dev:74a4338Patch 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:74a4338 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/robusta/core/sinks/slack/slack_sink.py`:
- Around line 54-60: Guard against missing finding.subject when resolving the
Slack channel: in the call to ChannelTransformer.template (using
self.params.channel_override, self.params.slack_channel, self.cluster_name,
finding.subject.labels, finding.subject.annotations) ensure you first check if
finding.subject is truthy and, if not, pass safe defaults (e.g., empty dicts for
labels and annotations or a default subject) so AttributeError is avoided during
channel resolution; update the code path that computes resolved_channel to
handle a missing subject before dereferencing finding.subject.
🪄 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: 056296d3-36d9-4093-b93c-cadb50b6b0ae
📒 Files selected for processing (2)
src/robusta/core/sinks/slack/slack_sink.pysrc/robusta/integrations/slack/sender.py
Summary
This PR fixes notification grouping in Slack when using
channel_overridewith label/annotation-based templating. Previously, findings routed to different channels via label-based overrides were incorrectly grouped together. Now they are properly separated into distinct groups, summaries, and threads per channel.Key Changes
slack_sink.py:
ChannelTransformerto resolve channels with label/annotation contexthandle_notification_grouping()using the finding's labels and annotationssend_or_update_summary_message()to avoid re-resolving without label contextsender.py:
send_or_update_summary_message()signature to accept an optional pre-resolvedchannelparameterImplementation Details
The fix ensures that when
channel_overrideuses label or annotation templating, the channel resolution happens in the sink layer where full finding context (labels/annotations) is available. This resolved channel is then:This maintains backward compatibility while properly supporting dynamic channel routing based on finding metadata.
https://claude.ai/code/session_01L3rB5LufiC9W9hGDgYMTXP