temporalio / sdk-go

Temporal Go SDK
https://docs.temporal.io/application-development?lang=go
MIT License
544 stars 215 forks source link

Auto propagate context value to activity #1691

Closed afifurrohman-id closed 1 month ago

afifurrohman-id commented 1 month ago

Is your feature request related to a problem? Please describe. we really need this feature since trace, logs, request data etc.. in our app is passing through context, change this behaviour is really error prone and breaking changes, because some context key are dynamic based on requests.

Describe the solution you'd like auto propagate each value in parent std lib context.

Describe alternatives you've considered none

Additional context none

Quinn-With-Two-Ns commented 1 month ago

Can you please clarify what the feature request here? What do you mean by "auto propagate each value in parent std lib context."?

afifurrohman-id commented 1 month ago

Can you please clarify what the feature request here? What do you mean by "auto propagate each value in parent std lib context."?

i mean:

func main() {
  ctx := context.WithValue(context.Background(), "key", "value")
  c, _ := client.Dial(client.Options{})

  c.ExecuteWorkflow(ctx, client.StartWorkflowOptions{TaskQueue: "FOO"}, workflow)

}
func workflow(ctx workflow.Context) error {

  workflow.ExecuteActivity(ctx, activity)
}

func activity(ctx context.Context) error {
 v := ctx.Value("key") // should return "value"
}
Quinn-With-Two-Ns commented 1 month ago

Context proportion is supported, please have a look at our samples https://github.com/temporalio/samples-go/tree/main/ctxpropagation and accompanying docs https://docs.temporal.io/develop/go/observability#tracing-and-context-propogation

afifurrohman-id commented 1 month ago

Context proportion is supported, please have a look at our samples https://github.com/temporalio/samples-go/tree/main/ctxpropagation and accompanying docs https://docs.temporal.io/develop/go/observability#tracing-and-context-propogation

yes, but still we need manually defined key and type of value.

but, in our usecase we have dynamic key based on request and we can't change the app behaviour to not use context.

why not automatically have each context.Value() when invoke activity? does the workflow use context.Context too right?

Quinn-With-Two-Ns commented 1 month ago

Yes you would need to manually define each key and make sure the value is serializable. Defining each key is Go language requirement there is nothing the Temporal SDK can do about that. Go's context package chose not to expose a way to iterate through all values.

workflows do not use context.Context, they use workflow.Context

Marking as closed as there is nothing more in the SDK we can add.