Add spandex_ecto #78
Add spandex_ecto #78
Conversation
ayrat555
force-pushed
the
ayrat555/integrate-spandex-ecto
branch
from
ef841b3
to
d26c5a8
Compare
| @@ -18,6 +18,10 @@ defmodule Engine.Application do | |||
|
|
|||
| def start(_type, _args) do | |||
| attach_telemetry() | |||
|
|
|||
| :ok = | |||
| :telemetry.attach("spandex-query-tracer", [:childchain, :repo], &SpandexEcto.TelemetryAdapter.handle_event/4, nil) | |||
There was a problem hiding this comment.
Perhaps we could expand on the above attach_telemetry/0 and we do all the telementry setup there?
There was a problem hiding this comment.
I moved it to the attach_telemetry
| @@ -18,6 +18,10 @@ defmodule Engine.Application do | |||
|
|
|||
| def start(_type, _args) do | |||
| attach_telemetry() | |||
|
|
|||
| :ok = | |||
| :telemetry.attach("spandex-query-tracer", [:childchain, :repo], &SpandexEcto.TelemetryAdapter.handle_event/4, nil) | |||
There was a problem hiding this comment.
| :telemetry.attach("spandex-query-tracer", [:childchain, :repo], &SpandexEcto.TelemetryAdapter.handle_event/4, nil) | |
| :telemetry.attach("spandex-query-tracer", [:childchain, :repo, :query], &SpandexEcto.TelemetryAdapter.handle_event/4, nil) |
I think we need the :query portion right? Where can we see example alerts for this?
There was a problem hiding this comment.
I added an example in the description
config/config.exs
Outdated
| @@ -66,6 +66,8 @@ config :status, Status.Metric.Tracer, | |||
| type: :backend, | |||
| env: "local-development-childchain-v2" | |||
|
|
|||
| config :engine, Engine.Repo, telemetry_prefix: [:childchain, :repo] | |||
There was a problem hiding this comment.
| config :engine, Engine.Repo, telemetry_prefix: [:childchain, :repo] | |
| config :engine, Engine.Repo, telemetry_prefix: [:engine, :repo] |
should we just use the defaults? We can make it explicit too
| _ = Logger.info("Attaching telemetry handlers #{inspect(Handler.supported_events())}") | ||
|
|
||
| case :telemetry.attach_many("alarm-handlers", Handler.supported_events(), &Handler.handle_event/4, nil) do | ||
| :ok -> :ok | ||
| {:error, :already_exists} -> :ok | ||
| end | ||
| end | ||
|
|
||
| defp submit_trace(arg1, arg2, arg3, arg4) do |
There was a problem hiding this comment.
I created this function as a workaround. current_trace_id in https://github.com/spandex-project/spandex_ecto/blob/master/lib/spandex_ecto/ecto_logger.ex#L21 is always nil so without my wrapper traces are not submitted.
There was a problem hiding this comment.
I noticed the same thing! But it's really really odd that a project is this old and this was never reported.
spandex-project/spandex_ecto#2
spandex-project/spandex_ecto#17
It seems that spandex_ecto only works together with spandex_phoenix, so that spandex_phoenix starts a trace (like a HTTP requests) and spandex_ecto just updates the trace?
There was a problem hiding this comment.
I created an issue spandex-project/spandex_ecto#22
There was a problem hiding this comment.
The problem I think with this approach is that watcher info has endpoints that call into ecto:
scope "/", OMG.WatcherRPC.Web do
pipe_through([:info_api])
post("/block.all", Controller.Block, :get_blocks)
end
our spandex web tracer would in this case start a trace in the cowboy request process (so current_trace_id would not be nil?),
and manually starting a trace would cause an error on spandex, perhaps?
There was a problem hiding this comment.
ugh, I hope you understand what I mean, difficult to explain. Will we have overlapping traces in some cases (cases where the http request already starts a trace)?
There was a problem hiding this comment.
I have an idea. we can check if current trace id is set. if it is, we won't wrap a call to the ecto logger in a new trace
There was a problem hiding this comment.
another idea! perhaps the correct way is to start a trace from the source code where the code initiated ecto? like so: https://github.com/omgnetwork/elixir-omg/blob/master/apps/omg/lib/omg/ethereum_event_listener.ex#L143
There was a problem hiding this comment.
@InoMurko do you mean to create a trace before every DB call?
There was a problem hiding this comment.
Yeah. Because if I’m not mistaking, then the current solution would give datadog the idea that the ecto query started in the application module, right?
|
it's not ready yet. spans for the same query are separated now. the correct one is omgnetwork/elixir-omg#1602 |
Does this work now or is there another fix arriving? |
|
@InoMurko it works now |
|
oh great! so you didn't have to wrap Repo as in elixir-omg? |
|
@InoMurko yes, I had to pass the same tracer that is used in ecto_spandex |
|
noice! cool! |
Resolves https://github.com//issues/70https://app.datadoghq.com/apm/resource/ecto/Engine.DB.ListenerState.get_height_1/91a4ccefd58b90af?end=1593420650301&env=local-development-childchain-v2&index=apm-search&paused=false&query=env%3Alocal-development-childchain-v2%20service%3Aecto%20operation_name%3AEngine.DB.ListenerState.get_height_1%20resource_name%3A%22Engine.DB.ListenerState.get_height%2F1%22&start=1593417050301&event=AQAAAXL_RLt2HMsdcgAAAABBWEwvUk1hYjl2UzFFWTdUR0VBOQ&traceID=4423310115256194805&spanID=6946230175853006616