-
Notifications
You must be signed in to change notification settings - Fork 509
Remove 3rd party patterns from the sdk #1047
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
Conversation
Codecov ReportAttention: Patch coverage is 📢 Thoughts on this report? Let us know! |
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.
Pull Request Overview
This PR removes the third-party OpenTelemetry instrumentation code for OpenAI from third_party/ and replaces it with an in-house implementation under agentops/instrumentation/openai. It also updates tests to point at the new wrapper infrastructure and adjusts the build configuration to stop packaging the deleted code.
- Deleted legacy third-party OpenTelemetry instrumentation for OpenAI under
third_party/opentelemetry - Added full in-house wrappers and instrumentor in
agentops/instrumentation/openai - Updated tests and
pyproject.tomlto reference the new modules
Reviewed Changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| third_party/opentelemetry/instrumentation/openai/** | Entire directory removed to drop third-party code |
| agentops/instrumentation/openai/** | New in-house instrumentation wrappers and instrumentor |
| tests/unit/instrumentation/openai_core/test_instrumentor.py | Updated import paths and assertions for new wrappers |
| pyproject.toml | Removed wheel inclusion of third_party |
Comments suppressed due to low confidence (1)
agentops/instrumentation/openai/v0.py:93
- This else appears to pair with the first
if is_metrics_enabled()rather than the embeddings metrics block. After removing the guard, it now attempts to unpack eight variables where only two were set. Consider restoring the conditional or realigning the indentation so the else correctly covers embedding metric initialization.
else:
|
This is clean! |
📥 Pull Request
📘 Description
Closes #1037
🧪 Testing
https://gist.github.com/Dwij1704/f4b3a9c1d34f3a956d0abfd7b7dffe39