-
Notifications
You must be signed in to change notification settings - Fork 15.7k
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 tracing support to langchain #741
Conversation
first pass at stdout callback for the most part, went pretty smoothly. aside from the code here, here are some comments/observations. 1. should somehow default to stdouthandler so i dont have to do ``` from langchain.callbacks import get_callback_manager from langchain.callbacks.stdout import StdOutCallbackHandler get_callback_manager().add_handler(StdOutCallbackHandler()) ``` 2. I kept around the verbosity flag. 1) this is pretty important for getting the stdout to look good for agents (and other things). 2) I actually added this for LLM class since it didn't have it. 3. The only part that isn't basically perfectly moved over is the end of the agent run. Here's a screenshot of the new stdout tracing ![Screen Shot 2022-12-29 at 4 03 50 PM](https://user-images.githubusercontent.com/11986836/210011538-6a74551a-2e61-437b-98d3-674212dede56.png) Noticing it is missing logging of the final thought, eg before this is what it looked like ![Screen Shot 2022-12-29 at 4 13 07 PM](https://user-images.githubusercontent.com/11986836/210011635-de68b3f5-e2b0-4cd3-9f1a-3afe970a8716.png) The reason its missing is that this was previously logged as part of agent end (lines 205 and 206) this is probably only relevant for the std out logger? any thoughts for how to get it back in?
i kinda like this just because we call `self.callback_manager` so many times, and thats nicer than `self._get_callback_manager()`?
…nkush/base-tracing
1. remove verbose from someplace it didnt relaly belong 2. everywhere else, make verbose Optional[bool] with default to None 3. make base classes accept None, and then look up globabl verbosity if thats the case
deprecate all prints in favor of callback_manager.on_text (open to better naming)
remove old logging class (no longer used anyways)
also add a set handler method usage is: ``` from langchain.callbacks.streamlit import StreamlitCallbackHandler import langchain langchain.set_handler(StreamlitCallbackHandler()) ``` produces the following output ![Screen Shot 2022-12-29 at 10 50 33 PM](https://user-images.githubusercontent.com/11986836/210032762-7f53fffa-cb2f-4dac-af39-7d4cf81e55dd.png) only works for agent stuff currently
* fix tracer tests * update notebook
* default session * cr
langchain/__init__.py
Outdated
@@ -4,6 +4,7 @@ | |||
|
|||
from langchain.agents import MRKLChain, ReActChain, SelfAskWithSearchChain | |||
from langchain.cache import BaseCache | |||
from langchain.callbacks import set_tracing_callback_manager # noqa: F401 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why the noqa?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's complaining about how it's not being used anywhere yet, but we need set_tracing_callback_manager
to be here so you can import it from top level langchain
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think if you add it to the all it should be good!
langchain/callbacks/tracers/base.py
Outdated
|
||
|
||
class BaseTracer(BaseCallbackHandler, ABC): | ||
"""An implementation of the Tracer interface that prints trace as nested json.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docstring
langchain/callbacks/tracers/base.py
Outdated
self._tracer_session = value | ||
|
||
|
||
class BaseLangChainTracer(BaseTracer, ABC): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
big file, could maybe be split up? base.py and then like langchain.py?
langchain/run.py
Outdated
@@ -0,0 +1,14 @@ | |||
"""Script to run langchain-server locally using docker-compose.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better file name than run probably. server.py?
pyproject.toml
Outdated
@@ -29,8 +32,10 @@ tiktoken = {version = "^0", optional = true, python="^3.9"} | |||
pinecone-client = {version = "^2", optional = true} | |||
weaviate-client = {version = "^3", optional = true} | |||
google-api-python-client = {version = "2.70.0", optional = true} | |||
freezegun = "^1.2.2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whats this needed for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unit tests. Since the tracer sets the start time and end time of each run using utcnow, this library overrides that for tests so we can test of equality. probably should add it as a test dep tho
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup test dep probably best!
BaseCallbackHandler
to support tracing:SharedTracer
which is thread-safe andTracer
which is not and is meant to be used locally.langchain-server