OpenAI v2 onboard onto semantic conventions 1.37.0: chat history and other breaking changes #3715
Conversation
github-actions
bot
assigned
codefromthecrypt,
karthikscale3,
lmolkova,
lzchen and
nirga
lmolkova
changed the title
|
As a common package we have https://github.com/open-telemetry/opentelemetry-python-contrib/tree/main/util/opentelemetry-util-genai |
|
thanks a lot, @xrmx! |
zhirafovod
left a comment
zhirafovod
left a comment
There was a problem hiding this comment.
Some minor suggestions, but overall it looks good for this iteration. Thanks for doing it, we can add some notes to follow up on these items in the following pr.
| # `gen_ai_latest_experimental` is not set in the | ||
| # `OTEL_SEMCONV_STABILITY_OPT_IN` environemnt variable. | ||
| # - everything else - don't record content on any signal | ||
| OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT=span |
There was a problem hiding this comment.
Optionally - introduce a new OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT_MODE variable, which will include a list of supported modes - i.e.
spaneventspan,event- sometimes preferred for development/teasting.
Reusing OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT seems fine too.
if we decide to support span,event, it will require either introducing ContentCapturingMode.SPAN_AND_EVENT or supporting a list of capturing modes. May be helper methods like is_capturing_span() and is_capturing_event.
There was a problem hiding this comment.
For now we have two config items in Spring AI prototype:
OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT- (for compatibility) decide whether we capture the chat messages.false- (by default) don't capture any chat messagestrue- capture all chat messages, specific behaviors depend on additional configuration
OTEL_INSTRUMENTATION_GENAI_MESSAGE_CONTENT_CAPTURE_STRATEGY- decide how we capture the chat messages.span-attributes- (by default) capturing them as span attributesevent- capturing them as a eventevent, span-attributes
There was a problem hiding this comment.
@Cirilla-zmh , thanks for sharing!
As a long-time java developer - can't resist the comment VERY_JAVA_STYLE_VERBOSE_EVNIRONMENT_VARIABLE_NAMING 🤣
But seriously
- 👍
span-attributesfor clarity. - 👍 a list of modes "
event, span-attributes, it is quite popular for testing. - 👎 different word ordering in
CAPTURE_MESSAGE_CONTENTandMESSAGE_CONTENT_CAPTURE_STRATEGYin env naming. I would 👍 to reuseOTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENTenv var.
cc: @lmolkova
There was a problem hiding this comment.
As a long-time java developer - can't resist the comment VERY_JAVA_STYLE_VERBOSE_EVNIRONMENT_VARIABLE_NAMING 🤣
Ahh, that's it...🤣 But it comes from the requirement of multi-language compatibility and isolation, it's more clear when transported to a property key: otel.instrumentation.genai.capture_messages_content.
For compatibility, I tend to add another config key to control the behavior of the messages capturing - the naming could be discussed again.
| return traced_method | ||
|
|
||
|
|
||
| def _record_metrics( |
There was a problem hiding this comment.
Ideally, we move _record_metrics() and set_response_attributes to the utils.py
| "", | ||
| tracer_provider, | ||
| schema_url=Schemas.V1_28_0.value, | ||
| schema_url="https://opentelemetry.io/schemas/1.37.0", # TODO: Schemas.V1_37_0.value, |
There was a problem hiding this comment.
we probably need a telemetry test for the attributes in this telemetry.
@xrmx , I would propose we merge this one before the common library is ready. It will provide testing needed to switch to the new library. Also, please review the PRs which are pre-requirement to switching to the util library. |
8 participants
Implements new semconv 1.37.0 behind
OTEL_SEMCONV_STABILITY_OPT_IN = gen_ai_latest_experimentalopt-in.Depends on #3709 (common code for
OTEL_SEMCONV_STABILITY_OPT_IN) and open-telemetry/opentelemetry-python#4731 (new semconv package).TODOs:
ContentCapturingMode(we should put it into https://github.com/open-telemetry/opentelemetry-python-contrib/tree/main/util/opentelemetry-util-genai, see @xrmx comment below)