-
Notifications
You must be signed in to change notification settings - Fork 55
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
feat(smolagents): updates to latest and makes examples use opentelemetry-instrument #1277
Conversation
fyi @mikeldking I tested all of these with phoenix. works fine "on my laptop" ™️ |
buffed out a couple other drifts and documented them in the PR desc |
Nice! you're on a roll @codefromthecrypt ! 😄 Can we leave at least one example, or perhaps add to the readme that you could instrument without opentelemtry-instrument, in case you need some specific customization. Correct me if I'm wrong, but I think there are certain things that you can't do with the no-code approach (e.g. batch vs simple span processor?). |
hey @nate-mar, and thanks for the feedback! I added a ENV var that exports spans every second. This should help for the higher latency examples, which before this would only flush when the program completes or 5s whichever comes first. I also added a note to the README about what to copy/paste alternatively for manual style. One hazard of manual is that if you end up adding metrics, people will have to remember to update their code to add that, too. Next q is should I make corresponding updates to the main smolagents instrumentation README? It currently has manual tracing setup there. Besides fuzz I've left that one alone so far. |
Awesome, thanks! This is great! Sure, go for it re: updating the main smolagents readme ! I would only just mention the same thing about including a blurb/section about the manual config for additional flexibility if needed. and noted about the metrics! |
Ok I pared down the instructions to least necessary. The top-level example needs no special env variables, due to not using openai etc, so that's a bit simpler. PTAL! |
@@ -14,42 +14,57 @@ pip install openinference-instrumentation-smolagents | |||
|
|||
## Quickstart | |||
|
|||
This quickstart shows you how to instrument your guardrailed LLM application | |||
This quickstart shows you how to instrument your LLM agent application. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed this copy/paste elsewhere, so will try to scrub for that if I end up nudging another package.
57289ea
to
037628b
Compare
@@ -95,6 +95,15 @@ def openai_api_key(monkeypatch: pytest.MonkeyPatch) -> str: | |||
return api_key | |||
|
|||
|
|||
class TestInstrumentor: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added this to make sure things don't drift and we lose registration by accident
Signed-off-by: Adrian Cole <[email protected]>
...strumentation-smolagents/tests/openinference/instrumentation/smolagents/test_instrumentor.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing! thanks again!
thanks! great timing as I'm online temporarily today and tomorrow |
This update smolagents code where recent versions drifted:
ManagedAgent
max_tokens
from all modelsThis also polishes the examples directory and adds a README there, some of the notable changes below:
opentelemetry-instrument
, so that you can easily copy/paste examples without needing to edit in instrumentationopentelemetry-instrument
entrypoint and._tracer
is aOITracer
I can do make the main README, use opentelemetry-instrument, too if this seems alright.