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

Discussion: Evolving the logger api #439

Closed
nfcampos opened this issue Dec 27, 2022 · 4 comments
Closed

Discussion: Evolving the logger api #439

nfcampos opened this issue Dec 27, 2022 · 4 comments

Comments

@nfcampos
Copy link
Collaborator

Hi @hwchase17

Thanks for making this, amazing open source project.

I'm looking to make Agents more usable in production environment (eg for building a user-facing "chatbot") and some parts of the existing langchain api might need some changes to make that possible. Namely, "streaming" (some kind of) progress to the user while an agent does its thing is key for a low latency experience. Another concern is to keep track of the "lineage" (ie. which LLM prompts / API calls) of a particular output, in order to eg. selectively display it to the user or support improving the agent over time.

From looking at the code of the library so far it looks to me like the best starting point to make this happen is to evolve the logger api, namely

  1. Make it (optionally) local to each instance, so that multiple agents can run independently in the same process without mixing things up, see PR Make logger instance local to each instance that uses it #437
  2. Evolve the BaseLogger API to be more amenable to structured logging, eg. the log_llm_response method could/should have available the prompt and inputs too, otherwise we don't know what that output is for, etc

Would welcome your thoughts?

Thanks
Nuno

@hwchase17
Copy link
Contributor

hi @nfcampos - these are great points, and something we're actively thinking about. would you have time to hop on quick call to discuss the points you raised? would love to talk through what we have in the works so far and see if it meets your needs! email is hw.chase.17@gmail.com, or on the discord server at hwchase17 - really would love to chat!

@jlewi
Copy link

jlewi commented Feb 24, 2023

I came across this issue while trying to figure out how to make LangChain playbetter with the logging module.
It looks like langchain/logger.py was deleted in
#478

It looks like the new approach relies on the callaback handler which gets invoked if verbose is true.

I'm guessing https://github.com/hwchase17/langchain/blob/master/langchain/callbacks/stdout.py is the default handler.

So I believe to use Python standard logging I would need to implement my own handler which relies on logging.

@anentropic
Copy link
Contributor

Maybe related, but I just noticed that different langchain modules are using different ways to set up a logger:

logger = logging.getLogger()
logger = logging.getLogger(__name__)
logger = logging.getLogger(__file__)

at first I thought it might be deliberate(?)... but I'm not so sure

In particular, using the root logger logger = logging.getLogger() seems like bad manners for a library

I suspect it'd be better if all of them used logger = logging.getLogger(__name__) which would then give a consistent hierarchical dotted path that would allow consumers of the library to handle and filter logs by name, e.g. everything with langchain. prefix

hwchase17 pushed a commit that referenced this issue Apr 16, 2023
re
#439 (comment)

I think it's not polite for a library to use the root logger

both of these forms are also used:
```
logger = logging.getLogger(__name__)
logger = logging.getLogger(__file__)
```
I am not sure if there is any reason behind one vs the other? (...I am
guessing maybe just contributed by different people)

it seems to me it'd be better to consistently use
`logging.getLogger(__name__)`

this makes it easier for consumers of the library to set up log
handlers, e.g. for everything with `langchain.` prefix
samching pushed a commit to samching/langchain that referenced this issue May 1, 2023
re
langchain-ai#439 (comment)

I think it's not polite for a library to use the root logger

both of these forms are also used:
```
logger = logging.getLogger(__name__)
logger = logging.getLogger(__file__)
```
I am not sure if there is any reason behind one vs the other? (...I am
guessing maybe just contributed by different people)

it seems to me it'd be better to consistently use
`logging.getLogger(__name__)`

this makes it easier for consumers of the library to set up log
handlers, e.g. for everything with `langchain.` prefix
@dosubot
Copy link

dosubot bot commented Sep 18, 2023

Hi, @nfcampos. I'm Dosu, and I'm helping the LangChain team manage their backlog. I wanted to let you know that we are marking this issue as stale.

Based on my understanding, you raised this issue to discuss evolving the logger API to make Agents more usable in a production environment. There have been comments from hwchase17, jlewi, and anentropic discussing the proposed changes and suggesting improvements to the logger setup and usage. However, the issue remains unresolved.

Could you please let us know if this issue is still relevant to the latest version of the LangChain repository? If it is, please comment on the issue to let the LangChain team know. Otherwise, feel free to close the issue yourself, or it will be automatically closed in 7 days.

Thank you for your contribution!

@dosubot dosubot bot added the stale Issue has not had recent activity or appears to be solved. Stale issues will be automatically closed label Sep 18, 2023
@dosubot dosubot bot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 25, 2023
@dosubot dosubot bot removed the stale Issue has not had recent activity or appears to be solved. Stale issues will be automatically closed label Sep 25, 2023
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

No branches or pull requests

4 participants