feat: Add reportAppFullyDisplayed to RUM for app launch time to full display (TTFD) - #1124
feat: Add reportAppFullyDisplayed to RUM for app launch time to full display (TTFD)#1124brunovsiqueira wants to merge 3 commits into
Conversation
…display (TTFD) Both native SDKs expose reportAppFullyDisplayed -- RUMMonitorProtocol on iOS and RumMonitor on Android -- but it was never bridged to Dart, so rum.measure.app.startup_to_full_display could not populate for Flutter apps at all. Adds the method through the platform interface, the method channel and both native plugins, with a no-op on Web since the Browser SDK has no equivalent. refs: DataDog#944 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fuzzybinary
left a comment
fuzzybinary
left a comment
There was a problem hiding this comment.
Looks good, just one very minor request - if we're going to add TTFD to the test scenario we should verify it sent the value.
| Future<void> reportAppFullyDisplayed() { | ||
| return methodChannel.invokeMethod( | ||
| 'reportAppFullyDisplayed', | ||
| <String, Object?>{}, |
There was a problem hiding this comment.
nit: We tend to add the type annotations on these maps, especially if they're empty.
There was a problem hiding this comment.
The map is already <String, Object?>{}, same as stopSession below. Did you mean invokeMethod<void>?
There was a problem hiding this comment.
Sorry, I missed a "not" in there. We tend to not add the type annotations on these maps. It slipped in on stopSession, and not a big deal so I'm not going to worry about it.
App launch vitals and operation step vitals share the `vital` event type but carry different properties, so the decoder now splits them into separate buckets instead of decoding every vital as an operation step. Without this the TTFD vital was counted as a fourth operation step in the manual RUM scenario. The scenario test now asserts that a single `ttfd` app launch vital is reported with a plausible duration. refs: DataDog#944 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| Future<void> reportAppFullyDisplayed() { | ||
| return methodChannel.invokeMethod( | ||
| 'reportAppFullyDisplayed', | ||
| <String, Object?>{}, |
There was a problem hiding this comment.
Sorry, I missed a "not" in there. We tend to not add the type annotations on these maps. It slipped in on stopSession, and not a big deal so I'm not going to worry about it.
| final ttfdVitals = rumLog | ||
| .where((e) => | ||
| e.eventType == 'vital' && | ||
| RumVitalAppLaunchEventDecoder.isAppLaunchVital(e.rumEvent)) | ||
| .map((e) => RumVitalAppLaunchEventDecoder(e.rumEvent)) | ||
| .where((e) => e.appLaunchMetric == 'ttfd') | ||
| .toList(); |
There was a problem hiding this comment.
I'll likely refactor this to have all vitals accessible on the session but for now this is fine.
| expect(ttfdVitals[0].duration, | ||
| lessThan(const Duration(seconds: 60).inNanoseconds)); |
There was a problem hiding this comment.
We'll see if this is enough for CI or if it flakes 😅.
This comment has been minimized.
This comment has been minimized.
|
@brunovsiqueira You need to run |
The Android SDK only sends the TTFD app launch vital once it has computed TTID for the startup scenario. That does not happen in the integration test app -- it sends no app launch vitals at all, only operation step vitals -- so there is nothing to assert on there yet and android-integration-test was failing. refs: DataDog#944 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| // This is only checked on iOS. The Browser SDK has no equivalent API, and | ||
| // the Android SDK only sends TTFD once it has computed TTID for the startup | ||
| // scenario, which does not happen in this app -- it sends no app launch | ||
| // vitals at all, so there is nothing to assert on there yet. | ||
| if (!kIsWeb && Platform.isIOS) { |
There was a problem hiding this comment.
If that's the case, we may want to hold off on this change. I know we're in the process of looking into TTID for Android. Let me look into the state of it and I'll get back to you.
2 participants
What and why?
Bridges
reportAppFullyDisplayed()from the native SDKs to Dart.Both native SDKs have shipped this API for a while —
RUMMonitorProtocol.reportAppFullyDisplayed()on iOS andRumMonitor.reportAppFullyDisplayed()on Android — but it was never exposed through the Flutter plugin.addViewLoadingTimewas bridged in #655; the app-launch equivalent was not. Without this method there is no way for a Flutter app to mark the moment its UI is fully displayed, sorum.measure.app.startup_to_full_displaynever populates for Flutter apps at all.#944 is the standing request for TTFD support on Flutter.
How?
Purely additive: one new no-argument method threaded through the existing layers —
DatadogRum→DdRumPlatform→ method channel →DatadogRumPluginon iOS and Android. Web is a no-op, as the Browser SDK has no equivalent (same treatment asaddViewLoadingTime).No existing behaviour changes.
addViewLoadingTimeis deliberately untouched — that is per-view loading time, a different metric.Review checklist
Tests mirror the
addViewLoadingTimeones: a method-channel test inddrum_method_channel_test.dart, plugin tests plus contract entries on bothDatadogRumPluginTest.ktandDatadogRumPluginTests.swift, and the call is exercised in the manual RUM integration scenario. The existing native mocks already implementedreportAppFullyDisplayed, so they needed no changes.🤖 Generated with Claude Code