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

Use phoenix and ecto instrumentation #7

Open
blatyo opened this issue Feb 22, 2017 · 8 comments
Open

Use phoenix and ecto instrumentation #7

blatyo opened this issue Feb 22, 2017 · 8 comments

Comments

@blatyo
Copy link

blatyo commented Feb 22, 2017

Phoenix instrumentation is mentioned here.

Ecto instrumentation is mentioned here. Search for :loggers.

@wfgilman
Copy link

wfgilman commented Mar 6, 2017

I've been looking into these two. I think there may be an issue with Ecto instrumentation because you are restricted to the data in the Ecto.LogEntry. You'd be missing out on the calling module.

For the Phoenix instrumentation, the approach this library takes seems much simpler. Instead of creating many event handlers for each action you want to instrument, you can pass it straight through to the collector from the controller via a plug.

@blatyo
Copy link
Author

blatyo commented Mar 8, 2017

For the Phoenix part, it's actually pretty simple, because they already instrument parts of your app including the web requests, web sockets, and view rendering. So, you just provide a function to tie into those existing instrumentations. The plug version only records the web request time.

For the Ecto version, yes, you would be limited to the Ecto.LogEntry information. As far as I'm aware, that provides more information than what is provided currently with this library. What calling module are you referring to?

Also, it appears conn is passed around just so that new_relixir_transaction can be accessed. That would probably be a candidate for storage in the process dictionary, which would not require the information to be passed around. This is what Logger uses for it's metadata for example, so that you don't have to pass metadata to each log request.

@wfgilman
Copy link

wfgilman commented Mar 8, 2017

Thanks for the color on the Phoenix instrumentation. I didn't realize that.

For Ecto, I'm looking to use this library separate from Phoenix, so I don't want to pass a conn around. I'd like to instrument my db functions which perform ETL and need something more general purpose - something like this:

  @doc """
  Records execution time of a function.
  """
  @spec record(mfa, atom) :: any
  def record({mod, fun, args}, scope) when is_atom(scope) do
    f = fn -> Kernel.apply(mod, fun, args) end
    {elapsed, result} = :timer.tc(f)
    name = "#{mod}.#{to_string(fun)}"
    :ok = NewRelic.Collector.record_value({name, {scope, name}}, elapsed)
    result
  end

By "calling module" I meant being able to access the meta data stored in the conn.private under new_relixir_transaction so I can associate the Ecto.LogEntry data with it.

I hadn't considered a process dictionary, I need to look into that.

@blatyo
Copy link
Author

blatyo commented Mar 8, 2017

I agree, this library should be usable outside a phoenix context. It makes sense to be able to instrument any type of application.

@wfgilman
Copy link

wfgilman commented Mar 8, 2017

I'm working on some modifications to it. I'll put it up in a couple of days.

@wfgilman
Copy link

I started working on a PR to add some of these features, but ended up having a hard time finding the documentation for the API that lhttpc was talking too. I also had trouble making sense of the transformation logic in the statman module. In short, I ended up re-writing the agent and statman modules and ultimately realized this was less a PR than a separate library. It's up on my git if anyone wants to take a look, called NewRelix. Maybe we can combine forces here.

@blatyo
Copy link
Author

blatyo commented Mar 14, 2017

I looked and it looks like you implemented it as a new relic plugin API instead of using the APM API. That's workable, but from my perspective inferior, because new relic can show you this request took a long time, because the database call took a long time. That context gets lost when sending them as separate events.

As for the APM API, it's not publicly available. I believe it was reverse engineered from the ruby gem [1]. If you wanted to go that route, I'd recommend starting by looking at the tests first.

@wfgilman
Copy link

I see your point. I probably won't go down that road and reinvent the wheel here. From what I understand, native Elixir support a WIP at New Relic.

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

2 participants