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

Add error logger #235

Closed
wants to merge 2 commits into from
Closed

Add error logger #235

wants to merge 2 commits into from

Conversation

dangra
Copy link

@dangra dangra commented Feb 24, 2023

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.

@dangra dangra changed the title Add custom error logger Add error logger Feb 24, 2023
@warmwaffles
Copy link
Member

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
Copy link
Member

@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
Copy link
Author

dangra commented Mar 15, 2023

@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
Copy link
Member

@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 ruslandoga mentioned this pull request Oct 19, 2023
@ruslandoga
Copy link
Contributor

ruslandoga commented Oct 19, 2023

👋

I've opened a PR with the suggested changes (send binary to pid): #266 (merged)

I mostly wanted to make sure this feature would work after #192 and one way to do that was to add set_log_hook/1 in master with some tests :)

@warmwaffles
Copy link
Member

#266 closes this

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