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

Query Logging: Appsignal.Ecto.log/1 errors when using the preferred Ecto.Repo loggers format #27

Closed
wheels opened this issue Oct 19, 2016 · 5 comments

Comments

@wheels
Copy link

wheels commented Oct 19, 2016

From the Ecto docs:

:loggers - a list of {mod, fun, args} tuples...

https://hexdocs.pm/ecto/Ecto.Repo.html

In Ecto 2.0.2, when I add Appsignal to Repo loggers using the preferred format

loggers: [{Appsignal.Ecto, :log, []}]

it throws an error here:

https://github.com/appsignal/appsignal-elixir/blob/master/lib/appsignal/ecto.ex#L29

** (exit) an exception was raised:
    ** (UndefinedFunctionError) function nil.queue_time/0 is undefined or private
        nil.queue_time()
        (appsignal) Appsignal.Ecto.log/1
@arjan
Copy link
Contributor

arjan commented Oct 20, 2016

Sorry, I can't seem to reproduce this. Are you using the latest version? It seems the LogEntry that's passed in is nil..

I also don't see how this variant could cause the error, they look pretty similar. Are you doing something special? e.g. when there is another logger that does not return the entry value back, the next logger function could crash.

@wheels
Copy link
Author

wheels commented Oct 20, 2016

It only happens in prod for me. I'm passing in other loggers as well so it looks like this:

loggers: [{Ecto.LogEntry, :log, []}, {MyApp.Repo.Metrics, :record_metric, []}, {Appsignal.Ecto, :log, []]

@arjan
Copy link
Contributor

arjan commented Oct 20, 2016

You should check whether your record_metric function returns the log entry.

@wheels
Copy link
Author

wheels commented Oct 20, 2016

Ah, nice catch. Didn't realize each one affected subsequent calls.

@wheels wheels closed this as completed Oct 20, 2016
@arjan
Copy link
Contributor

arjan commented Oct 21, 2016

NP! I also just realised this by looking at Ecto's source.

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