Skip to content
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

chore: add execution_id to any available report logs #23114

Closed
Closed
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
37 changes: 25 additions & 12 deletions superset/reports/commands/execute.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,11 @@
from superset.utils.celery import session_scope
from superset.utils.core import HeaderDataType, override_user
from superset.utils.csv import get_chart_csv_data, get_chart_dataframe
from superset.utils.screenshots import ChartScreenshot, DashboardScreenshot
from superset.utils.screenshots import (
ChartScreenshot,
DashboardScreenshot,
ScreenshotDetails,
)
from superset.utils.urls import get_url_path

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -204,19 +208,23 @@ def _get_screenshots(self) -> List[bytes]:
)
user = security_manager.find_user(username)
if self._report_schedule.chart:
screenshot: Union[ChartScreenshot, DashboardScreenshot] = ChartScreenshot(
url,
self._report_schedule.chart.digest,
chart_options = ScreenshotDetails(
execution_id=self._execution_id,
window_size=app.config["WEBDRIVER_WINDOW"]["slice"],
thumb_size=app.config["WEBDRIVER_WINDOW"]["slice"],
)
screenshot: Union[ChartScreenshot, DashboardScreenshot] = ChartScreenshot(
url, self._report_schedule.chart.digest, chart_options
)
else:
screenshot = DashboardScreenshot(
url,
self._report_schedule.dashboard.digest,
dashboard_options = ScreenshotDetails(
execution_id=self._execution_id,
window_size=app.config["WEBDRIVER_WINDOW"]["dashboard"],
thumb_size=app.config["WEBDRIVER_WINDOW"]["dashboard"],
)
screenshot = DashboardScreenshot(
url, self._report_schedule.dashboard.digest, dashboard_options
)
try:
image = screenshot.get_screenshot(user=user)
except SoftTimeLimitExceeded as ex:
Expand Down Expand Up @@ -324,6 +332,7 @@ def _get_log_data(self) -> HeaderDataType:
"chart_id": chart_id,
"dashboard_id": dashboard_id,
"owners": self._report_schedule.owners,
"execution_id": self._execution_id,
}
return log_data

Expand Down Expand Up @@ -453,7 +462,9 @@ def send_error(self, name: str, message: str) -> None:
self._execution_id,
)
notification_content = NotificationContent(
name=name, text=message, header_data=header_data
name=name,
text=message,
header_data=header_data,
)

# filter recipients to recipients who are also owners
Expand Down Expand Up @@ -708,9 +719,11 @@ def run(self) -> None:
user = security_manager.find_user(username)
with override_user(user):
logger.info(
"Running report schedule %s as user %s",
self._execution_id,
"Running report schedule as user %s",
username,
extra={
"execution_id": self._execution_id,
},
)
ReportScheduleStateMachine(
session, self._execution_id, self._model, self._scheduled_dttm
Expand All @@ -725,9 +738,9 @@ def validate( # pylint: disable=arguments-differ
) -> None:
# Validate/populate model exists
logger.info(
"session is validated: id %s, executionid: %s",
"session is validated: id %s",
self._model_id,
self._execution_id,
extra={"execution_id": self._execution_id},
)
self._model = (
session.query(ReportSchedule).filter_by(id=self._model_id).one_or_none()
Expand Down
4 changes: 3 additions & 1 deletion superset/reports/notifications/email.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ def _get_to(self) -> str:
def send(self) -> None:
subject = self._get_subject()
content = self._get_content()
execution_id = content.header_data and content.header_data.get("execution_id")
to = self._get_to()
try:
send_email_smtp(
Expand All @@ -203,7 +204,8 @@ def send(self) -> None:
header_data=content.header_data,
)
logger.info(
"Report sent to email, notification content is %s", content.header_data
"Report sent to email",
extra={"execution_id": execution_id},
)
except SupersetErrorsException as ex:
raise NotificationError(
Expand Down
5 changes: 4 additions & 1 deletion superset/reports/notifications/slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,10 @@ def send(self) -> None:
)
else:
client.chat_postMessage(channel=channel, text=body)
logger.info("Report sent to slack")
logger.info(
"Report sent to slack",
extra={"execution_id": self._content.header_data["execution_id"]},
)
except (
BotUserAccessError,
SlackRequestError,
Expand Down
17 changes: 14 additions & 3 deletions superset/tasks/thumbnails.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,11 @@
from superset.extensions import celery_app
from superset.tasks.utils import get_executor
from superset.utils.core import override_user
from superset.utils.screenshots import ChartScreenshot, DashboardScreenshot
from superset.utils.screenshots import (
ChartScreenshot,
DashboardScreenshot,
ScreenshotDetails,
)
from superset.utils.urls import get_url_path
from superset.utils.webdriver import WindowSize

Expand All @@ -47,6 +51,8 @@ def cache_chart_thumbnail(
if not thumbnail_cache:
logger.warning("No cache set, refusing to compute")
return None

task_id = None
chart = cast(Slice, Slice.get(chart_id))
url = get_url_path("Superset.slice", slice_id=chart.id)
logger.info("Caching chart: %s", url)
Expand All @@ -57,7 +63,8 @@ def cache_chart_thumbnail(
)
user = security_manager.find_user(username)
with override_user(user):
screenshot = ChartScreenshot(url, chart.digest)
options = ScreenshotDetails(execution_id=task_id)
screenshot = ChartScreenshot(url, chart.digest, options)
screenshot.compute_and_cache(
user=user,
cache=thumbnail_cache,
Expand All @@ -81,6 +88,8 @@ def cache_dashboard_thumbnail(
if not thumbnail_cache:
logging.warning("No cache set, refusing to compute")
return

task_id = None
dashboard = Dashboard.get(dashboard_id)
url = get_url_path("Superset.dashboard", dashboard_id_or_slug=dashboard.id)

Expand All @@ -91,8 +100,10 @@ def cache_dashboard_thumbnail(
current_user=current_user,
)
user = security_manager.find_user(username)
task_id = cache_dashboard_thumbnail.request.id
with override_user(user):
screenshot = DashboardScreenshot(url, dashboard.digest)
options = ScreenshotDetails(execution_id=task_id)
screenshot = DashboardScreenshot(url, dashboard.digest, options)
screenshot.compute_and_cache(
user=user,
cache=thumbnail_cache,
Expand Down
1 change: 1 addition & 0 deletions superset/utils/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ class HeaderDataType(TypedDict):
notification_source: Optional[str]
chart_id: Optional[int]
dashboard_id: Optional[int]
execution_id: uuid.UUID


class DatasourceDict(TypedDict):
Expand Down
52 changes: 30 additions & 22 deletions superset/utils/screenshots.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@

import logging
from io import BytesIO
from typing import Optional, TYPE_CHECKING, Union
from typing import Optional, TYPE_CHECKING, TypedDict, Union
from uuid import UUID

from flask import current_app

Expand All @@ -43,17 +44,24 @@
from flask_caching import Cache


class ScreenshotDetails(TypedDict, total=False):
execution_id: Optional[UUID]
window_size: Optional[WindowSize]
thumb_size: Optional[WindowSize]


class BaseScreenshot:
driver_type = current_app.config["WEBDRIVER_TYPE"]
thumbnail_type: str = ""
element: str = ""
window_size: WindowSize = (800, 600)
thumb_size: WindowSize = (400, 300)

def __init__(self, url: str, digest: str):
def __init__(self, url: str, digest: str, execution_id: Optional[UUID]):
self.digest: str = digest
self.url = url
self.screenshot: Optional[bytes] = None
self.execution_id = execution_id

def driver(self, window_size: Optional[WindowSize] = None) -> WebDriverProxy:
window_size = window_size or self.window_size
Expand All @@ -76,10 +84,17 @@ def cache_key(
return md5_sha_from_dict(args)

def get_screenshot(
self, user: User, window_size: Optional[WindowSize] = None
self,
user: User,
window_size: Optional[WindowSize] = None,
) -> Optional[bytes]:
driver = self.driver(window_size)
self.screenshot = driver.get_screenshot(self.url, self.element, user)
self.screenshot = driver.get_screenshot(
url=self.url,
element_name=self.element,
user=user,
execution_id=self.execution_id,
)
return self.screenshot

def get(
Expand Down Expand Up @@ -205,40 +220,33 @@ class ChartScreenshot(BaseScreenshot):
element: str = "chart-container"

def __init__(
self,
url: str,
digest: str,
window_size: Optional[WindowSize] = None,
thumb_size: Optional[WindowSize] = None,
self, url: str, digest: str, options: Optional[ScreenshotDetails] = None
):
# Chart reports are in standalone="true" mode
# Chart reports and thumbnails are in standalone="true" mode
url = modify_url_query(
url,
standalone=ChartStandaloneMode.HIDE_NAV.value,
)
super().__init__(url, digest)
self.window_size = window_size or (800, 600)
self.thumb_size = thumb_size or (800, 600)
options = options or {}
super().__init__(url, digest, options.get("execution_id"))
self.window_size = options.get("window_size") or (800, 600)
self.thumb_size = options.get("thumb_size") or (800, 600)


class DashboardScreenshot(BaseScreenshot):
thumbnail_type: str = "dashboard"
element: str = "standalone"

def __init__(
self,
url: str,
digest: str,
window_size: Optional[WindowSize] = None,
thumb_size: Optional[WindowSize] = None,
self, url: str, digest: str, options: Optional[ScreenshotDetails] = None
):
# per the element above, dashboard screenshots
# should always capture in standalone
url = modify_url_query(
url,
standalone=DashboardStandaloneMode.REPORT.value,
)

super().__init__(url, digest)
self.window_size = window_size or (1600, 1200)
self.thumb_size = thumb_size or (800, 600)
options = options or {}
super().__init__(url, digest, options.get("execution_id"))
self.window_size = options.get("window_size") or (1600, 1200)
self.thumb_size = options.get("thumb_size") or (800, 600)
9 changes: 7 additions & 2 deletions superset/utils/webdriver.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from enum import Enum
from time import sleep
from typing import Any, Dict, List, Optional, Tuple, TYPE_CHECKING
from uuid import UUID

from flask import current_app
from selenium.common.exceptions import (
Expand Down Expand Up @@ -165,7 +166,11 @@ def destroy(driver: WebDriver, tries: int = 2) -> None:
pass

def get_screenshot(
self, url: str, element_name: str, user: User
self,
url: str,
element_name: str,
user: User,
execution_id: Optional[UUID] = None,
) -> Optional[bytes]:
driver = self.auth(user)
driver.set_window_size(*self._window)
Expand All @@ -174,7 +179,6 @@ def get_screenshot(
selenium_headstart = current_app.config["SCREENSHOT_SELENIUM_HEADSTART"]
logger.debug("Sleeping for %i seconds", selenium_headstart)
sleep(selenium_headstart)

try:
logger.debug("Wait for the presence of %s", element_name)
element = WebDriverWait(driver, self._screenshot_locate_wait).until(
Expand Down Expand Up @@ -202,6 +206,7 @@ def get_screenshot(
"Taking a PNG screenshot of url %s as user %s",
url,
user.username,
extra={"execution_id": execution_id},
)

if current_app.config["SCREENSHOT_REPLACE_UNEXPECTED_ERRORS"]:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,19 +53,25 @@ def test_report_for_dashboard_with_tabs(
name="test report tabbed dashboard",
) as report_schedule:
dashboard: Dashboard = report_schedule.dashboard
execution_id = uuid4()
AsyncExecuteReportScheduleCommand(
str(uuid4()), report_schedule.id, datetime.utcnow()
str(execution_id), report_schedule.id, datetime.utcnow()
).run()
dashboard_state = report_schedule.extra.get("dashboard", {})
permalink_key = CreateDashboardPermalinkCommand(
dashboard.id, dashboard_state
).run()

assert dashboard_screenshot_mock.call_count == 1
(url, digest) = dashboard_screenshot_mock.call_args.args
(url, digest, options) = dashboard_screenshot_mock.call_args.args
assert url.endswith(f"/superset/dashboard/p/{permalink_key}/")
assert digest == dashboard.digest
assert send_email_smtp_mock.call_count == 1
assert options == {
"execution_id": execution_id,
"thumb_size": (1400, 2000),
"window_size": (1400, 2000),
}
assert len(send_email_smtp_mock.call_args.kwargs["images"]) == 1


Expand All @@ -92,22 +98,28 @@ def test_report_with_header_data(
name="test report tabbed dashboard",
) as report_schedule:
dashboard: Dashboard = report_schedule.dashboard
execution_id = uuid4()
AsyncExecuteReportScheduleCommand(
str(uuid4()), report_schedule.id, datetime.utcnow()
str(execution_id), report_schedule.id, datetime.utcnow()
).run()
dashboard_state = report_schedule.extra.get("dashboard", {})
permalink_key = CreateDashboardPermalinkCommand(
dashboard.id, dashboard_state
).run()

assert dashboard_screenshot_mock.call_count == 1
(url, digest) = dashboard_screenshot_mock.call_args.args
(url, digest, options) = dashboard_screenshot_mock.call_args.args
assert url.endswith(f"/superset/dashboard/p/{permalink_key}/")
assert digest == dashboard.digest
assert options == {
"execution_id": execution_id,
"thumb_size": (1400, 2000),
"window_size": (1400, 2000),
}
assert send_email_smtp_mock.call_count == 1
header_data = send_email_smtp_mock.call_args.kwargs["header_data"]
assert header_data.get("dashboard_id") == dashboard.id
assert header_data.get("notification_format") == report_schedule.report_format
assert header_data.get("notification_source") == ReportSourceFormat.DASHBOARD
assert header_data.get("notification_type") == report_schedule.type
assert len(send_email_smtp_mock.call_args.kwargs["header_data"]) == 6
assert len(send_email_smtp_mock.call_args.kwargs["header_data"]) == 7
Loading
Loading