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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add support for Langchain Callbacks #176

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Xabilahu
Copy link

@Xabilahu Xabilahu commented Mar 30, 2024

Solves Issue #133: 馃殌 Feature: re-write Langchain instrumentation to use Langchain Callbacks

鈿狅笍 This PR is actually in WIP status until we find a way to monkey patch class constructors 鈿狅笍

Implemented a BaseCallbackHandler (source for the interface). The main idea is that the handler has some state for each of the spans it starts, so that when handleXXEnd or handleXXError are invoked, the same span can be properly ended. To do that, we are using a stack, the logic is as follows:

  1. We start a Span whenever handleXXStart is called and push it into the stack.
  2. If handleXXEnd is called:
    1. If there are no Span instances in the stack, we throw an Error, as handleXXEnd cannot be called before the corresponding handleXXStart.
    2. Otherwise, we pop the Span from the stack, and end it accordingly.
  3. If handleXXError is called, we follow a similar logic as for handleXXEnd.

This approach has the benefit of being able to properly handle nested spans.


Tests

Unit tests
npm run build:all && npx nx run @traceloop/instrumentation-langchain:test
Integration tests (aka sample-app)
  • Modified the sample-app for LangChain (this is expected to go away before merging this PR, as it's just for my own testing purposes) to:
    • Not depend on OpenAI stuff, as I lack an API key 馃ゲ
    • Make it faster by just running a single workflow
    • Disable manual instrumentation of LangChain modules
    • Instantiate and pass the TraceloopCallbackHandler to check if the spans are generated properly
  • Start Ollama serving llama2 LLM by running:
docker run -d -v ollama:/root/.ollama -p 11434:11434 --name ollama ollama/ollama && docker exec -it ollama ollama run llama2
  • Ran the sample app via:
TRACELOOP_API_KEY=<REDACTED> npx nx run sample-app:run:langchain
  • Verified that there is a trace in my Traceloop development stage, but it lacks the spans from the CallbackHandler, as I'm using a ProxyTracerProvider, which defaults to NoOp tracer when no delegate is provided. This behavior should change, and spans will be sent once we manage to mockey-patch LangChain constructors, and inject the CallbackHandler in constructor params.
  • For the moment, I just verified the behavior via logs.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Xabier Lahuerta V谩zquez seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@nirga
Copy link
Member

nirga commented Apr 6, 2024

@Xabilahu just for my own records, linking our discussion from Slack where I suggested a solution to the problems you've raised here.

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.

None yet

3 participants