search

vanna-ai / vanna

🤖 Chat with your SQL database 📊. Accurate Text-to-SQL Generation via LLMs using RAG 🔄.
https://vanna.ai/docs/
MIT License
11.36k stars 898 forks source link

Add an overridable logging function #176

Open zainhoda opened 9 months ago

zainhoda commented 9 months ago

Instead of using hardcoded print statements, there should be a logging function in the base class. That logging function in particular should be called before submitting the prompt so that you can log what the fully constructed prompt looks like.

By default, the log function in the base class should just be a print statement. If a user wants to send logs to a specific destination, they can just override the log function.

justbee007 commented 9 months ago

Hi I am new to contributing for open source and would like to contribute to this project.

pedro-geraghty-sw commented 9 months ago

It would be nice to be able to log metrics on times response and token consumption and other related stuff

justbee007 commented 9 months ago

@pedro-geraghty-sw are you going to work on it?. I thought of fixing this issue.

justbee007 commented 9 months ago

@pedro-geraghty-sw are you gonna work on it?. I thought of fixing this issue.

pedro-geraghty-sw commented 9 months ago

Go ahead

NickCrews commented 9 months ago

Thoughts:

zainhoda commented 9 months ago

Thoughts:

  • The log function should take the same severity levels as the building logging module as an optional kwarg. Default is INFO?
  • If we're smart, it could also support structured logging eg structlog.

Agreed. Also, I think it would be good for generate_sql to start with a uuid as a trace_id that gets passed into all the constituent functions so that logs for the same generate_sql request can easily be connected

justbee007 commented 9 months ago

You can assign this ticket to me. Thank You

andreped commented 8 months ago

@justbee007 What is the status on this?

I can make a PR for this today, as I need it for one of my projects very urgently.

I have made similar logging solutions for framework before, so should not be that difficult. I can tag you in, @justbee007, if you wish to contribute to the PR.

cc: @zainhoda

justbee007 commented 8 months ago

@andreped Yes Please you can make a PR and tag me in. We can work on this together.

andreped commented 8 months ago

@andreped Yes Please you can make a PR and tag me in. We can work on this together.

Drafted a PR: https://github.com/vanna-ai/vanna/pull/234

Feel free to suggest improvements. I already hinted at some potential suggestions in the PR comments.

I think it would be good for generate_sql to start with a uuid as a trace_id that gets passed into all the constituent functions so that logs for the same generate_sql request can easily be connected

@zainhoda If you hint me in the right direction in the PR with an example, I can make an attempt. Feel free to review the PR :]

talrejanikhil commented 2 months ago

Was this PR merged and released?

andreped commented 2 months ago

Was this PR merged and released?

@talrejanikhil No, but I believe @zainhoda implemented a different solution. Not sure though.