add cpu start/end timestamps to concurrent profiling transactions#2235
Conversation
…oads added ui test to check times are different
...n-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt
Outdated
Show resolved
Hide resolved
| public fun getUnknown ()Ljava/util/Map; | ||
| public fun hashCode ()I | ||
| public fun notifyFinish (Ljava/lang/Long;Ljava/lang/Long;)V | ||
| public fun notifyFinish (Ljava/lang/Long;Ljava/lang/Long;Ljava/lang/Long;Ljava/lang/Long;)V |
There was a problem hiding this comment.
Technically a breaking change, should we annotate this method or type saying it's internal?
There was a problem hiding this comment.
yeah, but the class is already marked with @ApiStatus.Internal
so it shouldn't be exposed to the public. Also, it exists only in this pr and the other, it's not on master
…oads added ui test to check times are different
sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java
Show resolved
Hide resolved
|
@philipphofmann @bruno-garcia may i consider this pr reviewed and merge it? |
philipphofmann
left a comment
philipphofmann
left a comment
There was a problem hiding this comment.
One important comment about the hashCode to address, please.
...n-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt
Outdated
Show resolved
Hide resolved
sorted transaction list in ui test for better checks
sorted transaction list in ui test for better checks
…me' into feat/concurrent-profiling-cpu-time
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## feat/concurrent-profiling-data #2235 +/- ##
=================================================================
Coverage ? 80.33%
Complexity ? 3366
=================================================================
Files ? 241
Lines ? 12436
Branches ? 1657
=================================================================
Hits ? 9990
Misses ? 1821
Partials ? 625 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
5 participants
📜 Description
add cpu start/end timestamps to transactions in profile envelope payloads
added ui test to check times are different
#skip-changelog
💡 Motivation and Context
Profiling traces have a clock field to indicate how timestamps were captured in the trace itself and it can be dual/cpu/wall/global time.
If the timestamps are cpu time, there's no way to filter the trace based on start/end timestamps, as the payload only sends wall time.
💚 How did you test it?
I added a ui test to check correctness of cpu timestamps
📝 Checklist
🔮 Next steps