Skip to content

Commit 9f67611

Browse files
authored
fix(subscriptions): cap png export height to prevent OOM (#40754)
1 parent 157fd86 commit 9f67611

File tree

5 files changed

+46
-12
lines changed

5 files changed

+46
-12
lines changed

ee/tasks/subscriptions/subscription_utils.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@
2323

2424
UTM_TAGS_BASE = "utm_source=posthog&utm_campaign=subscription_report"
2525
DEFAULT_MAX_ASSET_COUNT = 6
26+
# Maximum height for screenshots in pixels. This prevents Chrome from consuming excessive memory
27+
# when rendering very tall pages (e.g., tables with thousands of rows).
28+
MAX_SCREENSHOT_HEIGHT_PIXELS = 5000
2629

2730

2831
def _get_failed_asset_info(assets: list[ExportedAsset], resource: Union[Subscription, SharingConfiguration]) -> dict:
@@ -187,7 +190,9 @@ async def export_single_asset(asset: ExportedAsset) -> None:
187190
subscription_id=getattr(resource, "id", None),
188191
team_id=resource.team_id,
189192
)
190-
await database_sync_to_async(exporter.export_asset_direct, thread_sensitive=False)(asset)
193+
await database_sync_to_async(exporter.export_asset_direct, thread_sensitive=False)(
194+
asset, max_height_pixels=MAX_SCREENSHOT_HEIGHT_PIXELS
195+
)
191196
logger.info(
192197
"generate_assets_async.asset_exported",
193198
asset_id=asset.id,

ee/tasks/test/subscriptions/test_generate_assets_async.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ async def test_async_foreign_key_access_with_real_subscription(team, user, dashb
221221

222222
@patch("ee.tasks.subscriptions.subscription_utils.exporter.export_asset_direct")
223223
async def test_async_generate_assets_basic(mock_export: MagicMock, team, user) -> None:
224-
def export_success(asset: ExportedAsset) -> None:
224+
def export_success(asset: ExportedAsset, **kwargs) -> None:
225225
asset.content = b"fake image data"
226226
asset.save()
227227

@@ -283,7 +283,7 @@ async def test_async_generate_assets_timeout_continues_with_partial_results(
283283
async def test_async_generate_assets_partial_success(mock_export: MagicMock, team, user) -> None:
284284
call_count = 0
285285

286-
def export_with_partial_success(asset: ExportedAsset) -> None:
286+
def export_with_partial_success(asset: ExportedAsset, **kwargs) -> None:
287287
nonlocal call_count
288288
call_count += 1
289289
if call_count <= 2:

posthog/api/test/test_exports.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ def test_can_create_new_valid_export_insight(self, mock_exporter_task, mock_expo
230230

231231
# Should warm up the cache
232232
export_image(exported_asset)
233-
mock_export_to_png.assert_called_once_with(exported_asset)
233+
mock_export_to_png.assert_called_once_with(exported_asset, max_height_pixels=None)
234234

235235
mock_process_query_dict.assert_called_once()
236236

posthog/tasks/exporter.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,17 +62,25 @@
6262
max_retries=3,
6363
)
6464
@transaction.atomic
65-
def export_asset(exported_asset_id: int, limit: Optional[int] = None) -> None:
65+
def export_asset(
66+
exported_asset_id: int,
67+
limit: Optional[int] = None, # For CSV/XLSX: max row count
68+
max_height_pixels: Optional[int] = None, # For images: max screenshot height in pixels
69+
) -> None:
6670
# if Celery is lagging then you can end up with an exported asset that has had a TTL added
6771
# and that TTL has passed, in the exporter we don't care about that.
6872
# the TTL is for later cleanup.
6973
exported_asset: ExportedAsset = ExportedAsset.objects_including_ttl_deleted.select_related(
7074
"created_by", "team", "team__organization"
7175
).get(pk=exported_asset_id)
72-
export_asset_direct(exported_asset, limit)
76+
export_asset_direct(exported_asset, limit=limit, max_height_pixels=max_height_pixels)
7377

7478

75-
def export_asset_direct(exported_asset: ExportedAsset, limit: Optional[int] = None) -> None:
79+
def export_asset_direct(
80+
exported_asset: ExportedAsset,
81+
limit: Optional[int] = None, # For CSV/XLSX: max row count
82+
max_height_pixels: Optional[int] = None, # For images: max screenshot height in pixels
83+
) -> None:
7684
from posthog.tasks.exports import csv_exporter, image_exporter
7785

7886
start_time = perf_counter()
@@ -107,7 +115,7 @@ def export_asset_direct(exported_asset: ExportedAsset, limit: Optional[int] = No
107115
csv_exporter.export_tabular(exported_asset, limit=limit)
108116
EXPORT_QUEUED_COUNTER.labels(type="csv").inc()
109117
else:
110-
image_exporter.export_image(exported_asset)
118+
image_exporter.export_image(exported_asset, max_height_pixels=max_height_pixels)
111119
EXPORT_QUEUED_COUNTER.labels(type="image").inc()
112120

113121
logger.info(

posthog/tasks/exports/image_exporter.py

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ def get_driver() -> webdriver.Chrome:
7474
)
7575

7676

77-
def _export_to_png(exported_asset: ExportedAsset) -> None:
77+
def _export_to_png(exported_asset: ExportedAsset, max_height_pixels: Optional[int] = None) -> None:
7878
"""
7979
Exporting an Insight means:
8080
1. Loading the Insight from the web app in a dedicated rendering mode
@@ -150,7 +150,9 @@ def _export_to_png(exported_asset: ExportedAsset) -> None:
150150

151151
logger.info("exporting_asset", asset_id=exported_asset.id, render_url=url_to_render)
152152

153-
_screenshot_asset(image_path, url_to_render, screenshot_width, wait_for_css_selector, screenshot_height)
153+
_screenshot_asset(
154+
image_path, url_to_render, screenshot_width, wait_for_css_selector, screenshot_height, max_height_pixels
155+
)
154156

155157
with open(image_path, "rb") as image_file:
156158
image_data = image_file.read()
@@ -181,6 +183,7 @@ def _screenshot_asset(
181183
screenshot_width: ScreenWidth,
182184
wait_for_css_selector: CSSSelector,
183185
screenshot_height: int = 600,
186+
max_height_pixels: Optional[int] = None,
184187
) -> None:
185188
driver: Optional[webdriver.Chrome] = None
186189
try:
@@ -238,6 +241,15 @@ def _screenshot_asset(
238241
"""
239242
)
240243

244+
if max_height_pixels and height > max_height_pixels:
245+
logger.warning(
246+
"screenshot_height_capped",
247+
original_height=height,
248+
capped_height=max_height_pixels,
249+
url=url_to_render,
250+
)
251+
height = max_height_pixels
252+
241253
# For example funnels use a table that can get very wide, so try to get its width
242254
# For replay players, check for player width
243255
width = driver.execute_script(
@@ -280,6 +292,15 @@ def _screenshot_asset(
280292
"""
281293
)
282294

295+
if max_height_pixels and final_height > max_height_pixels:
296+
logger.warning(
297+
"screenshot_final_height_capped",
298+
original_final_height=final_height,
299+
capped_height=max_height_pixels,
300+
url=url_to_render,
301+
)
302+
final_height = max_height_pixels
303+
283304
# Set final window size
284305
driver.set_window_size(width, final_height + HEIGHT_OFFSET)
285306
driver.save_screenshot(image_path)
@@ -302,7 +323,7 @@ def _screenshot_asset(
302323
driver.quit()
303324

304325

305-
def export_image(exported_asset: ExportedAsset) -> None:
326+
def export_image(exported_asset: ExportedAsset, max_height_pixels: Optional[int] = None) -> None:
306327
with posthoganalytics.new_context():
307328
posthoganalytics.tag("team_id", exported_asset.team_id if exported_asset else "unknown")
308329
posthoganalytics.tag("asset_id", exported_asset.id if exported_asset else "unknown")
@@ -324,7 +345,7 @@ def export_image(exported_asset: ExportedAsset) -> None:
324345

325346
if exported_asset.export_format == "image/png":
326347
with EXPORT_TIMER.labels(type="image").time():
327-
_export_to_png(exported_asset)
348+
_export_to_png(exported_asset, max_height_pixels=max_height_pixels)
328349
EXPORT_SUCCEEDED_COUNTER.labels(type="image").inc()
329350
else:
330351
raise NotImplementedError(

0 commit comments

Comments
 (0)