open-telemetry / opentelemetry-erlang-api

Erlang/Elixir OpenTelemetry API
Apache License 2.0
60 stars 13 forks source link

Elixir API #13

Closed tsloughter closed 4 years ago

tsloughter commented 4 years ago

I cherry-picked from the PagerDuty fork.

hauleth commented 4 years ago

Do we really need all that delegations without any other functions provided? In such case I would rather suggest using Erlang modules directly.

tsloughter commented 4 years ago

The argument is that if you are going to provide a Tracer module with a macro start_span (which I'm adding now) then it would be weird to mix the two.

As in, you'd use an Elixir module for starting the span but then have to go to :ot_tracer to add attributes to the span.

I think the argument makes sense, it just looks odd right now since I haven't added the other functionality to the PR yet.

hauleth commented 4 years ago

If there will be more features, like macros for spans then it makes sense.

tsloughter commented 4 years ago

Pushed a work in progress set f macros.

Likely the use of using and adding __get_tracer__ will be the biggest contention.

Because having names for tracers and meters is pretty much going to be required going forward I personally would prefer being able to set the default tracer to use for a module in one go when I'm including the Tracer module. I'd add this in Erlang if I could :)

I think the default name for the tracer should maybe be the application this module is in during compilation. Not sure if that is exposed by Elixir during compilation thought? Like a __APPLICATION__ variable?

hauleth commented 4 years ago

Like a __APPLICATION__ variable?

In theory you can use Mix.Project.config()[:app] to get that name, however if that would be needed then I would go with solution similar to one used by Ecto, which is defining module that will use OpenTelemetry.Tracer and define all needed functions. Ex.:

defmodule MyApplication.Tracer do
  use OpenTelemetry.Tracer
end

And then somewhere in application:

MyApplication.Tracer.with_span(fn ->
  # do stuff
end)
tsloughter commented 4 years ago

Hm, good point about the Ecto solution.

hauleth commented 4 years ago

I have provided example implementation in tsloughter/opentelemetry-erlang-api#1

irvingreid commented 4 years ago

I signed it

tsloughter commented 4 years ago

I've pushed an attempt at the "Ecto model".

One thing I didn't do was the otp_app thing that is used for going configuration by Ecto Repos.

I don't think there is anything to configure but the name on these tracers. Anything else is "global", like what sdk is used, what the sdk tracer module is, what processors to use, etc.

tsloughter commented 4 years ago

I'm going to first implement registering named tracers and then come back to this to finish it up.

Still undecided on the direction. Got push back on going the Ecto Repo route of creating a module. I can understand that, but I want something to ensure users will always use named and versioned tracers without feeling it is a useful pain. In Elixir that seems to be simple to do, in Erlang we likely are going to have to require users do stuff like:

Tracer = opentelmetry:get_tracer(myapp),
otel:with_span(Tracer, span_name, fun() -> ... end).
tsloughter commented 4 years ago

Closing and opening a new one in a bit.