elixir-sqlite / exqlite

An SQLite3 driver for Elixir
https://hexdocs.pm/exqlite
MIT License
217 stars 48 forks source link

Add error logger #235

Closed dangra closed 1 year ago

dangra commented 1 year ago

I am submitting this change looking for directions on what could be the best way to integrate it into official exqlite release. This particular change has been extremely useful when debugging errors on applications that use LiteFS and I am sure could be useful to others.

From sqlite docs:

The SQLITE_CONFIG_LOG option is used to configure the SQLite global error log. (The SQLITE_CONFIG_LOG option takes two arguments: a pointer to a function with a call signature of void()(void,int,const char*), and a pointer to void. If the function pointer is not NULL, it is invoked by sqlite3_log() to process each logging event. If the function pointer is NULL, the sqlite3_log() interface becomes a no-op.

The void pointer that is the second argument to SQLITE_CONFIG_LOG is passed through as the first parameter to the application-defined logger function whenever that function is invoked.

The second parameter to the logger function is a copy of the first parameter to the corresponding sqlite3_log() call and is intended to be a result code or an extended result code.

The third parameter passed to the logger is log message after formatting via sqlite3_snprintf().

The SQLite logging interface is not reentrant; the logger function supplied by the application must not invoke any SQLite interface. In a multi-threaded application, the application-defined logger function must be threadsafe.

warmwaffles commented 1 year ago

What we'll want to do here is not use that fprintf, but instead we'll need to take that error string that is emitted by sqlite, and notify erlang somehow with the message.

There is a function in the sqlite3_nif.c called make_binary(env, the_string, length_of_string). This will return the ERL_NIF_TERM which we can then enif_send to a pid.

We'll need to stand up a listener for this.

This does tie into #192 somewhat where we should probably spin up one process instance.

warmwaffles commented 1 year ago

@dangra What are you thinking about with consuming the log messages? Were you just wanting to dump to Logger and friends? Or were you wanting to monitor the events pushed out via a handler of some sort?

dangra commented 1 year ago

@warmwaffles I haven't had the time to look at it and honestly lack the experience with NIFs and Elixir, to me it is all new stuff. Not making excuses! just setting expectations here, if the open PR bothers you, I am ok closing it for now and resubmit once I have something that integrates your feedback.

that said, my only goal was to log the messages so in case of this type of errors, I can tail logs and see what sqlite3.c function and line have error'ed. I don't have the need to push the events to a handler of any type.

warmwaffles commented 1 year ago

@dangra I was going to implement this using the methods I outlined above. We shouldn't be printing to stdout using the NIF. We risk interleaving output in a highly concurrent environment.

ruslandoga commented 1 year ago

👋

I've opened a PR with the suggested changes (send binary to pid): https://github.com/elixir-sqlite/exqlite/pull/266 (merged)

I mostly wanted to make sure this feature would work after https://github.com/elixir-sqlite/exqlite/issues/192 and one way to do that was to add set_log_hook/1 in master with some tests :)

warmwaffles commented 1 year ago

266 closes this