Add documentation for propagators and how they are executed #1312
Conversation
alshopov
requested review from
Groxx,
shijiesheng,
agautam478,
jakobht,
3vilhamster,
dkrotx,
taylanisikdemir and
demirkayaender
as code owners
|
|
| @@ -36,8 +36,25 @@ type HeaderReader interface { | |||
| ForEachKey(handler func(string, []byte) error) error | |||
| } | |||
|
|
|||
| // ContextPropagator is an interface that determines what information from | |||
| // context to pass along | |||
| // ContextPropagator determines what information from context to pass along. | |||
There was a problem hiding this comment.
thanks for filling in this documentation gap. @shijiesheng can you review please?
internal/headers.go
Outdated
| // ContextPropagator is an interface that determines what information from | ||
| // context to pass along | ||
| // ContextPropagator determines what information from context to pass along. | ||
| // Returning an error from any of its methods will prevent the normal execution |
There was a problem hiding this comment.
This is actually not true. At least this Inject method will fail open.
There was a problem hiding this comment.
@alshopov Could you update the doc here? Or should we handle it on our side?
Clarify behavior on return of error Document the whole execution sequence Docs added to both internal package and exposed type alias due to: golang/go#44905 Signed-off-by: Alexander Shopov <[email protected]>
|
@shijiesheng, @3vilhamster: I have updated the docs. I have verified the behaviour via fiddling with: Note that I am documenting current behavior - I am not sure whether it is the intended behavior. I have no idea whether the current silent non-failure on errors from Inject* methods is a good idea - leaving this up to you. Sorry for the delay - free time is a rare premium |
| // -> ExtractToWorkflow -> Start executing a workflow -> InjectFromWorkflow -> | ||
| // Cadence -> Go Activity Worker -> Extract -> Execute Activity |
There was a problem hiding this comment.
nit: this might make it easier for users to understand
| // -> ExtractToWorkflow -> Start executing a workflow -> InjectFromWorkflow -> | |
| // Cadence -> Go Activity Worker -> Extract -> Execute Activity | |
| // -> ExtractToWorkflow -> Start executing a workflow -> Schedule an activity -> InjectFromWorkflow -> | |
| // Cadence -> Go Activity Worker -> Extract -> Execute Activity |
There was a problem hiding this comment.
I think that it is out of the scope of the context propagator, so we might add it if it causes confusion.
| // The whole sequence of execution is: | ||
| // | ||
| // Process initiating the workflow -> Inject -> Cadence -> Go Workflow Worker | ||
| // -> ExtractToWorkflow -> Start executing a workflow -> InjectFromWorkflow -> |
Codecov Report
Additional details and impacted files
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Clarify behavior on return of error
Document the whole execution sequence
Docs added to both internal package and exposed type alias due to: golang/go#44905