Allow send `options` to `Tracer` callbacks ?

[ulissesalmeida]

Hello.

On our current app we have resuable "Tracer" module where can trace things, collect metrics, and etc. But we would like to implement a way for developers to opt in or opt out some of these features.

So for example:

with_tracer(sage, MyApplicationTracer)

We would like to do something like:

with_tracer(sage, MyApplicationTracer, metrics?: true, trace_span: false, logging: false)

Today that is achievable by creating multiple modules:

TraceWithMetrics
TraceWithMetricsAndSpan
TraceWithMetricsButNotSpanWithLogging

Obviously not scalable. We could use the state/context, on execute:

execute(sage, %{my_thing | tracing_opts: [metrics?: true, trace_span: false, logging: false]}

The problem of that approach is limit the things that we can use in execute to be always a map. Approachable, but not great.

Another option is similar to module one, but using macros

def MyApp.TracerBuilder do
  defmacro ___using___(opts) do
#...
end

def MyApp.MySageTransactionTracer do
  use MyApp.TracerBuilder, [metrics?: true, trace_span: false, logging: false]
# ...

with_tracer(sage, MySageTransactionTracer)

The issue is the macro addition and the ceremony to always have to define a tracer module.

All that could be avoided with we dispatch an extra argument on with_tracer that could be accessed as last argument on handle_event.

What do you think? Would you be interested on such change?

[AndrewDryga]

Hey. I think this would be a great addition and we can extend with_tracer function to accept opts that we will send down to tracing functions. A PR would be welcome. To do that we'll need to convert tracers mapset to a map %{module => opts}. We also should add a check that when a tracer is overridden by multiple calls we should emit some sort of warning.

[AndrewDryga]

An alternative approach might be to accept mfa tuple or a function capture as a tracer, so that you can add extra arguments whenever you want: {MyTracer, trace, [metrics?: true, trace_span: false, logging: false]} or &MyTracer.trace(&1, ..., metrics?: true, trace_span: false, logging: false)

[ulissesalmeida]

@AndrewDryga The mfa is really a good idea.

However the tradeoff is we can't have behaviours for mfa. 🤔

And we need to document the first 3 parameters will be step_name, event and context. It would be good to simplify the interface, for example, we could merge these 3 parameters in a single structure, like Sage.Trace.Event. So the user know the first parameter is reserved to Sage.

What do you think? I like the flexibility. But it can be a little bit confusing having two mechanisms.

[AndrewDryga]

Honestly, I don't have preferences here. I feel like for documenting and type checking mfa might be the worst option (there is no way for Dialyzer to know what is that MFA doing) but it's pretty common in the community. I would lean towards anonymous function (easy to type check, self-documented, can be stored in the same MapSet with changing only guard clause and executor clause) or adding extra parameter opts to with_tracers as you initially proposed.

For our use cases, this is not a big deal and I would love you to do what is best for your codebase since this use case influences you the most :).

[ulissesalmeida]

I created a PR. I decided to go with function and mfa to keep it consistent with transaction and compensation.