Skip to content

feat(size): Log per-insight timing in Apple analyzer - #665

Merged
NicoHinderling merged 1 commit into
mainfrom
feat/insight-timing-logs
Sep 2, 2026
Merged

feat(size): Log per-insight timing in Apple analyzer#665
NicoHinderling merged 1 commit into
mainfrom
feat/insight-timing-logs

Conversation

@NicoHinderling

@NicoHinderling NicoHinderling commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Insight generation logged nothing per-insight. On a large anonymized iOS app that timed out, size.apple.generate_insights was the last log before silence — a slow insight was indistinguishable from a hang.

This logs each insight's duration on completion: size.apple.insight_completed with insight and elapsed_s.

It's observability-only, so there's no material speedup and none measured (460.7s vs 439.8s, within noise). What it buys is attribution: it immediately isolated the culprit — image_optimization was 129.06s of 129.3s insights (99.8%), while the other 11 summed to ~0.27s.

Tested with a unit test asserting the completion log fires with the insight name + elapsed_s, plus make check.

Insight generation emitted no progress logging, so a slow insight was
invisible in traces (it looked like a silent hang). Log each insight's
wall-clock duration on completion (size.apple.insight_completed with the
insight name and elapsed_s) so the expensive phase is attributable.
@sentry

sentry Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

📲 Install Builds

iOS

🔗 App Name App ID Version Configuration
HackerNews com.emergetools.hackernews 3.8 (1) Release

Android

🔗 App Name App ID Version Configuration
Hacker News com.emergetools.hackernews 1.0.2 (13) Release

⚙️ launchpad-test-ios Build Distribution Settings

@sentry

sentry Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Size Analysis

2 components analyzed

iOS Builds

Name Configuration Version Download Size Install Size
HackerNews (iOS)
com.emergetools.hackernews
Release 3.8 (1) 6.5 MB (0 B) 9.7 MB (0 B)

Android Builds

Name Configuration Version Download Size Uncompressed Size
Hacker News (Android)
com.emergetools.hackernews
Release 1.0.2 (13) 2.9 MB (-12 B) 6.6 MB (0 B)

Configure launchpad-test-ios status check rules

with sentry_sdk.start_span(op="insight", description=f"apple.insights.{insight_name}"):
return insight_class().generate(insights_input)
started = time.monotonic()
result = insight_class().generate(insights_input)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Am fine if we want to keep this for logging purposes but I'm a bit confused why we need it when we have span information right above? Can't we just look at the span timings in the trace to figure out the same thing?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

for a run that completes, the span indeed covers it. The log is for the timeout case: when a task is killed at the deadline mid-insights, the transaction is truncated and the running insight's span never closes, so the trace can't tell you which insight was grinding.

The logs flush per-insight as each finishes, leaving a durable trail up to the kill, which was missing in the case of the customer's build.. so it seems nice to add just in case - at least as long as timeouts still occasionally happen in prod

@NicoHinderling
NicoHinderling merged commit a9926ec into main Sep 2, 2026
26 checks passed
@NicoHinderling
NicoHinderling deleted the feat/insight-timing-logs branch September 2, 2026 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants