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

[feature request] JS: Support LangChain 0.3.x #1062

Closed
codefromthecrypt opened this issue Oct 14, 2024 · 7 comments
Closed

[feature request] JS: Support LangChain 0.3.x #1062

codefromthecrypt opened this issue Oct 14, 2024 · 7 comments
Assignees
Labels
enhancement New feature or request language: js Related to JavaScript or Typescript integration

Comments

@codefromthecrypt
Copy link

Is your feature request related to a problem? Please describe.

Right now, I can't use langchain 0.3.x in javascript due to a dependency constraint.

10.57 npm error Found: @langchain/[email protected]
10.57 npm error node_modules/@langchain/core
10.57 npm error   @langchain/core@"^0.3.11" from the root project
10.57 npm error
10.57 npm error Could not resolve dependency:
10.57 npm error peer @langchain/core@"^0.1.0 || ^0.2.0" from @arizeai/[email protected]

Describe the solution you'd like

Support langchain 0.3.x

Describe alternatives you've considered

downgrade

Additional context

@codefromthecrypt codefromthecrypt added enhancement New feature or request triage Issues that require triage labels Oct 14, 2024
@github-project-automation github-project-automation bot moved this to 📘 Todo in phoenix Oct 14, 2024
@dosubot dosubot bot added the language: js Related to JavaScript or Typescript integration label Oct 14, 2024
@mikeldking mikeldking removed the triage Issues that require triage label Oct 14, 2024
@mikeldking
Copy link
Contributor

@codefromthecrypt thanks for the ticket and for highlighting this issue. We'll take a look. cc @Parker-Stafford

@Parker-Stafford
Copy link
Contributor

closed in #1080
@codefromthecrypt this is released now under version 1.0.0 - there is also now support for a traceConfig for masking sensitive data and context attribute propagation to child spans see #1078

@codefromthecrypt
Copy link
Author

thanks a lot. I'll try it!

@codefromthecrypt
Copy link
Author

So, I noticed that langchain instrumentation creates different and redundant openai spans to the openai instrumentor. Is that intentional? It seems like we would want the same conventions or obviate redundant instrumentation (yet retain the parentage).

e.g. ChatOpenAI
Screenshot 2024-11-05 at 3 49 57 PM

which is in the correct trace, but if you also have the openai instrumentation configured, there's a separate trace for the same span:
Screenshot 2024-11-05 at 3 53 18 PM

That the openai instrumentation are not in the correct trace seems related to this, but possibly is a separate issue #1061

@Parker-Stafford
Copy link
Contributor

So, I noticed that langchain instrumentation creates different and redundant openai spans to the openai instrumentor. Is that intentional? It seems like we would want the same conventions or obviate redundant instrumentation (yet retain the parentage).

...
which is in the correct trace, but if you also have the openai instrumentation configured, there's a separate trace for the same span:
...
That the openai instrumentation are not in the correct trace seems related to this, but possibly is a separate issue #1061

Hey @codefromthecrypt thanks for testing it out! Yeah using the auto-instrumentors in conjunction may have some interesting consequences since right now they don't have any knowledge of the other one. Here's some context for your questions, hope this helps!

...creates different and redundant openai spans to the openai instrumentor. Is that intentional? It seems like we would want the same conventions or obviate redundant instrumentation (yet retain the parentage).

  • Reudundant: this is not necessarily intentional in that it is not a desired feature but it is expected behavior. The two auto instrumentors don't have any knowledge of the other, they simply start spans, capture information, and then end the spans. We don't have any plans to make our auto-instrumentors aware of each other, nor is there an obvious path forward in this regard.
  • Different: Again this is expected and a result of the fact that the OpenAI instrumentor patches methods on the openai client from the OpenAI SDK where as the LangChain instrumentor pulls attributes off of LangChain llm "runs". You can expect llm spans from LangChain to likely contain a bit more information since an llm "call" in LangChain could potentially contain arguments that a call to base OpenAI would not.

That the openai instrumentation are not in the correct trace seems related to this, but possibly is a separate issue #1061

Yes both auto instrumentors attempt to pull the active context which may have a span set on it (would become the parent of any spans created after). This is why you can nest auto-instrumented spans in manually instrumented ones as was discussed in #1061.

However, they do not have a shared context manager, so it's possible that the active context from LangChain (set here) is not available in the OpenAI instrumentation. We don't currently have it on our roadmap to support shared context between our auto-instrumentors. We looked into it for python but ran into issues with concurrency, and have not looked into it for JS. If this is something you'd be interested we'd be happy to take a look at an issue!

I'd be curious to hear your use case for using both of our auto-instrumentors in the same app to see if we can help out.

In the mean time you might be able to remove the redundancy by extending a span processor to export only certain spans. We've done something similar in our Vercel AI SDK span processors (here) to allow for filtering of non generative spans.

@codefromthecrypt
Copy link
Author

tl;dr; I'll just not make any example using any instrumentation with langchain as they don't connect traces and many would expect mis-parented traces as bugs.


I think the general opinion I've gathered is that auto-instrumentation or manual, there's no expectation of integrated context being classified as a bug. (e.g. we usually call this broken traces in other projects and there are usually bug fixes to correct span parentage concerns).

Mixing of instrumentators is a norm in opentelemetry, some being vendor-specific and others being from contrib. So, I think this mixed case isn't optimized, by choice, at the moment. So, basically I would suggest to make sure you don't add openai if also langchain.

The multiple instrumentors ending up in different traces, seems an extension of #1061, so basically regardless of manual tracing, otel instrumentors or other instrumentors in this code base, there isn't an expectation of trace propagation. This means causal relationships are not predicatable when using arise instrumentation as would otherwise be expected elsewhere. So, mainly my other takeaway is this, basically if anyone asks about it, refer to that issue, or possibly a document explaining the rationale if that happens one day.

@Parker-Stafford
Copy link
Contributor

tl;dr; I'll just not make any example using any instrumentation with langchain as they don't connect traces and many would expect mis-parented traces as bugs.

I think the general opinion I've gathered is that auto-instrumentation or manual, there's no expectation of integrated context being classified as a bug. (e.g. we usually call this broken traces in other projects and there are usually bug fixes to correct span parentage concerns).

Mixing of instrumentators is a norm in opentelemetry, some being vendor-specific and others being from contrib. So, I think this mixed case isn't optimized, by choice, at the moment. So, basically I would suggest to make sure you don't add openai if also langchain.

The multiple instrumentors ending up in different traces, seems an extension of #1061, so basically regardless of manual tracing, otel instrumentors or other instrumentors in this code base, there isn't an expectation of trace propagation. This means causal relationships are not predicatable when using arise instrumentation as would otherwise be expected elsewhere. So, mainly my other takeaway is this, basically if anyone asks about it, refer to that issue, or possibly a document explaining the rationale if that happens one day.

Hey @codefromthecrypt thanks for the quick response. Maybe I didn't word my previous reply correctly. It does seem to me that our auto-instrumentors should be correctly nested when used together, just that this is not something we've looked into yet. Didn't mean to dismiss your concern! I've created an issue here for tracking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request language: js Related to JavaScript or Typescript integration
Projects
Archived in project
Development

No branches or pull requests

3 participants