diff --git a/src/robusta/core/reporting/blocks.py b/src/robusta/core/reporting/blocks.py index 9e6c7cf6a..9c5b941ee 100644 --- a/src/robusta/core/reporting/blocks.py +++ b/src/robusta/core/reporting/blocks.py @@ -38,6 +38,9 @@ def tabulate(*args, **kwargs): BLOCK_SIZE_LIMIT = 2997 # due to slack block size limit of 3000 +# Don't shrink a text column below this, or its values become unreadable. +TABLE_MIN_COLUMN_WIDTH = 4 + class MarkdownBlock(BaseBlock): """ @@ -364,25 +367,53 @@ def __init__( if table_format: self.metadata["format"] = TableBlockFormat.vertical.value + @classmethod + def __is_number(cls, value) -> bool: + try: + float(str(value)) + return True + except (TypeError, ValueError): + return False + + @classmethod + def __numeric_column_indices(cls, rendered_rows, num_columns: int) -> set: + # Numeric columns (e.g. counters) are kept at full width. + numeric_indices = set() + for idx in range(num_columns): + non_empty = [str(row[idx]) for row in rendered_rows if idx < len(row) and str(row[idx]).strip()] + if non_empty and all(cls.__is_number(v) for v in non_empty): + numeric_indices.add(idx) + return numeric_indices + @classmethod def __calc_max_width(cls, headers, rendered_rows, table_max_width: int) -> List[int]: - # We need to make sure the total table width, doesn't exceed the max width, - # otherwise, the table is printed corrupted - columns_max_widths = [len(header) for header in headers] + # Keep the total table width within the max, otherwise it renders corrupted. + num_columns = max(len(headers), max((len(row) for row in rendered_rows), default=0)) + columns_max_widths = [0] * num_columns + for idx, header in enumerate(headers): + columns_max_widths[idx] = len(str(header)) for row in rendered_rows: for idx, val in enumerate(row): columns_max_widths[idx] = max(len(str(val)), columns_max_widths[idx]) - if sum(columns_max_widths) > table_max_width: # We want to limit the widest column - largest_width = max(columns_max_widths) - widest_column_idx = columns_max_widths.index(largest_width) - diff = sum(columns_max_widths) - table_max_width - columns_max_widths[widest_column_idx] = largest_width - diff - if columns_max_widths[widest_column_idx] < 0: # in case the diff is bigger than the largest column - # just divide equally - columns_max_widths = [ - int(table_max_width / len(columns_max_widths)) for i in range(0, len(columns_max_widths)) - ] + if sum(columns_max_widths) <= table_max_width: + return columns_max_widths + + # Only shrink text columns, so numeric columns never get wrapped. If every + # column is numeric, leave the widths as-is rather than wrapping numbers. + numeric_indices = cls.__numeric_column_indices(rendered_rows, num_columns) + shrinkable = [idx for idx in range(num_columns) if idx not in numeric_indices] + if not shrinkable: + return columns_max_widths + + # Water-fill: trim the widest shrinkable column by one char at a time (fair + # distribution), never below TABLE_MIN_COLUMN_WIDTH. + while sum(columns_max_widths) > table_max_width: + candidates = [idx for idx in shrinkable if columns_max_widths[idx] > TABLE_MIN_COLUMN_WIDTH] + if not candidates: + break + widest = max(candidates, key=lambda idx: columns_max_widths[idx]) + columns_max_widths[widest] -= 1 return columns_max_widths @@ -410,25 +441,88 @@ def __trim_rows(cls, contents: str, max_chars: int): return "\n".join(lines[:lines_to_include]) + truncator + @staticmethod + def default_omission_note(omitted_rows: List[List[str]]) -> str: + return f"... {len(omitted_rows)} more rows not shown" + + def __render_within_budget(self, max_chars: int, table_max_width: int, table_fmt: str, omission_note) -> str: + """Render the table, dropping whole rows to fit max_chars. + + Values themselves are never cut - long ones wrap onto extra lines - so rows must be + dropped as complete units. Cutting by physical line would leave a wrapped value's + continuation behind and show a partial value as if it were the whole thing. + """ + rows = self.__to_strings_rows(self.render_rows()) + # Column widths are computed once from all rows, so dropping rows never reflows + # the columns and the rendered size shrinks monotonically with the row count. + col_max_width = self.__calc_max_width(self.headers, rows, table_max_width) + + def render(subset: List[List[str]]) -> str: + if not subset: # tabulate raises IndexError on maxcolwidths with no rows + return tabulate(subset, headers=self.headers, tablefmt=table_fmt) + return tabulate(subset, headers=self.headers, tablefmt=table_fmt, maxcolwidths=col_max_width) + + full = render(rows) + if len(full) <= max_chars: + return full + + # Largest number of whole rows that fits alongside the "N more rows" note. + best = None + low, high = 0, len(rows) + while low <= high: + mid = (low + high) // 2 + candidate = render(rows[:mid]) + if mid < len(rows): + candidate = f"{candidate}\n{omission_note(rows[mid:])}" + if len(candidate) <= max_chars: + best = candidate + low = mid + 1 + else: + high = mid - 1 + + # Fall back to the line-based trim if not even a header-only table fits. + return best if best is not None else self.__trim_rows(full, max_chars) + @classmethod def __to_strings_rows(cls, rows): # This is just to assert all row column values are strings. Tabulate might fail on other types return [list(map(lambda column_value: str(column_value), row)) for row in rows] - def to_markdown(self, max_chars=None, add_table_header: bool = True) -> MarkdownBlock: - table_header = f"{self.table_name}\n" if self.table_name else "" - table_header = "" if not add_table_header else table_header - prefix = f"{table_header}```\n" - suffix = "\n```" - table_contents = self.to_table_string() - if max_chars is not None: - max_chars = max_chars - len(prefix) - len(suffix) - table_contents = self.__trim_rows(table_contents, max_chars) + def to_markdown(self, max_chars=None, add_table_header: bool = True, omission_note=None) -> MarkdownBlock: + """:param omission_note: builds the note shown when rows are dropped to fit the size limit. + Receives the dropped rows, so callers can summarise what was left out (e.g. residual counts).""" + # Stay under BLOCK_SIZE_LIMIT even when no caller-supplied limit: MarkdownBlock + # would otherwise blindly cut the text and lose the closing ``` fence, which + # makes the whole table render as unformatted (non-monospace) text. + budget = BLOCK_SIZE_LIMIT - 1 if max_chars is None else min(max_chars, BLOCK_SIZE_LIMIT - 1) + + table_header = f"{self.table_name}\n" if self.table_name and add_table_header else "" + opening_fence, suffix = "```\n", "\n```" + # The table name is caller-supplied and can be arbitrarily long, so trim it rather than + # let the envelope alone push the block past the limit and cost us the closing fence. + table_header = table_header[: max(0, budget - len(opening_fence) - len(suffix))] + prefix = f"{table_header}{opening_fence}" + + content_budget = budget - len(prefix) - len(suffix) + # With no room left for content, emit an empty (but properly closed) block rather than + # an overflow marker that wouldn't fit either. + table_contents = ( + self.__render_within_budget( + content_budget, + PRINTED_TABLE_MAX_WIDTH, + "presto", + omission_note or self.default_omission_note, + ) + if content_budget > 0 + else "" + ) return MarkdownBlock(f"{prefix}{table_contents}{suffix}") def to_table_string(self, table_max_width: int = PRINTED_TABLE_MAX_WIDTH, table_fmt: str = "presto") -> str: rendered_rows = self.__to_strings_rows(self.render_rows()) + if not rendered_rows: # tabulate raises IndexError on maxcolwidths with no rows + return tabulate(rendered_rows, headers=self.headers, tablefmt=table_fmt) col_max_width = self.__calc_max_width(self.headers, rendered_rows, table_max_width) return tabulate( rendered_rows, diff --git a/src/robusta/core/sinks/sink_base.py b/src/robusta/core/sinks/sink_base.py index 80fb6de66..c13d99e96 100644 --- a/src/robusta/core/sinks/sink_base.py +++ b/src/robusta/core/sinks/sink_base.py @@ -36,6 +36,11 @@ class NotificationSummary(BaseModel): start_ts: float = Field(default_factory=lambda: time.time()) # Timestamp of the first notification # Keys for the table are determined by grouping.notification_mode.summary.by summary_table: DefaultDict[KeyT, List[int]] = None + # The full summary table is attached as a file when it doesn't fit in the message. Files + # can't be edited in place, so the previous one is deleted whenever a new one is uploaded. + attachment_file_id: Optional[str] = None + attachment_permalink: Optional[str] = None + attachment_ts: float = 0 # when the attachment was last refreshed def register_notification(self, summary_key: KeyT, resolved: bool, interval: int): now_ts = time.time() @@ -45,6 +50,11 @@ def register_notification(self, summary_key: KeyT, resolved: bool, interval: int self.summary_table = defaultdict(lambda: [0, 0]) self.start_ts = now_ts self.message_id = None + # Drop the link so the new summary doesn't point at the previous interval's file, + # but keep the id so that whoever uploads next can still delete that file - the + # sink itself has no way to reach Slack. + self.attachment_permalink = None + self.attachment_ts = 0 self.summary_table[summary_key][idx] += 1 diff --git a/src/robusta/core/sinks/slack/slack_sink.py b/src/robusta/core/sinks/slack/slack_sink.py index afd2d9ab9..1411d2b76 100644 --- a/src/robusta/core/sinks/slack/slack_sink.py +++ b/src/robusta/core/sinks/slack/slack_sink.py @@ -1,4 +1,6 @@ +import json import logging +from urllib.parse import urlencode from robusta.core.model.env_vars import ROBUSTA_UI_DOMAIN from robusta.core.reporting.base import Finding, FindingStatus @@ -89,6 +91,7 @@ def handle_notification_grouping(self, finding: Finding, platform_enabled: bool) investigate_uri=investigate_uri, grouping_interval=self.params.grouping.interval, channel=resolved_channel, + summary_state=notification_summary, ) notification_summary.message_id = slack_thread_ts should_send_notification = self.params.grouping.notification_mode.summary.threaded @@ -104,7 +107,13 @@ def handle_notification_grouping(self, finding: Finding, platform_enabled: bool) ) def get_timeline_uri(self, account_id: str, cluster_name: str) -> str: - return f"{ROBUSTA_UI_DOMAIN}/graphs?account_id={account_id}&cluster={cluster_name}" + # The timeline reads its filters as JSON-encoded query params, and expects "clusters" + # (a list) rather than a single "cluster" - passing the latter left the link unfiltered. + params = { + "account_id": account_id, + "clusters": json.dumps([cluster_name]), + } + return f"{ROBUSTA_UI_DOMAIN}/graphs?{urlencode(params)}" def __replace_callback_with_string(self, slack_message, block_id, message_string): """ diff --git a/src/robusta/integrations/slack/sender.py b/src/robusta/integrations/slack/sender.py index 682fe7263..a5198d089 100644 --- a/src/robusta/integrations/slack/sender.py +++ b/src/robusta/integrations/slack/sender.py @@ -1,11 +1,12 @@ import copy import logging +import time import ssl import tempfile import re from datetime import datetime, timedelta from itertools import chain -from typing import Any, Dict, List, Optional, Set, Union +from typing import Any, Dict, List, Optional, Set, Tuple, Union import certifi import humanize from dateutil import tz @@ -56,6 +57,11 @@ ACTION_LINK = "link" SlackBlock = Dict[str, Any] MAX_BLOCK_CHARS = 3000 +# A file can hold a far wider table than a message can. +SUMMARY_ATTACHMENT_TABLE_WIDTH = 250 +# The summary message is rewritten on every notification; don't re-upload the attachment +# that often. +SUMMARY_ATTACHMENT_REFRESH_SECONDS = 60 MENTION_PATTERN = re.compile(r"<[^>]+>") @@ -860,6 +866,78 @@ def __send_finding_to_slack_preview( output_blocks=slack_blocks, ) + @staticmethod + def __summary_table_block(table_block: TableBlock) -> Tuple[BaseBlock, int]: + """Render the summary table, noting the totals of any rows dropped to fit the size limit. + + Without the residual counts the numbers in the table wouldn't add up to the notification + count in the message header, making the summary look wrong. + + Returns the block and the number of rows that didn't fit. + """ + if len(table_block.headers) <= 2: + return table_block, 0 # rendered as a bullet list rather than a table, see __to_slack_table + + omitted_count = 0 + + def omission_note(omitted_rows: List[List[str]]) -> str: + nonlocal omitted_count + omitted_count = len(omitted_rows) + fired = resolved = 0 + for row in omitted_rows: + try: + fired += int(row[-2]) + resolved += int(row[-1]) + except (IndexError, ValueError): # not the expected numeric columns - just count rows + return TableBlock.default_omission_note(omitted_rows) + return f"... {len(omitted_rows)} more groups ({fired} fired, {resolved} resolved) not shown" + + block = table_block.to_markdown(omission_note=omission_note) + return block, omitted_count + + def __refresh_summary_attachment(self, table_block: TableBlock, summary_state) -> Optional[str]: + """Attach the complete table as a file, since the message can only show part of it. + + Slack files are immutable, but the summary message is rewritten on every notification, so + a fresh file is uploaded (and the previous one removed) rather than edited. That is + throttled - re-uploading on every single notification would be wasteful. + """ + if summary_state is None: + return None + + now = time.time() + if summary_state.attachment_permalink and now - summary_state.attachment_ts < SUMMARY_ATTACHMENT_REFRESH_SECONDS: + return summary_state.attachment_permalink + + # A much wider table fits in a file than in a message, so nothing wraps there. + contents = table_block.to_table_string(table_max_width=SUMMARY_ATTACHMENT_TABLE_WIDTH) + file_block = FileBlock("alerts-summary.txt", contents.encode("utf-8")) + try: + resp = self.slack_client.files_upload_v2( + title=file_block.filename, + filename=file_block.filename, + content=file_block.contents, + ) + permalink = resp["file"]["permalink"] + new_file_id = resp["file"]["id"] + except Exception: + logging.exception("Failed to upload the full summary table to Slack") + return summary_state.attachment_permalink + + previous_file_id = summary_state.attachment_file_id + summary_state.attachment_file_id = new_file_id + summary_state.attachment_permalink = permalink + summary_state.attachment_ts = now + + if previous_file_id: + # Best effort - a leftover file is preferable to failing the summary update. + try: + self.slack_client.files_delete(file=previous_file_id) + except Exception as e: + logging.warning(f"Could not delete the superseded summary attachment: {e}") + + return permalink + def send_or_update_summary_message( self, group_by_classification_header: List[str], @@ -873,12 +951,23 @@ def send_or_update_summary_message( investigate_uri: str = None, grouping_interval: int = None, # in seconds channel: str = None, # pre-resolved channel (when channel_override uses labels/annotations) + summary_state=None, # NotificationSummary, for tracking the attachment across updates ): """Create or update a summary message with tabular information about the amount of events fired/resolved and a header describing the event group that this information concerns.""" rows = [] n_total_alerts = 0 - for key, value in sorted(summary_table.items()): + # Sort by firing count, then resolved count (both descending), so that when the table + # is too long to fit in a message the rows that get dropped are the least significant + # ones rather than whichever groups happen to sort last alphabetically. The key is only + # a final tie-break, to keep the order stable between updates of the same message. It is + # normalised to strings first: keys hold raw attribute values, which may mix None with + # strings (e.g. "workload" is None for a finding with no service) and aren't comparable. + def sort_key(item): + group_key, (fired, resolved) = item + return -fired, -resolved, tuple(str(value) for value in group_key) + + for key, value in sorted(summary_table.items(), key=sort_key): # key is a tuple of attribute names; value is a 2-element list with # the number of firing and resolved notifications. row = list(str(e) for e in chain(key, value)) @@ -924,21 +1013,17 @@ def send_or_update_summary_message( else: blocks.append(MarkdownBlock(text=cluster_txt)) + summary_block, omitted_count = self.__summary_table_block(table_block) blocks.extend( [ MarkdownBlock(f"*Matching criteria*: {group_by_criteria_str}"), MarkdownBlock(text=time_text), - table_block, + summary_block, ] ) - if threaded: - blocks.append(MarkdownBlock(text="See thread for individual alerts")) - - output_blocks = [] - for block in blocks: - output_blocks.extend(self.__to_slack(block, sink_params.name)) - + # Resolve the target before building the rest of the message, so that bailing out + # doesn't leave an uploaded attachment behind. if not channel: # Fallback: resolve channel without labels/annotations (only cluster_name will work) channel = ChannelTransformer.template( @@ -964,10 +1049,31 @@ def send_or_update_summary_message( method = self.slack_client.chat_postMessage kwargs = {} + attachment_permalink = None + if omitted_count: + # Too many groups to show inline - attach the complete table as a file. The link has + # to sit outside the table's code block, Slack doesn't render links inside one. + attachment_permalink = self.__refresh_summary_attachment(table_block, summary_state) + if attachment_permalink: + blocks.append(MarkdownBlock(text=f"<{attachment_permalink}|See all {len(rows)} groups> (attached)")) + + if threaded: + blocks.append(MarkdownBlock(text="See thread for individual alerts")) + + output_blocks = [] + for block in blocks: + output_blocks.extend(self.__to_slack(block, sink_params.name)) + + message_text = "Summary for: " + ", ".join(group_by_classification_header) + if attachment_permalink: + # Uploaded files have to be referenced from the message text, not just the blocks, + # for Slack to actually share them (see prepare_slack_text). + message_text = f"{message_text}\n<{attachment_permalink}|alerts-summary.txt>" + try: resp = method( channel=channel, - text="Summary for: " + ", ".join(group_by_classification_header), + text=message_text, blocks=output_blocks, display_as_bot=True, **kwargs, diff --git a/tests/test_blocks.py b/tests/test_blocks.py index 86fdc4517..106cc8562 100644 --- a/tests/test_blocks.py +++ b/tests/test_blocks.py @@ -1,4 +1,5 @@ import json +import re from hikaru.model.rel_1_26 import HikaruDocumentBase, ObjectMeta, Pod @@ -19,6 +20,7 @@ TableBlock, action, ) +from robusta.core.reporting.blocks import BLOCK_SIZE_LIMIT from robusta.core.reporting.consts import ScanType from robusta.core.sinks.slack.slack_sink_params import SlackSinkParams from tests.config import CONFIG @@ -134,3 +136,134 @@ def test_all_block_types(slack_channel: SlackChannel): result = slack_sender.send_finding_to_slack(finding, slack_params, False) # result = slack_sender.send_finding_to_slack(finding, slack_params, True) print(result) + + +# Regression tests for FRO-211 / ROB-3946: long values must stay complete (wrapped +# onto extra lines, never cut mid-value), and an over-long table must drop whole rows +# with a note rather than blow past Slack's block limit and lose its closing ``` fence. +LONG_CLASS_NAME = "ats.betting.betcatcher.settlement.settler.AbstractBetSettler" + + +def _column_values(output, column_index=0): + """Rebuild each logical row's column value from a wrapped presto table.""" + values, lines = [], output.splitlines() + for line in lines[2:]: # skip header + separator + if "|" not in line: + continue + cell = line.split("|")[column_index].strip() + starts_new_row = all(part.strip() for part in line.split("|")[1:]) + if starts_new_row: + values.append(cell) + elif values: + values[-1] += cell # continuation of the previous row's wrapped value + return values + + +def test_long_values_are_wrapped_not_cut(): + rows = [[LONG_CLASS_NAME, "103", "0"], ["orders.checkout.impl.OrderServiceImpl", "16", "4"]] + table_block = TableBlock(rows=rows, headers=["label:site", "Fired", "Resolved"]) + + output = table_block.to_table_string(table_max_width=40) + + # The full path survives, reassembled across the wrapped lines - nothing is elided. + assert _column_values(output) == [LONG_CLASS_NAME, "orders.checkout.impl.OrderServiceImpl"] + assert "…" not in output + + +def test_numeric_columns_are_not_shrunk(): + table_block = TableBlock( + rows=[[LONG_CLASS_NAME, "103", "9999"]], headers=["label:site", "Fired", "Resolved"] + ) + + output = table_block.to_table_string(table_max_width=40) + + # Counters stay on one line - only the wide text column absorbs the reduction. + assert "103" in output and "9999" in output + assert all(line.count("|") == 2 for line in output.splitlines() if "|" in line) + + +def test_all_numeric_table_is_not_shrunk(): + values = ["123456789012345", "678901234567890", "112233445566778"] + table_block = TableBlock(rows=[values], headers=["a", "b", "c"]) + + output = table_block.to_table_string(table_max_width=10) + + for value in values: + assert value in output + + +def test_headerless_and_ragged_rows(): + # No headers, and rows wider than the (empty) header list must not IndexError. + table_block = TableBlock(rows=[[LONG_CLASS_NAME, "extra", "cols"]], headers=[]) + + assert table_block.to_table_string(table_max_width=30) + + +def test_to_markdown_small_table_has_no_omission_note(): + table_block = TableBlock(rows=[[LONG_CLASS_NAME, "1", "0"]], headers=["label:site", "Fired", "Resolved"]) + + markdown = table_block.to_markdown().text + + assert "more rows not shown" not in markdown + assert markdown.startswith("```") and markdown.endswith("```") + + +def test_to_markdown_drops_whole_rows_and_keeps_code_fence(): + # The 4-column shape of the "Alerts Summary" digest that triggered the bug. + rows = [[f"ats.betting.betcatcher.validation.impl.Validator{i:03d}Foo", "nj", "1", "0"] for i in range(200)] + table_block = TableBlock(rows=rows, headers=["label:site", "label:component", "Fired", "Resolved"]) + + markdown = table_block.to_markdown().text + + # Small enough that MarkdownBlock never blind-cuts it, so the fence is intact. + assert len(markdown) < BLOCK_SIZE_LIMIT + assert markdown.startswith("```") and markdown.endswith("```") + # The reader is told what was left out. + assert re.search(r"\.\.\. \d+ more rows not shown", markdown) + + # Every displayed value is a complete class name - rows are dropped as whole units, + # so no row is left showing only the first half of a wrapped value. + inner = markdown.split("```")[1] + displayed = _column_values(inner) + assert displayed # some rows did survive + for value in displayed: + assert re.fullmatch(r"ats\.betting\.betcatcher\.validation\.impl\.Validator\d{3}Foo", value), value + + +def test_to_markdown_custom_omission_note_receives_dropped_rows(): + rows = [[f"ats.betting.betcatcher.validation.impl.Validator{i:03d}Foo", "nj", "1", "2"] for i in range(200)] + table_block = TableBlock(rows=rows, headers=["label:site", "label:component", "Fired", "Resolved"]) + + def omission_note(omitted): + return f"... {len(omitted)} more groups ({sum(int(r[-2]) for r in omitted)} fired) not shown" + + markdown = table_block.to_markdown(omission_note=omission_note).text + + # The note reports the residual totals, so the table still reconciles with the header count. + match = re.search(r"\.\.\. (\d+) more groups \((\d+) fired\) not shown", markdown) + assert match, markdown[-200:] + omitted_count, omitted_fired = int(match.group(1)), int(match.group(2)) + assert omitted_fired == omitted_count # one "fired" per dropped row + assert len(markdown) < BLOCK_SIZE_LIMIT + + +def test_empty_table_does_not_raise(): + # tabulate raises IndexError if maxcolwidths is passed with no rows, which also hit the + # row-dropping path when not even one row fits the size limit. + table_block = TableBlock(rows=[], headers=["label:site", "Fired", "Resolved"]) + + assert table_block.to_table_string() + markdown = table_block.to_markdown().text + assert markdown.startswith("```") and markdown.endswith("```") + + +def test_to_markdown_always_closes_code_fence(): + # Sweep sizes/widths that previously produced an unterminated fence (which made Slack + # render the whole table as non-monospace text) or a blind cut past the block limit. + for rows_n in (0, 1, 18, 200): + for width in (5, 80, 600): + rows = [[("v" * width) + str(i), "1", "0"] for i in range(rows_n)] + markdown = TableBlock(rows=rows, headers=["label:site", "Fired", "Resolved"]).to_markdown().text + assert markdown.count("```") % 2 == 0, (rows_n, width) + assert markdown.rstrip().endswith("```"), (rows_n, width) + assert len(markdown) < BLOCK_SIZE_LIMIT, (rows_n, width, len(markdown)) diff --git a/tests/test_slack_summary_attachment.py b/tests/test_slack_summary_attachment.py new file mode 100644 index 000000000..23e2d8817 --- /dev/null +++ b/tests/test_slack_summary_attachment.py @@ -0,0 +1,153 @@ +"""Tests for the file attachment on the Slack "Alerts Summary" digest (ROB-3946 / FRO-211). + +The digest can only show part of a large table, so the complete one is attached as a file. +Slack files are immutable while the message is rewritten on every notification, so these tests +pin down the upload/refresh/delete lifecycle. +""" +import time +from unittest.mock import MagicMock, patch + +import pytest + +from robusta.core.sinks.sink_base import NotificationSummary +from robusta.core.sinks.slack.slack_sink_params import SlackSinkParams + +SUMMARY_HEADER = ["label:site", "label:component"] +GROUP_HEADER = ["cluster: prod-nj1"] + + +@pytest.fixture +def slack(): + """A SlackSender wired to a mock Slack client, plus the recorded uploads/deletes.""" + with patch("robusta.integrations.slack.sender.WebClient") as web_client: + from robusta.integrations.slack.sender import SlackSender + + client = MagicMock() + uploads, deletes = [], [] + + def upload(**kwargs): + uploads.append(kwargs) + return {"file": {"permalink": f"https://files.slack.com/f{len(uploads)}", "id": f"F{len(uploads)}"}} + + client.files_upload_v2.side_effect = upload + client.files_delete.side_effect = lambda **kwargs: deletes.append(kwargs["file"]) + client.chat_postMessage.return_value = {"ts": "111.1", "channel": "C1"} + client.chat_update.return_value = {"ts": "111.1", "channel": "C1"} + web_client.return_value = client + + sender = SlackSender("xoxb-test", "account", "prod-nj1", "key", "chan", registry=None) + yield sender, client, uploads, deletes + + +def _table(n_groups): + return { + (f"ats.betting.betcatcher.validation.impl.Validator{i:03d}Foo", "nj"): [i % 7, i % 3] + for i in range(n_groups) + } + + +def _send(sender, summary_table, state, **kwargs): + return sender.send_or_update_summary_message( + GROUP_HEADER, + SUMMARY_HEADER, + summary_table, + SlackSinkParams(name="test", slack_channel="chan", api_key=""), + False, + time.time(), + False, + grouping_interval=86400, + channel="chan", + summary_state=state, + **kwargs, + ) + + +def test_no_attachment_when_the_table_fits(slack): + sender, client, uploads, _ = slack + + _send(sender, _table(1), NotificationSummary()) + + assert uploads == [] # nothing was dropped, so there is nothing to attach + assert "files.slack.com" not in str(client.chat_postMessage.call_args.kwargs["blocks"]) + + +def test_attaches_the_full_table_when_rows_are_dropped(slack): + sender, client, uploads, _ = slack + state = NotificationSummary() + + _send(sender, _table(200), state) + + assert len(uploads) == 1 + assert state.attachment_permalink and state.attachment_file_id + # Every group is in the file, even though the message only shows a fraction of them. + assert uploads[0]["content"].decode().count("Validator") == 200 + + kwargs = client.chat_postMessage.call_args.kwargs + assert any("files.slack.com" in str(block) for block in kwargs["blocks"]) + # Slack only shares an uploaded file if it is referenced from the message text too. + assert "files.slack.com" in kwargs["text"] + + +def test_attachment_is_throttled_across_updates(slack): + sender, _, uploads, _ = slack + state = NotificationSummary() + + _send(sender, _table(200), state) + sender.channel_name_to_id["chan"] = "C1" + _send(sender, _table(200), state, msg_ts="111.1") + + assert len(uploads) == 1 # the message is rewritten per notification, the file is not + + +def test_refreshing_the_attachment_deletes_the_previous_file(slack): + sender, _, uploads, deletes = slack + state = NotificationSummary() + + _send(sender, _table(200), state) + state.attachment_ts = 0 # pretend the throttle window elapsed + sender.channel_name_to_id["chan"] = "C1" + _send(sender, _table(200), state, msg_ts="111.1") + + assert len(uploads) == 2 + assert deletes == ["F1"] + + +def test_previous_interval_file_is_cleaned_up_after_reset(slack): + sender, _, uploads, deletes = slack + state = NotificationSummary() + _send(sender, _table(200), state) + + state.start_ts = time.time() - 999999 # force the interval to expire + state.register_notification(("x", "y"), False, 86400) + # The link is dropped so the new summary can't point at the old file, but the id is kept + # so the next upload can still delete it. + assert state.attachment_permalink is None + assert state.attachment_file_id == "F1" + + _send(sender, _table(200), state) + assert deletes == ["F1"] + + +def test_upload_failure_does_not_break_the_summary(slack): + sender, client, _, _ = slack + client.files_upload_v2.side_effect = Exception("no files:write scope") + + ts = _send(sender, _table(200), NotificationSummary()) + + assert ts == "111.1" # the digest is still posted, just without the attachment + assert "files.slack.com" not in str(client.chat_postMessage.call_args.kwargs["blocks"]) + + +def test_summary_keys_mixing_none_and_strings_do_not_crash(slack): + # Group keys hold raw attribute values - "workload" is None when a finding has no service - + # and None is not comparable with str, so sorting must normalise before comparing. + sender, client, _, _ = slack + summary_table = { + ("ats.wallet.DefaultFundsAdjuster", "nj"): [5, 2], + (None, "nj"): [5, 2], # same counts, so the tie-break has to compare the keys + } + + ts = _send(sender, summary_table, NotificationSummary()) + + assert ts == "111.1" + assert "None" in str(client.chat_postMessage.call_args.kwargs["blocks"])