Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add effects_so_far to Sage.Tracer callbacks #61

Open
ulissesalmeida opened this issue Apr 28, 2021 · 5 comments
Open

Add effects_so_far to Sage.Tracer callbacks #61

ulissesalmeida opened this issue Apr 28, 2021 · 5 comments

Comments

@ulissesalmeida
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor Author

ulissesalmeida commented Apr 28, 2021

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
Copy link

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
Copy link

rauann commented Mar 17, 2022

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
Copy link
Member

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants