Add Nexus context propagation sample #394
Conversation
| "google.golang.org/protobuf/encoding/protojson" | ||
| ) | ||
|
|
||
| type WorkerInterceptor struct { |
There was a problem hiding this comment.
I am doing this in Java right now and one thing I debated was just putting all the context propagation in the interceptor. Is is generally what we recommend to customers now
There was a problem hiding this comment.
Why do we recommend that? I wouldn't expect users to have to know every method that context might be propagated from. I think newer SDKs should implement a context propagation interceptor that could potentially include Nexus if we have a good way to ensure a consistent format (e.g. everything is a payload that can be encrypted).
There was a problem hiding this comment.
Interceptors are far more flexible and support far more use cases. Context propagators are very limited in scope, users often hit these limitations and have to rewrite using interceptors anyway. I would like to deprecate them in SDKs that have them and steer towards interceptors. SDKs could provide a context propagation interceptor helper like we do for tracing
There was a problem hiding this comment.
SDKs could provide a context propagation interceptor helper like we do for tracing
Definitely agree with this. While it's simple to implement one, you still need to know which contexts to propagate to and from and remember to update your interceptors for new APIs that require interception.
| ctx workflow.Context, | ||
| input interceptor.ExecuteNexusOperationInput, | ||
| ) workflow.NexusOperationFuture { | ||
| if values, ok := ctx.Value(ctxpropagation.PropagateKey).(ctxpropagation.Values); ok { |
There was a problem hiding this comment.
In Java I can just copy the headers from the context directly because MDC only supports strings, this is fine though for Go
|
You you could add a sync handler and show the context is propagated there as well. I understand that your code might implicitly show that, but it might not be obvious to users |
I was debating whether to do that or not. Let's merge since this isn't blocking and I think I should have time to do a quick follow up today. |
Also added the Nexus Cancelation sample to the README list.
I made some slight adjustments to the
ctxpropgationsince I reused the propagator in this new sample.