switch to 2.18.1, and switch openai to upstream#763
Conversation
|
The idea is that the issue will be solved, right? Otherwise, we might have to mark the update as a breaking change where relevant... |
Co-authored-by: Fabrizio Ferri-Benedetti <fabri.ferribenedetti@elastic.co>
Co-authored-by: Fabrizio Ferri-Benedetti <fabri.ferribenedetti@elastic.co>
Co-authored-by: Fabrizio Ferri-Benedetti <fabri.ferribenedetti@elastic.co>
Co-authored-by: Fabrizio Ferri-Benedetti <fabri.ferribenedetti@elastic.co>
This will be a permanent breaking change. If we merge this, I'll open PRs for the docs for the next version |
SylvainJuge
left a comment
SylvainJuge
left a comment
There was a problem hiding this comment.
LGTM, I think we just need to have an issue that describes the follow-up tasks for the removal of openai.
| // OTEL_INSTRUMENTATION_OPENAI=false | ||
| // OTEL_INSTRUMENTATION_OPENAI_CLIENT=true |
There was a problem hiding this comment.
What is the difference between those two variables ? are there two instrumentations or at least two config options in upstream ?
There was a problem hiding this comment.
the upstream one uses OTEL_INSTRUMENTATION_OPENAI, while our implementation uses OTEL_INSTRUMENTATION_OPENAI_CLIENT. If the upstream one had used the same var, it wouldn't have been a breaking change and we could have just removed our implementation
There was a problem hiding this comment.
If I understand correctly disabling those tests is required because now the upstream instrumentation is used. Do we already have an issue that describes the deprecation and removal steps ? Aligning the removal with the update to 3.x upstream seems a good option as it already forces to do a new major.
There was a problem hiding this comment.
No issue opened yet, I will open when this is merged
3 participants
upstream agent now 2.18.1 (needs some small internal test changes)
this distributions openai-client is turned off, leaving the upstream openai instrumentation to do the work (needed a couple of tests disabled)