Add profiling activity to Android sample app #2192
Conversation
…background threads to get an idea of size of: profiling trace file, envelope item and compressed envelope item enabled profiling disabled auto user interaction instrumentation (creates concurrent transactions not currently supported by profiling)
…background threads to get an idea of size of: profiling trace file, envelope item and compressed envelope item enabled profiling disabled auto user interaction instrumentation (creates concurrent transactions not currently supported by profiling)
Codecov Report
@@ Coverage Diff @@
## feat/profiling-sample-rate #2192 +/- ##
================================================================
+ Coverage 80.88% 80.91% +0.02%
+ Complexity 3347 3343 -4
================================================================
Files 236 236
Lines 12207 12199 -8
Branches 1626 1623 -3
================================================================
- Hits 9874 9871 -3
+ Misses 1732 1729 -3
+ Partials 601 599 -2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| @@ -109,7 +113,7 @@ | |||
| <meta-data android:name="io.sentry.traces.activity.auto-finish.enable" android:value="false" /> | |||
|
|
|||
| <!-- how to enable the UI auto instrumentation for tracing --> | |||
| <meta-data android:name="io.sentry.traces.user-interaction.enable" android:value="true" /> | |||
| <!-- <meta-data android:name="io.sentry.traces.user-interaction.enable" android:value="true" />--> | |||
There was a problem hiding this comment.
Can you elaborate on:
Disabled auto user interaction instrumentation (creates concurrent transactions not currently supported by profiling)
I don't follow, there should be a single transaction in the scope, this is like this by design, so I'm not sure what you mean by concurrent transactions.
Also, disabling another feature isn't the best idea, either when testing this feature, we manually disable and enable it again (not comiting the changes).
We can also create a new flavor that enables and disables certain features.
We can also create a new sample app for profiling as suggested before as well if needed.
There was a problem hiding this comment.
The thing is that i added an activity where i click on a button and it manually starts a transaction to be profiled.
But when i click on the button an automatic transaction is started first, so that one is profiled. When the manual transaction is started, the automatic one hasn't finished, yet, so the profiler ignores the manual transaction, since it doesn't have the notion of scope.
We're currently working on allowing profiling to work in these cases.
We can reenable the user-interaction option, disabling it when testing the profiler.
I'm not sure a new sample app just for profiling is worth it, as we can find some edge cases we didn't consider when using other options of the sdk, too.
There was a problem hiding this comment.
Is it possible this uncovered a scenario we need to address? Should the old transaction be completed (and so profiling ended) when the new starts? Or should the new transaction not start because we already have one on going. We only create ui event transactions if none exists?
There was a problem hiding this comment.
We create it only if none exists, but by default it's automatically stopped after 3 seconds. This case is a transaction manually started in those 3 seconds.
we discussed this scenario, and we decided to make profiling work with multiple transactions here.
We should support it anyway, as this has less attrition for the user, and it's more important in the backend clients, where it's easy to have multiple concurrent transactions, possibly spawn over multiple threads.
There was a problem hiding this comment.
I'd say let's move ahead with this then and we can track this on an issue? Would you please open an issue?
There was a problem hiding this comment.
yeah, we already created a doc
Anyway, i just created this issue and i'll track it in the new pr i'll do
* added profilesSampleRate and profilesSampler options * deprecated SentryOptions.setProfilingEnabled() * updated internal code and unit tests to change from SentryOptions.setProfilingEnabled() to SentryOptions.setProfilesSampleRate() * Add profiling activity to Android sample app (#2192) * added profiling activity to the sample app, with possibility to set time and number of background threads to get an idea of size of: profiling trace file, envelope item and compressed envelope item * enabled profiling in the sample app Co-authored-by: Bruno Garcia <[email protected]>
📜 Description
Few additions to the Android sample app:
💡 Motivation and Context
There was no clear way to answer the "How big is a profile?" question.
Now we can have a way to know, depending on length and number of working threads, how big is the profiling trace file, how big is the created envelope item, and how big is the actual data sent to Sentry (the item compressed with gzip).
Also, we can now feed different profiles by running the sample app and editing the dsn.
#skip-changelog
💚 How did you test it?
No way 😄
📝 Checklist
🔮 Next steps