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

Allow instrumenting OpenAI/Anthropic client classes or modules #191

Merged
merged 8 commits into from
May 20, 2024

Conversation

alexmojaki
Copy link
Contributor

@alexmojaki alexmojaki commented May 18, 2024

For a user who wanted to just logfire.instrument_openai() without passing a client, since they want to instrument langchain which doesn't expose/accept an openai client object: https://pydanticlogfire.slack.com/archives/C06EDRBSAH3/p1715977569378699

Copy link

cloudflare-pages bot commented May 19, 2024

Deploying logfire-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 76f8c45
Status: ✅  Deploy successful!
Preview URL: https://ef37eff9.logfire-docs.pages.dev
Branch Preview URL: https://alex-instrument-llm-class.logfire-docs.pages.dev

View logs

Copy link

codecov bot commented May 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@alexmojaki alexmojaki marked this pull request as ready for review May 19, 2024 16:02
@alexmojaki alexmojaki requested a review from Kludex May 19, 2024 16:02
@alexmojaki
Copy link
Contributor Author

@willbakst might be interested in reviewing too.

Copy link
Contributor

@willbakst willbakst left a comment

Choose a reason for hiding this comment

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

LGTM

"""Instruments the provided `client` with `logfire`."""
"""Instruments the provided `client` (or clients) with `logfire`.

`client` can be:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
`client` can be:
The `client` can be:

Comment on lines 31 to 33
- a single client instance, e.g. an instance of `openai.OpenAI`.
- a class of a client
- an iterable of clients/classes.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- a single client instance, e.g. an instance of `openai.OpenAI`.
- a class of a client
- an iterable of clients/classes.
- a single client instance, e.g. an instance of `openai.OpenAI`;
- a class of a client;
- an iterable of clients/classes.

@alexmojaki alexmojaki merged commit 4006a2b into main May 20, 2024
13 checks passed
@alexmojaki alexmojaki deleted the alex/instrument-llm-class branch May 20, 2024 14:44
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