-
Notifications
You must be signed in to change notification settings - Fork 798
Add shared Dataclass Types for GenAi instrumentations #3718
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
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.
Huge thanks, it'd be very useful in all GenAI instrumentations!
There could be a few other things we can put here - comments, validation, serialization helpers, but all of it can be added incrementally.
This LGTM but is there an issue or draft PR to show what is the plan for these dataclasses? |
Check out #3709 for an example - basically the intent is just to use them as an intermediate store of the data before the export of events/spans |
Got it. I was thinking this util would provide an API to actually emit the telemetry since there is a lot of shared boilerplate. Something like class GenAiInstrumentation:
def __init__(self, meter, tracer, logger): ...
def handle_completion(
self,
input: InputMessage,
outputs: OutputMessages,
system: SystemInstruction,
) -> None:
"""Emits telemetry in the format chosen by the user
depending on `OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT` and `OTEL_SEMCONV_STABILITY_OPT_IN` uploader configuration (TBD)
"""
### foollm_instrumentation.py
response = call_llm(input)
# convert to OTel format
input_messages, output_messages, system_instruction = massage_data(input, response)
self._instrumentation_util.handle_completion(input_messages, output_messages, system_instruction) Can definitely handle it at a later point but I'm guessing #3709 pretty much already does that? |
util/opentelemetry-util-genai/src/opentelemetry/util/genai/types.py
Outdated
Show resolved
Hide resolved
I did not write the PR that way, I branched on the env var flag much earlier in the code. I think massaging the data into the |
Discussed offline. This was regarding when the instrumentation is not opted into the new conventions. When they choose new conventions, we can have some shared util to avoid boilerplate. This LGTM to merge. |
Description
Add data classes which correspond to the types in the GenAI json schemas listed in https://github.com/open-telemetry/semantic-conventions/tree/main/docs/gen-ai
There's probably a lot of ways to do this.. Open to suggestions on how to keep these in-sync with the json schemas or to other approaches.
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Just adding types..
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.