-
Notifications
You must be signed in to change notification settings - Fork 15
Deprecate all SafeTrace/SafeTraceFunctions APIs #83
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
base: main
Are you sure you want to change the base?
Changes from all commits
e0e00fc
f3ec685
9677855
21b7a35
b26a3f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,9 +4,7 @@ import android.content.pm.ApplicationInfo | |
| import android.os.Build | ||
| import papa.SafeTrace.MAX_LABEL_LENGTH | ||
| import papa.SafeTrace.beginSection | ||
| import papa.SafeTrace.isCurrentlyTracing | ||
| import papa.SafeTrace.isShellProfileable | ||
| import papa.internal.SafeTraceMainThreadMessages | ||
|
|
||
| /** | ||
| * This is a wrapper for [androidx.tracing.Trace] that should be used instead as [beginSection] and | ||
|
|
@@ -27,13 +25,6 @@ object SafeTrace { | |
| * This is true if the app manifest has the debuggable to true or if it includes the | ||
| * `<profileable android:shell="true"/>` on API 29+, which indicate an intention for this build | ||
| * to be a special build that you want to profile. | ||
| * | ||
| * You can force this to be true by calling [forceShellProfileable], which | ||
| * will enable app tracing even on non-shell-profileable builds. | ||
| * | ||
| * You can also trigger [forceShellProfileable] at runtime by sending a | ||
| * `papa.FORCE_SHELL_PROFILEABLE` broadcast. To support this you should add a | ||
| * dependency on the papa-dev artifact. | ||
| */ | ||
| @JvmStatic | ||
| val isShellProfileable: Boolean | ||
|
|
@@ -48,30 +39,18 @@ object SafeTrace { | |
| * Whether we are currently tracing, which determines whether calls to | ||
| * tracing functions will be forwarded to the Android tracing APIs. | ||
| */ | ||
| @Deprecated("Use androidx.tracing.Trace.isEnabled() instead", ReplaceWith("Trace.isEnabled()", "androidx.tracing.Trace")) | ||
| @JvmStatic | ||
| val isCurrentlyTracing: Boolean | ||
| get() = androidx.tracing.Trace.isEnabled() | ||
|
|
||
| @Deprecated("Use forceShellProfileable instead", ReplaceWith("forceShellProfileable()")) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was unused. Rather than keep a deprecated version of it, I think it makes more sense to remove it entirely. |
||
| @JvmStatic | ||
| fun forceTraceable() = forceShellProfileable() | ||
|
|
||
| /** | ||
| * @see isShellProfileable | ||
| */ | ||
| @JvmStatic | ||
| fun forceShellProfileable() { | ||
| androidx.tracing.Trace.forceEnableAppTracing() | ||
| _isTraceable = true | ||
| SafeTraceMainThreadMessages.enableMainThreadMessageTracing() | ||
| } | ||
|
|
||
| @Suppress("NOTHING_TO_INLINE") | ||
| private inline fun isShellProfileableInlined(): Boolean { | ||
| // Prior to SafeTraceSetup.initDone we can't determine if the app is traceable or not, so we | ||
| // always return false, unless something called forceTraceable(). The first call after | ||
| // SafeTraceSetup.initDone becomes true will compute the actual value based on debuggable | ||
| // and profileable manifest flags, then cache it so that we don't need to recheck again. | ||
| // always return false. The first call after SafeTraceSetup.initDone | ||
| // becomes true will compute the actual value based on debuggable and | ||
| // profileable manifest flags, then cache it so that we don't need to | ||
| // recheck again. | ||
| return _isTraceable | ||
| ?: if (SafeTraceSetup.initDone) { | ||
| val application = SafeTraceSetup.application | ||
|
|
@@ -93,17 +72,19 @@ object SafeTrace { | |
| * Writes a trace message to indicate that a given section of code has begun. This call must | ||
| * be followed by a corresponding call to {@link #endSection()} on the same thread. | ||
| */ | ||
| @Deprecated("Call androidx.tracing.Trace.beginSection instead", ReplaceWith("beginSection", "androidx.tracing.Trace.beginSection")) | ||
| @JvmStatic | ||
| fun beginSection(label: String) { | ||
| if (!isCurrentlyTracing) { | ||
| if (!androidx.tracing.Trace.isEnabled()) { | ||
| return | ||
| } | ||
| androidx.tracing.Trace.beginSection(label.take(MAX_LABEL_LENGTH)) | ||
| } | ||
|
|
||
| @Deprecated("Call androidx.tracing.Trace.beginSection instead", ReplaceWith("beginSection", "androidx.tracing.Trace.beginSection")) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| @JvmStatic | ||
| inline fun beginSection(crossinline labelLambda: () -> String) { | ||
| if (!isCurrentlyTracing) { | ||
| if (!androidx.tracing.Trace.isEnabled()) { | ||
| return | ||
| } | ||
| androidx.tracing.Trace.beginSection(labelLambda().take(MAX_LABEL_LENGTH)) | ||
|
|
@@ -113,8 +94,9 @@ object SafeTrace { | |
| * Begins and ends a section immediately. Useful for reporting information in the trace. | ||
| */ | ||
| @JvmStatic | ||
| @Deprecated("Call androidx.tracing.Trace.beginSection/endSection instead") | ||
| fun logSection(label: String) { | ||
| if (!isCurrentlyTracing) { | ||
| if (!androidx.tracing.Trace.isEnabled()) { | ||
| return | ||
| } | ||
| androidx.tracing.Trace.beginSection(label.take(MAX_LABEL_LENGTH)) | ||
|
|
@@ -124,9 +106,10 @@ object SafeTrace { | |
| /** | ||
| * @see [logSection] | ||
| */ | ||
| @Deprecated("Call androidx.tracing.Trace.beginSection/endSection instead") | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logSection doesn't exist in Jetpack, but we could provide an extension function |
||
| @JvmStatic | ||
| inline fun logSection(crossinline labelLambda: () -> String) { | ||
| if (!isCurrentlyTracing) { | ||
| if (!androidx.tracing.Trace.isEnabled()) { | ||
| return | ||
| } | ||
| val label = labelLambda().take(MAX_LABEL_LENGTH) | ||
|
|
@@ -141,9 +124,10 @@ object SafeTrace { | |
| * ensure that beginSection / endSection pairs are properly nested and called from the same | ||
| * thread. | ||
| */ | ||
| @Deprecated("Call androidx.tracing.Trace.endSection instead", ReplaceWith("endSection", "androidx.tracing.Trace.endSection")) | ||
| @JvmStatic | ||
| fun endSection() { | ||
| if (!isCurrentlyTracing) { | ||
| if (!androidx.tracing.Trace.isEnabled()) { | ||
| return | ||
| } | ||
| androidx.tracing.Trace.endSection() | ||
|
|
@@ -152,11 +136,12 @@ object SafeTrace { | |
| /** | ||
| * @see androidx.tracing.Trace.beginAsyncSection | ||
| */ | ||
| @Deprecated("Call androidx.tracing.Trace.beginAsyncSection instead", ReplaceWith("beginAsyncSection", "androidx.tracing.Trace.beginAsyncSection")) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| @JvmStatic | ||
| inline fun beginAsyncSection( | ||
| crossinline labelCookiePairLambda: () -> Pair<String, Int> | ||
| ) { | ||
| if (!isCurrentlyTracing) { | ||
| if (!androidx.tracing.Trace.isEnabled()) { | ||
| return | ||
| } | ||
| val (label, cookie) = labelCookiePairLambda() | ||
|
|
@@ -167,12 +152,13 @@ object SafeTrace { | |
| * [cookie] defaults to 0 (cookie is used for async traces that overlap) | ||
| * @see androidx.tracing.Trace.beginAsyncSection | ||
| */ | ||
| @Deprecated("Call androidx.tracing.Trace.beginAsyncSection instead", ReplaceWith("beginAsyncSection", "androidx.tracing.Trace.beginAsyncSection")) | ||
| @JvmStatic | ||
| fun beginAsyncSection( | ||
| label: String, | ||
| cookie: Int = 0 | ||
| ) { | ||
| if (!isCurrentlyTracing) { | ||
| if (!androidx.tracing.Trace.isEnabled()) { | ||
| return | ||
| } | ||
| androidx.tracing.Trace.beginAsyncSection(label, cookie) | ||
|
|
@@ -182,12 +168,13 @@ object SafeTrace { | |
| * [cookie] defaults to 0 (cookie is used for async traces that overlap) | ||
| * @see androidx.tracing.Trace.endAsyncSection | ||
| */ | ||
| @Deprecated("Call androidx.tracing.Trace.endAsyncSection instead", ReplaceWith("endAsyncSection", "androidx.tracing.Trace.endAsyncSection")) | ||
| @JvmStatic | ||
| fun endAsyncSection( | ||
| label: String, | ||
| cookie: Int = 0 | ||
| ) { | ||
| if (!isCurrentlyTracing) { | ||
| if (!androidx.tracing.Trace.isEnabled()) { | ||
| return | ||
| } | ||
| androidx.tracing.Trace.endAsyncSection(label, cookie) | ||
|
|
@@ -196,11 +183,12 @@ object SafeTrace { | |
| /** | ||
| * @see androidx.tracing.Trace.beginAsyncSection | ||
| */ | ||
| @Deprecated("Call androidx.tracing.Trace.endAsyncSection instead", ReplaceWith("endAsyncSection", "androidx.tracing.Trace.endAsyncSection")) | ||
| @JvmStatic | ||
| inline fun endAsyncSection( | ||
| crossinline labelCookiePairLambda: () -> Pair<String, Int> | ||
| ) { | ||
| if (!isCurrentlyTracing) { | ||
| if (!androidx.tracing.Trace.isEnabled()) { | ||
| return | ||
| } | ||
| val (label, cookie) = labelCookiePairLambda() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,10 @@ object SafeTraceSetup { | |
|
|
||
| fun init(application: Application) { | ||
| this.application = application | ||
| enableMainThreadMessageTracing() | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we still need the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I need it until I remove all callers of |
||
|
|
||
| fun enableMainThreadMessageTracing() { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we're deprecating |
||
| SafeTraceMainThreadMessages.enableMainThreadMessageTracing() | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we probably ought to make a public API for this, but ideally something cohesive not a random function of a SafeTrace object that's not about safe traces anymore
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about I move SafeTraceMainThreadMessages out of the internal package and make it public? |
||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| package papa | ||
|
|
||
| import androidx.tracing.Trace | ||
| import papa.InteractionUpdated.CanceledOnEvent | ||
| import papa.InteractionUpdated.CanceledOnRuleRemoved | ||
| import papa.InteractionUpdated.CanceledOnTimeout | ||
|
|
@@ -119,18 +120,16 @@ private class InteractionEngine<ParentEventType : Any>( | |
| * called with an unknown [Runnable]. | ||
| */ | ||
| private val cancelOnTimeout: Runnable = Runnable { | ||
| SafeTrace.logSection { | ||
| "PAPA-cancel:timeout" | ||
| } | ||
| Trace.beginSection("PAPA-cancel:timeout") | ||
| Trace.endSection() | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not use the lambda version with or at least bring back |
||
| stopRunning() | ||
| trace.endTrace() | ||
| updateListener.onInteractionUpdate(CanceledOnTimeout(cancelTimeout, this)) | ||
| } | ||
|
|
||
| fun cancelOnRuleRemoved() { | ||
| SafeTrace.logSection { | ||
| "PAPA-cancel:ruleRemoved" | ||
| } | ||
| Trace.beginSection("PAPA-cancel:ruleRemoved") | ||
| Trace.endSection() | ||
| stopRunning() | ||
| trace.endTrace() | ||
| updateListener.onInteractionUpdate(CanceledOnRuleRemoved(this)) | ||
|
|
@@ -153,19 +152,17 @@ private class InteractionEngine<ParentEventType : Any>( | |
|
|
||
| override fun cancel(reason: String) { | ||
| val sentEvent = eventInScope!! | ||
| SafeTrace.logSection { | ||
| "PAPA-cancel:${sentEvent.event}:$reason" | ||
| } | ||
| Trace.beginSection("PAPA-cancel:${sentEvent.event}:$reason") | ||
| Trace.endSection() | ||
| stopRunning() | ||
| trace.endTrace() | ||
| updateListener.onInteractionUpdate(CanceledOnEvent(sentEvent, this, reason)) | ||
| } | ||
|
|
||
| override fun finish(): FinishingInteraction<ParentEventType> { | ||
| val sentEvent = eventInScope!! | ||
| SafeTrace.logSection { | ||
| "PAPA-finishInteraction:${sentEvent.event}" | ||
| } | ||
| Trace.beginSection("PAPA-finishInteraction:${sentEvent.event}") | ||
| Trace.endSection() | ||
| stopRunning() | ||
| finishingInteractions += this | ||
| addRecordedEvent() | ||
|
|
@@ -188,9 +185,8 @@ private class InteractionEngine<ParentEventType : Any>( | |
|
|
||
| override fun recordEvent() { | ||
| val sentEvent = eventInScope!! | ||
| SafeTrace.logSection { | ||
| "PAPA-recordEvent:${sentEvent.event}" | ||
| } | ||
| Trace.beginSection("PAPA-recordEvent:${sentEvent.event}") | ||
| Trace.endSection() | ||
| addRecordedEvent() | ||
| updateListener.onInteractionUpdate(EventRecorded(sentEvent, this)) | ||
| } | ||
|
|
@@ -230,9 +226,8 @@ private class InteractionEngine<ParentEventType : Any>( | |
| trace: InteractionTrace, | ||
| cancelTimeout: Duration | ||
| ): RunningInteraction<ParentEventType> { | ||
| SafeTrace.logSection { | ||
| "PAPA-startInteraction:${sentEvent.event}" | ||
| } | ||
| Trace.beginSection("PAPA-startInteraction:${sentEvent.event}") | ||
| Trace.endSection() | ||
| val runningInteraction = RealRunningInteraction( | ||
| interactionTrigger = trigger, | ||
| trace = trace, | ||
|
|
||
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.
These are unused.