Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Do not unconditionally set OTEL_INSTRUMENTATION_GENAI_EMIT_EVENT during instrumentation.
Original file line number Diff line number Diff line change
Expand Up @@ -842,7 +842,6 @@ def instrument_generate_content(
telemetry_handler: TelemetryHandler,
generate_content_config_key_allowlist: AllowList,
) -> object:
os.environ["OTEL_INSTRUMENTATION_GENAI_EMIT_EVENT"] = "true"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this was added because we want to emit the event regardless of the CONTENT_CAPUTRE mode ( we want CONTENT_CAPTURE to just impact whether content is captured on events/spans) where as I think other instrumentations were using the content_capture mode / env var to determine whether an event is emitted at all -- can you check if that is still true or not ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Hi @DylanRussell, thank you for the feedback. I investigated the entire codebase across all instrumentations and opentelemetry-util-genai. Here is what I verified:

  1. How other instrumentations operate:
    All instrumentations using InferenceInvocation and TelemetryHandler (openai, bedrock, langchain, smolagents, portkey, and google-genai) delegate event emission entirely to InferenceInvocation._maybe_create_event(), which calls should_emit_event(). None of the other instrumentations set os.environ["OTEL_INSTRUMENTATION_GENAI_EMIT_EVENT"].

  2. The contract in opentelemetry-util-genai/src/opentelemetry/util/genai/utils.py:
    should_emit_event() explicitly implements:

  • If OTEL_INSTRUMENTATION_GENAI_EMIT_EVENT is set, user preference takes highest priority ("true" -> True, "false" -> False).
  • If OTEL_INSTRUMENTATION_GENAI_EMIT_EVENT is not set, it defaults based on OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT:
    • NO_CONTENT or SPAN_ONLY: defaults to False.
    • EVENT_ONLY or SPAN_AND_EVENT: defaults to True.
  1. Content capture vs event emission:
    In InferenceInvocation._maybe_create_event(), message content serialization is decoupled via get_content_attributes():
  • When OTEL_INSTRUMENTATION_GENAI_EMIT_EVENT=true is set, should_emit_event() returns True, so the gen_ai.client.inference.operation.details event is emitted.
  • If CONTENT_CAPTURE is NO_CONTENT, get_content_attributes() omits input_messages, output_messages, and system_instruction from the event payload, while preserving top-level operation attributes.
  • Thus, users can already emit events without content by setting OTEL_INSTRUMENTATION_GENAI_EMIT_EVENT=true and OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT=NO_CONTENT.
  1. Why hardcoding os.environ["OTEL_INSTRUMENTATION_GENAI_EMIT_EVENT"] = "true" was problematic:
  • In generate_content.py, mutating os.environ directly at instrumentation time overrides any explicit user setting (such as OTEL_INSTRUMENTATION_GENAI_EMIT_EVENT=false) and mutates the global process environment for all instrumentations.
  • Removing the hardcoded assignment brings google-genai into exact alignment with all other instrumentations and restores respect for user-configured environment variables.

Let me know if you would prefer google-genai to retain a specific default when no environment variables are set, or if aligning with the shared should_emit_event() contract across all instrumentations is the desired direction.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yeah that will be a breaking change for users of this instrumentation.. i don't like that the CAPTURE_CONTENT env var is responsible for event emission.. that is not intuitive.. i'd prefer we got rid of that logic instead...

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Hi @DylanRussell,

Thank you for the review and for calling this out. You have raised two critical points:

  1. Semantic Separation of Concerns & Public Documentation:
    You are completely right that OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT should not be governing event emission. To answer your question from the issue: this behavior is NOT documented or required anywhere in the public OpenTelemetry Semantic Conventions specification. The official GenAI semconv defines payload privacy (capturing prompt/completion content) and event emission (gen_ai.client.inference.operation.details) as orthogonal concepts. An event can and should record operational telemetry (model name, token counts, response status) even when content capture is off. The fallback was introduced internally in Python PR #3994 (opentelemetry-util-genai) as a local heuristic.

  2. Backwards Compatibility for google-genai:
    Removing the hardcoded os.environ assignment exposes that utility fallback, causing google-genai to stop emitting events by default for unconfigured users, which is indeed a breaking change for existing users of this package. At the same time, the original issue (google-genai: instrument_generate_content() unconditionally sets OTEL_INSTRUMENTATION_GENAI_EMIT_EVENT=true, overriding config and user setting #619) arose because unconditionally writing os.environ["OTEL_INSTRUMENTATION_GENAI_EMIT_EVENT"] = "true" clobbers explicit user opt-outs (e.g. when an operator or framework explicitly sets OTEL_INSTRUMENTATION_GENAI_EMIT_EVENT=false) and leaks across the entire process.

To resolve this cleanly, there are two paths:

Path A (Scoped, non-breaking fix for google-genai in this PR):
We preserve default event emission for google-genai when OTEL_INSTRUMENTATION_GENAI_EMIT_EVENT is unset (for example, using os.environ.setdefault("OTEL_INSTRUMENTATION_GENAI_EMIT_EVENT", "true") or resolving defaults in-memory), so existing users continue getting events without any disruption, while strictly honoring explicit user opt-outs like OTEL_INSTRUMENTATION_GENAI_EMIT_EVENT="false".

Path B (Decoupling in opentelemetry-util-genai):
As you suggested, we remove the CAPTURE_CONTENT fallback logic from should_emit_event() in opentelemetry-util-genai. If we do that, what should the global default for should_emit_event() be when OTEL_INSTRUMENTATION_GENAI_EMIT_EVENT is unset? Should it default to True (events on by default across all instrumentations, with message content gated by CAPTURE_CONTENT), or False?

Please let me know which direction you would prefer, and I will be happy to update this PR or submit a PR for opentelemetry-util-genai.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

My preference is to default it to True and remove the capture content fallback logic.. Someone else should weigh in on this though

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks @DylanRussell, that makes total sense. Defaulting event emission to True aligns well with standard telemetry expectations while keeping content payload capture decoupled under CAPTURE_CONTENT. Let's see if other maintainers / semconv WG members want to chime in on the global default. In the meantime, I can prepare a PR against opentelemetry-util-genai implementing that decoupling if you'd like, or wait for consensus here.

snapshot = _MethodsSnapshot()
wrapped = wrap_function_wrapper(
"google.genai.models",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

"""Tests for GoogleGenAiSdkInstrumentor."""

import os

from google.genai.models import AsyncModels, Models

from opentelemetry.instrumentation.google_genai import (
Expand All @@ -14,6 +16,9 @@
InteractionsResource,
)
from opentelemetry.test_util_genai.instrumentor import instrument
from opentelemetry.util.genai.environment_variables import (
OTEL_INSTRUMENTATION_GENAI_EMIT_EVENT,
)


def test_co_filename_on_wrapped_functions(
Expand Down Expand Up @@ -58,3 +63,29 @@ def test_co_filename_on_wrapped_functions(
), (
f"Expected opentelemetry/instrumentation/google_genai removed from {co_filename} upon uninstrument"
)


def test_instrument_does_not_mutate_emit_event_env(
monkeypatch, tracer_provider, logger_provider, meter_provider
):
monkeypatch.delenv(OTEL_INSTRUMENTATION_GENAI_EMIT_EVENT, raising=False)
with instrument(
GoogleGenAiSdkInstrumentor(),
tracer_provider=tracer_provider,
logger_provider=logger_provider,
meter_provider=meter_provider,
):
assert OTEL_INSTRUMENTATION_GENAI_EMIT_EVENT not in os.environ


def test_instrument_preserves_explicit_emit_event_env(
monkeypatch, tracer_provider, logger_provider, meter_provider
):
monkeypatch.setenv(OTEL_INSTRUMENTATION_GENAI_EMIT_EVENT, "false")
with instrument(
GoogleGenAiSdkInstrumentor(),
tracer_provider=tracer_provider,
logger_provider=logger_provider,
meter_provider=meter_provider,
):
assert os.environ.get(OTEL_INSTRUMENTATION_GENAI_EMIT_EVENT) == "false"