add saga interceptor

[ychensha]

What was changed

add worker interceptor for hosting saga workflow

Why?

saga sample has many duplicate error handle,
besides, every one will have to learn Saga pattern and then to read&copy code from others, it sucks.
Interceptor and local state is suitable for temporal to impl Saga pattern.

Checklist

1. Closes

2. How was this tested:

3. Any docs updates needed?

[cretz]

Still not completely convinced hiding this stuff behind an interceptor is the clearest way. Here's an alternative from a comment on the other PR at temporalio/sdk-go#936 (comment) that I didn't test (just wrote there in comment):

func testWorkflow(ctx workflow.Context, a int) error {
	var txn txn
	zap.L().Debug("enter workflow")
	ctx = workflow.WithActivityOptions(ctx, workflow.ActivityOptions{
		StartToCloseTimeout: time.Minute,
	})

	var id string
	if err := workflow.ExecuteActivity(ctx, "createOrder", a).Get(ctx, &id); err != nil {
		return err
	}
	zap.L().Debug("create order, id:", zap.String("id", id))
	defer txn.executeRollbackActivity(ctx, "deleteOrder", id)

	if err := workflow.ExecuteActivity(ctx, "stockDeduct", a).Get(ctx, nil); err != nil {
		return err
	}
	defer txn.executeRollbackActivity(ctx, "stockInc", a)

	if err := workflow.ExecuteActivity(ctx, "createPay", a).Get(ctx, nil); err != nil {
		return err
	}
	txn.markSuccessful()
	return nil
}

type txn bool

func (t *txn) markSuccessful() { *s = true }

func (t *txn) executeRollbackActivity(ctx workflow.Context, activity interface{}, args ...interface{}) {
	if !*txn {
		ctx, cancel := workflow.NewDisconnectedContext(ctx)
		defer cancel()
		if err := workflow.ExecuteActivity(ctx, activity, args...).Get(ctx); err != nil {
			workflow.GetLogger(ctx).Error("failed to convert to compensate req", zap.Error(err))
		}
	}
}

You can obviously convert the arg wherever you want. This uses much more familiar Go constructs like defer and is clearer to the developer what is called on rollback. There is no hiding things on interceptor and there is no pre-registration of what is run on which activity, etc.

[ychensha]

I understand the concerns, but interceptor itself is hidden behind workflow. Action/Compensation is registered aside with workflow like the testing code, the removal of defer is by design.

[cretz]

While I understand it is by design, the question is if that is the best design. Explicit defer at the callsite is clearer to a code writer than a hidden equivalent at the registration site. Using interceptors for this, besides hiding what is happening from someone reading the workflow code and matching against history, also is a bit error prone due to making sure that interceptors are always setup the same way for all workers and that the compensations never change in successive deployments. Not to mention interceptors also add more code, more complexity, and are more limiting feature wise (e.g. can't skip a compensation if it doesn't make sense, can't use a local variable as a parameter to the compensation, etc).

[ychensha]

I believe it is good enough to quick start. When number of code in workflow is large, and activity executions is in multi functions, defer is not a easy way to handle error and do compensations. Interceptor is designed to seperate non business logic apart.

[ychensha]

So interceptor is not welcome to implement saga pattern?

[cretz]

I believe it is good enough to quick start

Sure, the point is that it's easy to run something on complete when your code fails

defer is not a easy way to handle error and do compensations.

I disagree, I think it s very easy and common in Go. Many Go functions run defer after success of something but on failure of the function as a whole.

Interceptor is designed to seperate non business logic apart.

I am not sure we want to encourage that separation. Hiding some steps of a workflow can make it hard to match against history for readers. We want workflow actions to be explicit not implicit.

So interceptor is not welcome to implement saga pattern?

I am not sure it's the best way to implement the saga pattern as it hides so much from the user as opposed to the defer pattern. These samples are meant to document Temporal best practices, and I am not sure we want to encourage use of interceptors this way.

Let me see if I can get others' opinions here.