raven-erlang
raven-erlang copied to clipboard
Make http request async
raven:capture/2 blocks until http request finishes. It can slow down error_logger when raven is used with {error_logger, true} because error_logger waits until raven_capture/2 finishes.
Currently raven:capture/2 does not check return value of http request so it's safe to make http request async.
Any comment?
@yjh0502 raven-erlang isnt a super widely used thing unfortunately. That said, and knowing nothing about erlang, async defaults are +1 from me
I am not quite sure to be honest, the changes you suggest will lead to errors being silently dropped and your logs being lost, also it you are removing back pressure which means that you might end up with memory usage ballooning if you are producing more logs than what sentry can ingest.
@asabil Errors are already silently dropped because there is no return value check on httpc:request. If we need to check errors, we can handle error in both sync/async cases. With current implementation memory usage is ballooning when reporting errors to sentry via error_logger, because error_logger is buffering errors for raven-erlang. httpc:request can takes hundreds of milliseconds only with network latency when using hosted sentry service at other continents, in this case throughput of whole error logging system is limited to <10 tps.
How about adding raven:capture/3 which takes option argument? It can take [async]
or [{timeout, 1000}]
as options. For back-pressure, we can add timeout to httpc:request
which effectively limits memory usage. I'll prepare a patch for this.
Sounds good, however I would suggest adding the options to the DSN, in a similar manner to other Sentry clients (for example: https://github.com/getsentry/raven-java)