-
Notifications
You must be signed in to change notification settings - Fork 209
feat: add --slack-full-width for full-width Slack alerts with markdown test result tables #2107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
b4977bd
1945d18
e4d81f8
bf14655
99cb783
68ddb44
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -24,6 +24,8 @@ Make sure to run the following command after your dbt runs and tests: | |||||
| edr monitor --slack-token <your_slack_token> --slack-channel-name <slack_channel_to_post_at> --group-by [table | alert] | ||||||
| ``` | ||||||
|
|
||||||
| 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). | ||||||
|
|
||||||
|
Comment on lines
+27
to
+28
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Suggested change
🧰 Tools🪛 LanguageTool[uncategorized] ~27-~27: Did you mean the formatting language “Markdown” (= proper noun)? (MARKDOWN_NNP) 🤖 Prompt for AI Agents |
||||||
| Or just `edr monitor` if you used `config.yml`. Please note that when you specify the --slack-channel-name, it's the | ||||||
| default channel name to which all the alerts will be sent that are not attributed to any custom channel. Therefore, | ||||||
| if you execute several `edr monitor` commands at the same time with different `slack-channel-name` arguments, they can | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,8 @@ | |
| from elementary.clients.slack.schema import SlackBlocksType, SlackMessageSchema | ||
| from elementary.clients.slack.slack_message_builder import MessageColor | ||
| from elementary.config.config import Config | ||
| from elementary.messages.blocks import Icon | ||
| from elementary.messages.formats.unicode import ICON_TO_UNICODE | ||
| from elementary.monitor.alerts.alerts_groups import AlertsGroup, GroupedByTableAlerts | ||
| from elementary.monitor.alerts.alerts_groups.base_alerts_group import BaseAlertsGroup | ||
| from elementary.monitor.alerts.model_alert import ModelAlertModel | ||
|
|
@@ -26,6 +28,7 @@ | |
| ) | ||
| from elementary.tracking.tracking_interface import Tracking | ||
| from elementary.utils.json_utils import ( | ||
| list_of_dicts_to_markdown_table, | ||
| list_of_lists_of_strings_to_comma_delimited_unique_strings, | ||
| ) | ||
| from elementary.utils.log import get_logger | ||
|
|
@@ -78,7 +81,9 @@ def __init__( | |
| self.config = config | ||
| self.tracking = tracking | ||
| self.override_config_defaults = override_config_defaults | ||
| self.message_builder = SlackAlertMessageBuilder() | ||
| self.message_builder = SlackAlertMessageBuilder( | ||
| full_width=config.slack_full_width | ||
| ) | ||
| super().__init__() | ||
|
|
||
| # Enforce typing | ||
|
|
@@ -116,7 +121,10 @@ def _get_dbt_test_template( | |
| title = [ | ||
| self.message_builder.create_header_block( | ||
| f"{self._get_display_name(alert.status)}: {alert.summary}" | ||
| ) | ||
| ), | ||
| self.message_builder.create_text_section_block( | ||
| "Powered by <https://www.elementary-data.com/|Elementary>" | ||
| ), | ||
| ] | ||
| if alert.suppression_interval: | ||
| title.extend( | ||
|
|
@@ -165,8 +173,11 @@ def _get_dbt_test_template( | |
| ) | ||
|
|
||
| compacted_sections = [] | ||
| if COLUMN_FIELD in (alert.alert_fields or DEFAULT_ALERT_FIELDS): | ||
| compacted_sections.append(f"*Column*\n{alert.column_name or '_No column_'}") | ||
| if ( | ||
| COLUMN_FIELD in (alert.alert_fields or DEFAULT_ALERT_FIELDS) | ||
| and alert.column_name | ||
| ): | ||
| compacted_sections.append(f"*Column*\n{alert.column_name}") | ||
| if TAGS_FIELD in (alert.alert_fields or DEFAULT_ALERT_FIELDS): | ||
| tags = prettify_and_dedup_list(alert.tags or []) | ||
| compacted_sections.append(f"*Tags*\n{tags or '_No tags_'}") | ||
|
|
@@ -186,21 +197,12 @@ def _get_dbt_test_template( | |
| ) | ||
|
|
||
| if DESCRIPTION_FIELD in (alert.alert_fields or DEFAULT_ALERT_FIELDS): | ||
| if alert.test_description: | ||
| preview.extend( | ||
| [ | ||
| self.message_builder.create_text_section_block("*Description*"), | ||
| self.message_builder.create_context_block( | ||
| [alert.test_description] | ||
| ), | ||
| ] | ||
| ) | ||
| else: | ||
| preview.append( | ||
| self.message_builder.create_text_section_block( | ||
| "*Description*\n_No description_" | ||
| ) | ||
| description_text = alert.test_description or "_No description_" | ||
| preview.append( | ||
| self.message_builder.create_text_section_block( | ||
| f"*Description*\n{description_text}" | ||
| ) | ||
| ) | ||
|
|
||
| result = [] | ||
| if ( | ||
|
|
@@ -209,7 +211,7 @@ def _get_dbt_test_template( | |
| ): | ||
| result.extend( | ||
| [ | ||
| self.message_builder.create_context_block(["*Result message*"]), | ||
| self.message_builder.create_text_section_block("*Result message*"), | ||
| self.message_builder.create_text_section_block( | ||
| f"```{alert.error_message.strip()}```" | ||
| ), | ||
|
|
@@ -220,13 +222,16 @@ def _get_dbt_test_template( | |
| TEST_RESULTS_SAMPLE_FIELD in (alert.alert_fields or DEFAULT_ALERT_FIELDS) | ||
| and alert.test_rows_sample | ||
| ): | ||
| 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}```" | ||
|
Comment on lines
+225
to
+234
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 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.
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 |
||
| ), | ||
| ] | ||
| ) | ||
|
|
@@ -235,7 +240,9 @@ def _get_dbt_test_template( | |
| TEST_QUERY_FIELD in (alert.alert_fields or DEFAULT_ALERT_FIELDS) | ||
| and alert.test_results_query | ||
| ): | ||
| result.append(self.message_builder.create_context_block(["*Test query*"])) | ||
| result.append( | ||
| self.message_builder.create_text_section_block("*Test query*") | ||
| ) | ||
|
|
||
| msg = f"```{alert.test_results_query}```" | ||
| if len(msg) > SectionBlock.text_max_length: | ||
|
|
@@ -330,8 +337,11 @@ def _get_elementary_test_template( | |
| ) | ||
|
|
||
| compacted_sections = [] | ||
| if COLUMN_FIELD in (alert.alert_fields or DEFAULT_ALERT_FIELDS): | ||
| compacted_sections.append(f"*Column*\n{alert.column_name or '_No column_'}") | ||
| if ( | ||
| COLUMN_FIELD in (alert.alert_fields or DEFAULT_ALERT_FIELDS) | ||
| and alert.column_name | ||
| ): | ||
| compacted_sections.append(f"*Column*\n{alert.column_name}") | ||
| if TAGS_FIELD in (alert.alert_fields or DEFAULT_ALERT_FIELDS): | ||
| tags = prettify_and_dedup_list(alert.tags or []) | ||
| compacted_sections.append(f"*Tags*\n{tags or '_No tags_'}") | ||
|
|
@@ -1194,7 +1204,9 @@ def _create_single_alert_details_blocks( | |
| if result: | ||
| details_blocks.extend( | ||
| [ | ||
| self.message_builder.create_text_section_block(":mag: *Result*"), | ||
| self.message_builder.create_text_section_block( | ||
| f"{ICON_TO_UNICODE[Icon.INFO]} *Details*" | ||
| ), | ||
| self.message_builder.create_divider_block(), | ||
| *result, | ||
| ] | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Capitalize "Markdown" as a proper noun.
Static analysis correctly flags that "markdown" should be "Markdown" (lines 59 and 65).
Proposed fix
🧰 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