feat: add --slack-full-width for full-width Slack alerts with markdown test result tables #2107
Conversation
|
👋 @michrzan |
📝 WalkthroughWalkthroughThe PR introduces full-width Slack alert formatting by adding a Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI as monitor CLI
participant Config as Config
participant Integration as Integrations
participant MessageBuilder as SlackMessageBuilder
participant SlackAPI as Slack API
User->>CLI: Run monitor --slack-full-width
CLI->>Config: Initialize with slack_full_width=True
Config->>Integration: Pass slack_full_width to get_integration()
Integration->>Integration: Check slack_full_width flag
Integration->>MessageBuilder: Create SlackAlertMessageBuilder(full_width=True)
MessageBuilder->>MessageBuilder: Format test_rows_sample as markdown table
MessageBuilder->>MessageBuilder: Place blocks in main message (not attachments)
MessageBuilder->>MessageBuilder: Inject rich_text block for full-width layout
MessageBuilder->>SlackAPI: Send formatted alert with table in blocks
SlackAPI-->>User: Display full-width alert with markdown table
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
michrzan
requested a deployment
to
elementary_test_env
GitHub Actions
Waiting
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@docs/oss/deployment-and-configuration/slack.mdx`:
- Around line 56-67: Change the lowercase "markdown" occurrences to the proper
noun "Markdown" in the Slack docs section that describes full-width alerts (the
paragraph referencing `--slack-full-width` and the sentence about test result
samples). Locate the two instances where "markdown table" and "markdown tables"
are used and update them to "Markdown table" and "Markdown tables" respectively
so the term is capitalized consistently.
In `@docs/oss/guides/alerts/send-slack-alerts.mdx`:
- Around line 27-28: Replace the lowercase word "markdown" with capitalized
"Markdown" in the sentence that mentions `--slack-full-width` so the doc reads
"show test results as Markdown tables"; update the phrase near the
`--slack-full-width` reference to use the corrected capitalization.
In `@elementary/monitor/data_monitoring/alerts/integrations/slack/slack.py`:
- Around line 225-234: The markdown table from list_of_dicts_to_markdown_table
can be truncated mid-row by get_limited_markdown_msg causing malformed markdown;
update the Slack message construction in slack.py (the code that calls
create_text_section_block with the code-fenced test_rows_sample) to first check
the full table length/character limit using the same logic as the test-query
branch, and if it exceeds the limit, either (a) render a safe summary by
converting only the first N rows (or columns) and append a clear "(truncated)"
note, or (b) replace the code block with a short text section that says the
table was truncated and provide row/column counts; ensure this uses
list_of_dicts_to_markdown_table for the sampled subset and then pass the safe
string to create_text_section_block/get_limited_markdown_msg so no markdown is
cut mid-structure.
🧹 Nitpick comments (4)
elementary/utils/json_utils.py (1)
101-112: Consider precision loss for very small decimal floats.The fixed
.10fformat means values with magnitude smaller than ~1e-10 will be rounded to zero. For example,0.00000000001would render as"0.0". If test result data could contain very small non-zero decimals, this might silently hide meaningful values.If this is acceptable for the display use case, no change needed — just flagging the edge case.
tests/unit/monitor/data_monitoring/alerts/integrations/slack/test_slack_alert_message_builder.py (1)
199-220: Consider using an exact assertion for block count.Line 213:
assert len(blocks) >= 5— given the schema has exactly one title block, one preview block, and one detail block, the expected count is deterministically 5 (rich_text + title + divider + preview + detail). Using== 5would make the test stricter and catch unexpected extra blocks.Proposed fix
- assert len(blocks) >= 5 + assert len(blocks) == 5elementary/monitor/data_monitoring/alerts/integrations/slack/message_builder.py (1)
38-46: The emptyrich_textblock hack relies on undocumented Slack behavior.The empty
rich_textblock to force full-width rendering is a known workaround, but it's not part of the official Slack Block Kit API contract. Slack could change this behavior without notice. The inline comment is helpful — consider also adding a link to any community discussion or internal reference documenting this technique, so future maintainers understand the fragility.elementary/monitor/data_monitoring/alerts/integrations/slack/slack.py (1)
199-205: Inconsistent description rendering between dbt and elementary test templates.
_get_dbt_test_templatenow uses a simplified single-block approach (description_text = alert.test_description or "_No description_"), while_get_elementary_test_template(lines 363–378) still uses the old if/else with separatecreate_text_section_block+create_context_blockfor descriptions. Consider aligning both templates for consistency — unless the difference is intentional due to rendering requirements.
|
|
||
| ## Full-width alerts | ||
|
|
||
| By default, Slack alerts use a narrower layout with some content in attachments. To use the full message width and show test results as a markdown table in the main message body, pass the flag when running the monitor: | ||
|
|
||
| ```shell | ||
| edr monitor --slack-token <your_slack_token> --slack-channel-name <channel> --slack-full-width | ||
| ``` | ||
|
|
||
| With `--slack-full-width`, alerts are sent using Slack Block Kit in the main message body instead of attachments, so the full channel width is used and test result samples appear as formatted markdown tables. | ||
|
|
||
| --- |
There was a problem hiding this comment.
Capitalize "Markdown" as a proper noun.
Static analysis correctly flags that "markdown" should be "Markdown" (lines 59 and 65).
Proposed fix
-By default, Slack alerts use a narrower layout with some content in attachments. To use the full message width and show test results as a markdown table in the main message body, pass the flag when running the monitor:
+By default, Slack alerts use a narrower layout with some content in attachments. To use the full message width and show test results as a Markdown table in the main message body, pass the flag when running the monitor:-With `--slack-full-width`, alerts are sent using Slack Block Kit in the main message body instead of attachments, so the full channel width is used and test result samples appear as formatted markdown tables.
+With `--slack-full-width`, alerts are sent using Slack Block Kit in the main message body instead of attachments, so the full channel width is used and test result samples appear as formatted Markdown tables.🧰 Tools
🪛 LanguageTool
[uncategorized] ~59-~59: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...essage width and show test results as a markdown table in the main message body, pass th...
(MARKDOWN_NNP)
[uncategorized] ~65-~65: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ...test result samples appear as formatted markdown tables. ---
(MARKDOWN_NNP)
🤖 Prompt for AI Agents
In `@docs/oss/deployment-and-configuration/slack.mdx` around lines 56 - 67, Change
the lowercase "markdown" occurrences to the proper noun "Markdown" in the Slack
docs section that describes full-width alerts (the paragraph referencing
`--slack-full-width` and the sentence about test result samples). Locate the two
instances where "markdown table" and "markdown tables" are used and update them
to "Markdown table" and "Markdown tables" respectively so the term is
capitalized consistently.
| Add `--slack-full-width` to use the full message width and show test results as markdown tables. See [Slack setup - Full-width alerts](/oss/deployment-and-configuration/slack#full-width-alerts). | ||
|
|
There was a problem hiding this comment.
Same capitalization nit: "markdown" → "Markdown".
Proposed fix
-Add `--slack-full-width` to use the full message width and show test results as markdown tables. See [Slack setup - Full-width alerts](/oss/deployment-and-configuration/slack#full-width-alerts).
+Add `--slack-full-width` to use the full message width and show test results as Markdown tables. See [Slack setup - Full-width alerts](/oss/deployment-and-configuration/slack#full-width-alerts).📝 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.
| Add `--slack-full-width` to use the full message width and show test results as markdown tables. See [Slack setup - Full-width alerts](/oss/deployment-and-configuration/slack#full-width-alerts). | |
| Add `--slack-full-width` to use the full message width and show test results as Markdown tables. See [Slack setup - Full-width alerts](/oss/deployment-and-configuration/slack#full-width-alerts). |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~27-~27: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ... message width and show test results as markdown tables. See [Slack setup - Full-width a...
(MARKDOWN_NNP)
🤖 Prompt for AI Agents
In `@docs/oss/guides/alerts/send-slack-alerts.mdx` around lines 27 - 28, Replace
the lowercase word "markdown" with capitalized "Markdown" in the sentence that
mentions `--slack-full-width` so the doc reads "show test results as Markdown
tables"; update the phrase near the `--slack-full-width` reference to use the
corrected capitalization.
| test_rows_sample_table = list_of_dicts_to_markdown_table( | ||
| alert.test_rows_sample | ||
| ) | ||
| result.extend( | ||
| [ | ||
| self.message_builder.create_context_block( | ||
| ["*Test results sample*"] | ||
| self.message_builder.create_text_section_block( | ||
| f"{ICON_TO_UNICODE[Icon.MAGNIFYING_GLASS]} *Test results sample*" | ||
| ), | ||
| self.message_builder.create_text_section_block( | ||
| f"```{alert.test_rows_sample}```" | ||
| f"```{test_rows_sample_table}```" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and examine get_limited_markdown_msg function
rg -n --type=py -A 15 'def get_limited_markdown_msg'Repository: elementary-data/elementary
Length of output: 1539
🏁 Script executed:
#!/bin/bash
# Get the constants and more context
rg -n --type=py 'CONTINUATION_SYMBOL|_LONGEST_MARKDOWN_SUFFIX_LEN' elementary/clients/slack/slack_message_builder.py | head -20
# Also check the lines in the slack.py around 248-252 to see test query handling
sed -n '240,260p' elementary/monitor/data_monitoring/alerts/integrations/slack/slack.pyRepository: elementary-data/elementary
Length of output: 1347
🏁 Script executed:
#!/bin/bash
# Check the create_text_section_block implementation
rg -n --type=py -A 5 'def create_text_section_block' elementary/clients/slack/slack_message_builder.py
# Also get the full context around lines 225-234 in slack.py
sed -n '215,240p' elementary/monitor/data_monitoring/alerts/integrations/slack/slack.pyRepository: elementary-data/elementary
Length of output: 1294
🏁 Script executed:
#!/bin/bash
# Let's see what SectionBlock.text_max_length is and understand the full flow
rg -n --type=py 'text_max_length|class SectionBlock' | head -20
# Also check if there are other similar patterns for graceful handling
rg -n --type=py 'The test query was too long' -B 2 -A 2Repository: elementary-data/elementary
Length of output: 1952
🏁 Script executed:
#!/bin/bash
# Let's examine the list_of_dicts_to_markdown_table function to understand table structure
rg -n --type=py -A 15 'def list_of_dicts_to_markdown_table'
# Also check if there's any handling for table size elsewhere
rg -n --type=py 'test_rows_sample_table' elementary/monitor/data_monitoring/alerts/integrations/slack/slack.py -B 2 -A 8Repository: elementary-data/elementary
Length of output: 1957
🏁 Script executed:
#!/bin/bash
# Let's check if there's any test for how get_limited_markdown_msg handles tables
rg -n --type=py 'get_limited_markdown_msg' tests/ -B 2 -A 5
# Also check the implementation once more to understand the exact truncation
sed -n '58,69p' elementary/clients/slack/slack_message_builder.py
# Check what happens with markdown in truncation
sed -n '20,25p' elementary/clients/slack/slack_message_builder.pyRepository: elementary-data/elementary
Length of output: 2240
Markdown table truncation can produce malformed output — consider graceful handling for large tables.
list_of_dicts_to_markdown_table output is wrapped in a code block and passed to create_text_section_block, which calls get_limited_markdown_msg. This function truncates at exactly 3000 characters without respecting markdown structure. A table with many rows/columns will be cut mid-row, producing malformed markdown (e.g., unclosed pipes).
The test query handling (lines 248–252) shows a better pattern: detect when content exceeds the limit and provide an alternative explanation or note like "(truncated)" rather than rendering broken markdown. Consider applying a similar approach to tables.
🤖 Prompt for AI Agents
In `@elementary/monitor/data_monitoring/alerts/integrations/slack/slack.py` around
lines 225 - 234, The markdown table from list_of_dicts_to_markdown_table can be
truncated mid-row by get_limited_markdown_msg causing malformed markdown; update
the Slack message construction in slack.py (the code that calls
create_text_section_block with the code-fenced test_rows_sample) to first check
the full table length/character limit using the same logic as the test-query
branch, and if it exceeds the limit, either (a) render a safe summary by
converting only the first N rows (or columns) and append a clear "(truncated)"
note, or (b) replace the code block with a short text section that says the
table was truncated and provide row/column counts; ensure this uses
list_of_dicts_to_markdown_table for the sampled subset and then pass the safe
string to create_text_section_block/get_limited_markdown_msg so no markdown is
cut mid-structure.
1 participant
Summary
This PR adds a --slack-full-width option that allows Slack alerts to use the full message width by rendering the entire message with Block Kit (no attachments) and displaying test result samples as markdown tables.
This significantly improves readability when test results contain many columns or long values.
Problem
When the Test Results Sample includes multiple columns or long strings, the current Slack alerts become difficult to read due to the narrow attachment layout. In practice, this makes it hard for alert recipients to quickly understand the context and take action.
or when there is more columns like this:
Solution
With the --slack-full-width option enabled, Slack alerts are rendered at full width and test results are consistently shown as markdown tables:
or when there is more columns like this:
Admittedly, it looks better on a wide screen.
Implementation details
This is achieved by:
Trade-offs
As a result of moving away from attachments, the colored severity indicator (yellow/red pipe on the left) is no longer available. This is a known limitation of the approach.
Notes
This resolves #1079.
This is jsut something I am using internally and wanted to share. Happy to discuss alternatives, trade-offs, or refinements!
Summary by CodeRabbit
Release Notes
New Features
--slack-full-widthflag for monitor command to display Slack alerts with full message width, rendering test results as markdown tables directly in the message body instead of attachmentsDocumentation