Nebo15 / sage

A dependency-free tool to run distributed transactions in Elixir, inspired by Sagas pattern.
MIT License
912 stars 40 forks source link

Add `effects_so_far` to `Sage.Tracer` callbacks #61

Open ulissesalmeida opened 3 years ago

ulissesalmeida commented 3 years ago

Hi! I'm implementing opentelemetry tracing on my job application and collecting the effects_so_far as tracing attributes would be a very handful to understand some Sage pipelines. Today Sage provides the hook's name, the action, and the shared opts.

What do you all think about this idea?

AndrewDryga commented 3 years ago

Hello @ulissesalmeida. Can you please elaborate a bit on how it's going to be used? In theory, if you know the name you should know that should be put under that name (otherwise you can't compose the saga).

I'm asking because even though I'm not opposed to adding it I want to make sure that there is no better way. The biggest concern is that it's not a safe change - effects can be very large (we don't know what people put there) and sending them can cause some issues in production.

ulissesalmeida commented 3 years ago

Thank you for the quick reply ❤️

Today we have something like this:


  @impl true
  def handle_event(step_name, :start_transaction, opts) do
    span_opts = %{attributes: [{"sage_opts", Traceable.attributes(opts)}}
    OpenTelemetry.Tracer.start_span(to_string(step_name), span_opts)

    opts
  end

  def handle_event(step_name, :finish_transaction, _opts) do
    OpenTelemetry.Tracer.end_span()
    opts
  end

Traceable is our internal protocol, where the developers need to explicit implement which fields are allowed in the tracing. So, it would be nice to trace the effects_so_far in the handle_event callback. The problem of effects being large will be avoided by our company developers explicitly implementing the Traceable protocol and filtering out unwanted data in the tracing.

I imagine addinghandle_event/4 as an optional callback, where the last argument is the effects_so_far. If the developers don't want to use it, they can implement handle_event/3.

What do you think?

joseustra commented 3 years ago

Hey 👋

That would be great.

I'm using sage as the executor, so I can use all the features it provides, but I have a module that wraps sage, so I can decide if a specific function should be retried, skipped, or even if the whole process should stop and wait for manual intervention.

For context: I have a use case, that involves third parties services that don't allow compensation and are not idempotent. Having the effects_so_far available in the callback would be great because it will allow me to keep track of the responses, so my "manager" module can decide what to do with the responses.

I like the suggestion of handle_event/4. It will keep the current implementations working, but adds a new option for those who need it.

rauann commented 2 years ago

Hi everyone, any updates on this? Having the effects_so_far in the Tracer can be pretty useful but would be possible/make sense to have the current transaction effect when the Trace receives the finish_transaction action?

AndrewDryga commented 2 years ago

@rauann we can extend the callback and make it accept effects_so_far. But we should make sure it's not a default behavior as it can be not safe - effects can be a very large structure that takes a lot of time copying it between processes. We can discuss implementation and once we have a plan a PR would be welcome.