feat(langchain): improve callbacks

traceloop / openllmetry

Open-source observability for your LLM application, based on OpenTelemetry

https://www.traceloop.com/openllmetry

Apache License 2.0

1.52k stars 114 forks source link

feat(langchain): improve callbacks #1426

Closed tibor-reiss closed 4 days ago

tibor-reiss commented 1 week ago

Builds on top of https://github.com/traceloop/openllmetry/pull/1317 which should be merged first:

Added (a)stream and (a)transform to wrapped function list.
Removed "instance" logic.
[x] I have added tests that cover my changes.
[ ] If adding a new instrumentation or changing an existing one, I've added screenshots from some observability platform showing the change.
[x] PR name follows conventional commits format: feat(instrumentation): ... or fix(instrumentation): ....
[ ] (If applicable) I have updated the documentation accordingly.

nirga commented 5 days ago

@tibor-reiss it's not that the OpenAI span is not generated - it's generated on a separate trace, which is a problem. It should be on the same trace as the rest of the LangChain spans:

Screenshot 2024-07-03 at 21 39 36

tibor-reiss commented 5 days ago

@tibor-reiss it's not that the OpenAI span is not generated - it's generated on a separate trace, which is a problem. It should be on the same trace as the rest of the LangChain spans:

Hi @nirga, could you please share your code which resulted in this?

I checked several places and noticed that this seems to be already broken in main. E.g. in test_lcel / test_streaming (I renamed this test to test_invoke in this PR).

The issue seems to come from that BaseChatModel.generate is called. This is still in the non-callback world. It has a different signature (no config in 2nd place), so adding it to SYNC_FUNCTIONS is not enough - probably needs a separate logic to use it's 3rd argument, callbacks - I'll look into it asap...

nirga commented 5 days ago

Hmm @tibor-reiss might be new since we introduced the callbacks, cause this used to work with the previous mechanism

tibor-reiss commented 5 days ago

Hmm @tibor-reiss might be new since we introduced the callbacks, cause this used to work with the previous mechanism

@nirga Thought the same, so was just checking the stuff before the callbacks (https://github.com/tibor-reiss/openllmetry/blob/aaa303b9ffe6926d1830c51f911918d570ce2b1c/packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/__init__.py): the test test_simple_lcel already has this problem, i.e. openai_chat is not part of the workflow.