-
Notifications
You must be signed in to change notification settings - Fork 112
Add Dispatch Tracking #1425
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: sedwards/update-snapshot-timeout
Are you sure you want to change the base?
Add Dispatch Tracking #1425
Conversation
d17b4e0 to
ee8b977
Compare
ee8b977 to
9c18611
Compare
| public abstract fun getParent ()Lcom/squareup/workflow1/WorkflowInterceptor$WorkflowSession; | ||
| public abstract fun getRenderKey ()Ljava/lang/String; | ||
| public abstract fun getRuntimeConfig ()Ljava/util/Set; | ||
| public abstract fun getRuntimeContext ()Lkotlin/coroutines/CoroutineContext; |
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.
This is a breaking change for anyone implementing WorkflowInterceptor. I don't know if that's a concern....
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.
New method is fine, no?
| if (config.isEmpty()) { | ||
| append("Base, ") | ||
| } | ||
| append("Dispatch: ${runtimeDispatch?.toString()?.take(6)}") |
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.
This won't always work. e.g. see the documentation from https://github.com/Kotlin/kotlinx.coroutines/blob/5d4954b17ac18c572e22a9a6e7bb4e4368ec568c/kotlinx-coroutines-core/common/src/MainCoroutineDispatcher.kt#L45 :
/**
* Returns a name of this main dispatcher for debugging purposes. This implementation returns
* `Dispatchers.Main` or `Dispatchers.Main.immediate` if it is the same as the corresponding
* reference in [Dispatchers] or a short class-name representation with address otherwise.
*/
override fun toString(): String = toStringInternalImpl() ?: "$classSimpleName@$hexAddress"
The IO dispatcher - https://github.com/Kotlin/kotlinx.coroutines/blob/5d4954b17ac18c572e22a9a6e7bb4e4368ec568c/kotlinx-coroutines-core/native/src/Dispatchers.kt#L44 - is similar
Maybe you should drop the Dispatchers. from the beginning of the string if it exists?
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.
good idea.
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.
Changed it to
append("Dispatch: ${runtimeDispatch?.toString()?.substringAfter("Dispatchers.")?.take(8)}")
substringAfter returns original string by default if does not contain the substring.
9c18611 to
d6a2382
Compare
d6a2382 to
80b2d5d
Compare
Add the runtime's
CoroutineContextto theWorkflowSessioninfo and then also forward theCoroutineDispatcherthrough theConfigSnapshotthat is used by theWorkflowRuntimeTracer. This can help us get definitive logging about the dispatcher being used in the runtime.