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

Add documentation about LlamaIndex #695

Closed
wants to merge 1 commit into from
Closed

Conversation

Kludex
Copy link
Member

@Kludex Kludex commented Dec 18, 2024

No description provided.

Copy link
Contributor

@hyperlint-ai hyperlint-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The style guide flagged several spelling errors that seemed like false positives. We skipped posting inline suggestions for the following words:

  • LLMs

Copy link

Deploying logfire-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0ce3ae5
Status: ✅  Deploy successful!
Preview URL: https://dc9b9449.logfire-docs.pages.dev
Branch Preview URL: https://add-docs-about-llamaindex.logfire-docs.pages.dev

View logs

Copy link

codecov bot commented Dec 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (7e6b140) to head (0ce3ae5).
Report is 60 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #695   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          137       137           
  Lines        10920     10929    +9     
  Branches      1520      1524    +4     
=========================================
+ Hits         10920     10929    +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


# Get response
response = query_engine.query('How can I use Pydantic models?')
print(str(response))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should it show something below this? 😄 Not sure what the response is

Copy link
Contributor

@dmontagu dmontagu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally this seems good, but should we be instrumenting the things LlamaIndex does that aren't just making calls to the LLM providers?

Like, should we be instrumenting/discussing how to instrument the web page reading to build the index? Is there any instrumentation for llama_index that goes beyond the instrumentation of the LLM calls?

It would be nice to explain/show what sorts of things will show up in logfire if you enable the LLM instrumentation and use llama_index, like, what do you see in logfire as a result of that query_engine.query call?

But I mean I think this is a reasonable starting point, I don't see a reason to delay merging this, even if we can add more.

Comment on lines +5 to +6
**Logfire** supports instrumenting calls to different LLMs with one extra line of code.
Since LlamaIndex supports multiple LLMs, you can use **Logfire** to instrument calls to those LLMs.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
**Logfire** supports instrumenting calls to different LLMs with one extra line of code.
Since LlamaIndex supports multiple LLMs, you can use **Logfire** to instrument calls to those LLMs.
The way we recommend instrumenting LlamaIndex is to instrument the underlying LLM, and to rely on that instrumentation to ensure that calls made by LlamaIndex end up in Logfire.
**Logfire** supports instrumenting calls to different LLMs with one extra line of code.
Since LlamaIndex supports multiple LLMs, you can use **Logfire** to instrument calls to those LLMs.

I think we can be a bit more explicit here that we aren't specifically instrumenting LlamaIndex, we are just relying on its usage of something that is instrumented. That said, if there is an actual LlamaIndex instrumentation maybe we should add that. Not sure if there is or if the only out-of-the-box thing available is the LLM instrumentation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added that one:

Screenshot 2024-12-23 at 11 42 04

@Kludex Kludex closed this Jan 3, 2025
@Kludex Kludex deleted the add-docs-about-llamaindex branch January 3, 2025 08:43
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.

3 participants