Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/sentry/charts/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class ChartType(Enum):
SLACK_METRIC_DETECTOR_EVENTS = "slack:metricDetector.events"
SLACK_METRIC_DETECTOR_SESSIONS = "slack:metricDetector.sessions"
SLACK_TIMESERIES = "slack:timeseries"
SLACK_DASHBOARDS_WIDGET = "slack:dashboardsWidget"


class ChartSize(TypedDict):
Expand Down
13 changes: 8 additions & 5 deletions src/sentry/integrations/slack/unfurl/dashboards.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
from sentry import analytics, features
from sentry.api import client
from sentry.api.endpoints.timeseries import TimeSeries
from sentry.api.serializers import serialize
from sentry.api.serializers.models.dashboard import DashboardWidgetSerializer
from sentry.charts import backend as charts
from sentry.charts.types import ChartSize, ChartType
from sentry.integrations.messaging.metrics import (
Expand Down Expand Up @@ -156,9 +158,9 @@ def _unfurl_dashboards(
if not per_query_params:
continue

combined_time_series: list[TimeSeries] = []
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dictionary values in _TIMESERIES_DISPLAY_TYPES now unused

Low Severity

The display_type variable fetched from _TIMESERIES_DISPLAY_TYPES is only used for the is None guard check — its string value ("line", "area", "bar") is no longer consumed anywhere after the removal of chart_data["type"]. The dict now effectively functions as a set, making its values dead code. This could mislead future maintainers into thinking those values are meaningful or consumed downstream.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 4f7bef5. Configure here.

combined_time_series: list[tuple[TimeSeries, int]] = []
request_failed = False
for params in per_query_params:
for query_index, params in enumerate(per_query_params):
try:
resp = client.get(
auth=ApiKey(organization_id=org.id, scope_list=["org:read"]),
Expand All @@ -171,19 +173,20 @@ def _unfurl_dashboards(
request_failed = True
break

combined_time_series.extend(resp.data.get("timeSeries", []))
for ts in resp.data.get("timeSeries", []):
combined_time_series.append((ts, query_index))

if request_failed:
continue

chart_data: dict[str, Any] = {
"timeSeries": combined_time_series,
"type": display_type,
"widget": serialize(widget, user, DashboardWidgetSerializer()),
}

try:
url = charts.generate_chart(
ChartType.SLACK_TIMESERIES, chart_data, size=DASHBOARDS_CHART_SIZE
ChartType.SLACK_DASHBOARDS_WIDGET, chart_data, size=DASHBOARDS_CHART_SIZE
)
except RuntimeError:
_logger.warning("Failed to generate chart for dashboards unfurl")
Expand Down
135 changes: 129 additions & 6 deletions tests/sentry/integrations/slack/test_unfurl.py
Original file line number Diff line number Diff line change
Expand Up @@ -2579,10 +2579,14 @@ def test_unfurl_dashboards_spans_widget(
== SlackDiscoverMessageBuilder(title=widget.title, chart_url="chart-url").build()
)
assert mock_generate_chart.call_count == 1
assert mock_generate_chart.call_args[0][0] == ChartType.SLACK_TIMESERIES
assert mock_generate_chart.call_args[0][0] == ChartType.SLACK_DASHBOARDS_WIDGET
chart_data = mock_generate_chart.call_args[0][1]
assert chart_data["type"] == "line"
assert "timeSeries" in chart_data
assert chart_data["widget"]["title"] == "My Spans Widget"
assert chart_data["widget"]["widgetType"] == "spans"
assert chart_data["widget"]["displayType"] == "line"
assert chart_data["widget"]["queries"][0]["aggregates"] == ["avg(span.duration)"]
assert all(pair[1] == 0 for pair in chart_data["timeSeries"])
assert chart_data["timeSeries"][0][0]["yAxis"] == "avg(span.duration)"

api_params = mock_client_get.call_args[1]["params"]
assert "/events-timeseries/" in mock_client_get.call_args[1]["path"]
Expand Down Expand Up @@ -2774,8 +2778,127 @@ def test_unfurl_dashboards_multiple_queries_are_joined(
assert len(unfurls) == 1
assert mock_client_get.call_count == 2
chart_data = mock_generate_chart.call_args[0][1]
y_axes = [series["yAxis"] for series in chart_data["timeSeries"]]
assert y_axes == ["avg(span.duration)", "p75(span.duration)"]
pairs = chart_data["timeSeries"]
assert [pair[0]["yAxis"] for pair in pairs] == [
"avg(span.duration)",
"p75(span.duration)",
]
assert [pair[1] for pair in pairs] == [0, 1]

@patch("sentry.integrations.slack.unfurl.dashboards.client.get")
@patch("sentry.charts.backend.generate_chart", return_value="chart-url")
def test_unfurl_dashboards_multi_query_same_aggregate(
self, mock_generate_chart: MagicMock, mock_client_get: MagicMock
) -> None:
mock_client_get.side_effect = [
MagicMock(data=self._build_mock_timeseries_response(y_axis="count(span.duration)")),
MagicMock(data=self._build_mock_timeseries_response(y_axis="count(span.duration)")),
]

dashboard = self.create_dashboard(organization=self.organization)
widget = self.create_dashboard_widget(
dashboard=dashboard,
title="Multi query",
display_type=DashboardWidgetDisplayTypes.LINE_CHART,
widget_type=DashboardWidgetTypes.SPANS,
order=0,
)
self.create_dashboard_widget_query(
widget=widget,
order=0,
name="",
fields=["count(span.duration)"],
aggregates=["count(span.duration)"],
columns=[],
conditions="span.op:db",
)
self.create_dashboard_widget_query(
widget=widget,
order=1,
name="",
fields=["count(span.duration)"],
aggregates=["count(span.duration)"],
columns=[],
conditions="span.op:http.client",
)

url = (
f"https://sentry.io/organizations/{self.organization.slug}"
f"/dashboard/{dashboard.id}/widget/0/?statsPeriod=7d"
)
link_type, args = match_link(url)
assert link_type is not None and args is not None
links = [UnfurlableUrl(url=url, args=args)]

with self.feature(["organizations:dashboards-widget-unfurl"]):
link_handlers[link_type].fn(self.integration, links, self.user)

chart_data = mock_generate_chart.call_args[0][1]
pairs = chart_data["timeSeries"]
assert [pair[1] for pair in pairs] == [0, 1]
assert [pair[0]["yAxis"] for pair in pairs] == [
"count(span.duration)",
"count(span.duration)",
]
widget_payload = chart_data["widget"]
assert [q["conditions"] for q in widget_payload["queries"]] == [
"span.op:db",
"span.op:http.client",
]

@patch("sentry.integrations.slack.unfurl.dashboards.client.get")
@patch("sentry.charts.backend.generate_chart", return_value="chart-url")
def test_unfurl_dashboards_single_query_multi_series_share_query_index(
self, mock_generate_chart: MagicMock, mock_client_get: MagicMock
) -> None:
def grouped_response(group_value: str):
return {
"timeSeries": [
{
"yAxis": "count(span.duration)",
"groupBy": [{"key": "transaction", "value": group_value}],
"meta": {
"valueType": "duration",
"valueUnit": "millisecond",
"interval": INTERVAL_COUNT * 1000,
},
"values": [],
}
],
}

mock_client_get.return_value = MagicMock(
data={
"timeSeries": [
grouped_response("/api/db")["timeSeries"][0],
grouped_response("/api/http")["timeSeries"][0],
]
}
)

dashboard, _ = self._create_spans_widget(
aggregates=["count(span.duration)"],
columns=["transaction"],
)

url = (
f"https://sentry.io/organizations/{self.organization.slug}"
f"/dashboard/{dashboard.id}/widget/0/?statsPeriod=7d"
)
link_type, args = match_link(url)
assert link_type is not None and args is not None
links = [UnfurlableUrl(url=url, args=args)]

with self.feature(["organizations:dashboards-widget-unfurl"]):
link_handlers[link_type].fn(self.integration, links, self.user)

chart_data = mock_generate_chart.call_args[0][1]
pairs = chart_data["timeSeries"]
assert [pair[1] for pair in pairs] == [0, 0]
assert [pair[0]["groupBy"][0]["value"] for pair in pairs] == [
"/api/db",
"/api/http",
]

@patch("sentry.integrations.slack.unfurl.dashboards.client.get")
@patch("sentry.charts.backend.generate_chart", return_value="chart-url")
Expand All @@ -2800,7 +2923,7 @@ def test_unfurl_dashboards_bar_display_type(

assert len(unfurls) == 1
chart_data = mock_generate_chart.call_args[0][1]
assert chart_data["type"] == "bar"
assert chart_data["widget"]["displayType"] == "bar"

def test_match_link_dashboards(self) -> None:
# Primary domain
Expand Down
Loading