openllmetry-js icon indicating copy to clipboard operation
openllmetry-js copied to clipboard

🚀 Feature: re-write Langchain instrumentation to use Langchain Callbacks

Open nirga opened this issue 1 year ago • 7 comments

Which component is this feature for?

Langchain Instrumentation

🔖 Feature description

Right now, we monkey-patch classes and methods in LlamaIndex which requires endless work and constant maintenance. Langchain has a system for callbacks that can potentially be used to create/end spans without being too coupled with with the framework's inner structure.

🎤 Why is this feature needed ?

Support Langchain entirely and be future-proof to internal API changes

✌️ How do you aim to achieve this?

Look into Langchain callbacks and how other frameworks are using it.

🔄️ Additional Information

No response

👀 Have you spent some time to check if this feature request has been raised before?

  • [X] I checked and didn't find similar issue

Are you willing to submit PR?

None

nirga avatar Mar 07 '24 11:03 nirga

Hi, I will try to solve this issue

Xabilahu avatar Mar 14 '24 22:03 Xabilahu

Looking at the documentation about Langchain callbacks, I can see that there are 2 places where the callbacks argument is accepted:

  1. Constructor callback
  2. Request callback (a.k.a on call method)

I don't see a way on how this mechanism could replace monkey patching, as for 1 we would need to instantiate the objects for consumers, and for 2 we would need to find a way to intercept / wrap calls.

An idea would be to extend the classes that we want to instrument, and have users consume those instead of the ones from langchain, but I guess that defeats the purpose of auto-instrumentation...

Am I missing something?

Xabilahu avatar Mar 15 '24 01:03 Xabilahu

Thanks @Xabilahu! So what I had in mind is to monkey patch the constructors of objects like LLMChain, etc. and add our own callback there, and then build a callback handler that creates/ends spans accordingly. The benefit is that instead of being tightly coupled with all of Langchain implementation (which changes quite often), we need to know only the names of classes with callbacks - not that many. WDYT?

nirga avatar Mar 15 '24 13:03 nirga

It makes much more sense :) I will give it a try this weekend!

Xabilahu avatar Mar 15 '24 23:03 Xabilahu

Small status update. I've been trying to make this integration work, but I'm not able to. The issue is that the snippet below never gets executed (and thus, the spans are not being reflected in Traceloop), so I still need some more deep dive.

I don't know if this integration has ever been tested, if so, let me know, maybe I'm missing something.

https://github.com/traceloop/openllmetry-js/blob/3688da7f426d4ebc179ead8d90b47d52ee5e4ef9/packages/instrumentation-langchain/src/instrumentation.ts#L81-L102

Xabilahu avatar Mar 17 '24 10:03 Xabilahu

@Xabilahu - weird, have you change the log level to debug? Want to join slack to discuss?

nirga avatar Mar 17 '24 12:03 nirga

Submitted the WIP PR https://github.com/traceloop/openllmetry-js/pull/176, the missing piece is being able to monkey-patch JS class constructors via shimmer (as it's the library being used by OTel under the hood, source) so that we can inject the CallbackHandler in constructor params. For the moment, I've not been able to.

Xabilahu avatar Mar 30 '24 03:03 Xabilahu