-
Notifications
You must be signed in to change notification settings - Fork 76
RUM-12923: Attach RUM information on profiling event #3017
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
RUM-12923: Attach RUM information on profiling event #3017
Conversation
3f944ee to
40ef8eb
Compare
|
application id + view id |
40ef8eb to
915ca04
Compare
e0a7ca3 to
965c1b2
Compare
|
🎯 Code Coverage 🔗 Commit SHA: 729c7ff | Docs | Datadog PR Page | Was this helpful? Give us feedback! |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## feature/perfetto-profiling #3017 +/- ##
==============================================================
- Coverage 71.25% 71.21% -0.04%
==============================================================
Files 874 875 +1
Lines 31768 31810 +42
Branches 5321 5326 +5
==============================================================
+ Hits 22636 22653 +17
- Misses 7626 7644 +18
- Partials 1506 1513 +7
🚀 New features to boost your workflow:
|
dd-sdk-android-internal/src/main/java/com/datadog/android/rum/TTIDEvent.kt
Outdated
Show resolved
Hide resolved
dd-sdk-android-internal/src/main/java/com/datadog/android/rum/TTIDEvent.kt
Show resolved
Hide resolved
...k-android-profiling/src/main/java/com/datadog/android/profiling/internal/ProfilingFeature.kt
Outdated
Show resolved
Hide resolved
...rc/main/kotlin/com/datadog/android/rum/internal/domain/scope/RumVitalAppLaunchEventHelper.kt
Outdated
Show resolved
Hide resolved
965c1b2 to
01719ec
Compare
...um/src/main/kotlin/com/datadog/android/rum/internal/startup/RumSessionScopeStartupManager.kt
Outdated
Show resolved
Hide resolved
4c1b0ec to
58d9a31
Compare
...um/src/main/kotlin/com/datadog/android/rum/internal/startup/RumSessionScopeStartupManager.kt
Show resolved
Hide resolved
...nternal/src/testFixtures/kotlin/com/datadog/android/internal/tests/elmyr/TTIDEventFactory.kt
Outdated
Show resolved
Hide resolved
| override fun write( | ||
| profilingResult: PerfettoResult | ||
| profilingResult: PerfettoResult, | ||
| ttidEvent: TTIDEvent? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now, if we don't have TTIDEvent, we shouldn't even write profile and upload it.
I guess the goal is to have only profiles with RUM context associated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in previous dogfooding, all the profiles > 1min are probably the profiles without correct TTID signal, if we stop sending them, we won't be able to see these kinds of abnormal profiles in next dogfooding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all the profiles > 1min are probably the profiles without correct TTID signal
I don't think it is a problem of TTID signal, but rather a problem of Perfetto callback result delayed.
But we can keep it if you suggest it is valuable to have such, we just need to remove that before the GA.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes I agree, my point is that we need to keep it for exposing the perfetto callback issue
...k-android-profiling/src/main/java/com/datadog/android/profiling/internal/ProfilingFeature.kt
Show resolved
Hide resolved
...um/src/main/kotlin/com/datadog/android/rum/internal/startup/RumSessionScopeStartupManager.kt
Outdated
Show resolved
Hide resolved
beec57e
58d9a31 to
beec57e
Compare
beec57e to
2af8c71
Compare
2af8c71 to
729c7ff
Compare
|
/merge |
|
View all feedbacks in Devflow UI.
This pull request is not mergeable according to GitHub. Common reasons include pending required checks, missing approvals, or merge conflicts — but it could also be blocked by other repository rules or settings.
The expected merge time in
This pull request was merged directly. |
What does this PR do?
This PR attaches following RUM context information in Profiling event so that it can be shown as tags in profiling page:
Also it changes the timing of RUM feature send TTID event to Profiling feature in order to stop the recording, so that the profiling event can attach the real first view event instead of artificial "AppLaunch" view event.
Motivation
RUM-12923
Demo
Review checklist (to be filled by reviewers)