Skip to content

Conversation

@Dwij1704
Copy link
Member

@Dwij1704 Dwij1704 commented Jun 5, 2025

📥 Pull Request

📘 Description
Closes #1037

🧪 Testing
https://gist.github.com/Dwij1704/f4b3a9c1d34f3a956d0abfd7b7dffe39

@Dwij1704 Dwij1704 requested a review from tcdent June 6, 2025 15:53
@dot-agi dot-agi linked an issue Jun 9, 2025 that may be closed by this pull request
@Dwij1704 Dwij1704 requested a review from Copilot June 9, 2025 22:57
Copy link
Contributor

Copilot AI left a 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.toml to 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:

@tcdent
Copy link
Contributor

tcdent commented Jun 10, 2025

This is clean!

@Dwij1704 Dwij1704 merged commit b1f0b9b into main Jun 10, 2025
9 of 10 checks passed
@Dwij1704 Dwij1704 deleted the openai-first-party branch June 10, 2025 21:22
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.

remove 3rd party patterns from the sdk investigate ways to clean sdk

3 participants