Skip to content

Conversation

@fishcakez
Copy link
Member

First commit changes the behaviour and the second commit does a substantial refactor to help with #82 and any future metrics work. Hopefully this will pave the way for future integration with any standard instrumentation lib. This PR uses a stateful approach because it fits well with multiple timestampings. However all "connection_time" work is carried out inside a closure except begin/commit/rollback.

@fishcakez fishcakez requested a review from josevalim July 3, 2017 22:35
catch
kind, reason ->
stack = System.stacktrace()
{kind, reason, stack, meter}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now long poolboy time outs

meter = event(meter, :checkout)
with {:ok, conn, conn_info, meter} <- checkout(pool, meter, opts) do
try do
fun.(conn, conn_info, arg, meter, opts)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All "connection time" runs inside this fun (or similar, above/below), which could be instrumented using fun based instrumentation.

end
defp run_begin(conn, conn_info, meter, opts) do
meter = event(meter, :begin)
handle_trans(conn, conn_info, :handle_begin, meter, opts, :transaction)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Transaction calls aren't wrapped in run calls with funs but could be as parameters align.

defp event(nil, _),
do: nil
defp event({log, events}, event),
do: {log, [{event, :erlang.monotonic_time()} | events]}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simple stack based/pure implementation so could wrap any stateful instrumenter

@fishcakez fishcakez merged commit 53271f7 into master Jul 4, 2017
@fishcakez fishcakez deleted the jf-no-log-raise branch July 4, 2017 09:21
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

Successfully merging this pull request may close these issues.

3 participants