Skip to content

Conversation

argaj
Copy link

@argaj argaj commented Sep 22, 2025

Description

Accomodation of semantic conventions changes made in open-telemetry/semantic-conventions#2179.

Also, when logging the completion details, the upload hook is called, so if user has configured OTEL_INSTRUMENTATION_GENAI_UPLOAD_BASE_PATH and OTEL_INSTRUMENTATION_GENAI_UPLOAD_HOOK env vars, completion details will also be logged as refs (if appropriate capture content env var is set).

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

WIP, only some manual tests for now.

Does This PR Require a Core Repo Change?

No?

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

Copy link

CLA Not Signed

]:
attributes.update(completion_details_attributes)
event = Event(name="gen_ai.completion.details", attributes=attributes)
hook = load_upload_hook()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the instrumentor class should do a DI pattern for this. It probably should accept an optional upload hook as a parameter to instrument(), otherwise use load_upload_hook().

Comment on lines +466 to +474
ContentCapturingMode.SPAN_ONLY,
ContentCapturingMode.SPAN_AND_EVENT,
]:
span = trace.get_current_span()
span.set_attributes(completion_details_attributes)
if self._content_recording_enabled in [
ContentCapturingMode.EVENT_ONLY,
ContentCapturingMode.SPAN_AND_EVENT,
]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DylanRussell should we still do this if there is an upload hook? One practical concern for GCP logging exporter is that if you include the JSON in the log directly and it exceeds the size limit (256kB) the whole log gets dropped. Since the upload hook works on the same log, that would mean everything gets dropped.

Alternatively, maybe we could emit two separate logs to avoid this problem

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.

6 participants