Use an allowlist for dynamic keys per PR feedback.

Author: michaelsafyan

instrumentation-genai/opentelemetry-instrumentation-google-genai/src/opentelemetry/instrumentation/google_genai/allowlist_util.py

@@ -0,0 +1,61 @@
+# Copyright The OpenTelemetry Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import os
+from typing import Optional, Set, Callable, List, Union
+
+ALLOWED = True
+DENIED = False
+
+def _parse_env_list(s: str) -> Set[str]:
+    result = set()
+    for entry in s.split(','):
+        stripped_entry = entry.strip()
+        if not stripped_entry:
+            continue
+        result.add(stripped_entry)
+    return result
+
+
+class AllowList:
+
+    def __init__(
+        self,
+        includes: Optional[Union[Set[str], List[str]]] = None,
+        excludes: Optional[Union[Set[str], List[str]]] = None,
+        if_none_match: Optional[Callable[str, bool]] = None):
+        self._includes = set(includes or [])
+        self._excludes = set(excludes or [])
+        self._include_all = '*' in self._includes
+        self._exclude_all = '*' in self._excludes
+        assert (not self._include_all) or (not self._exclude_all), "Can't have '*' in both includes and excludes."
+
+    def allowed(self, x: str):
+        if self._exclude_all:
+            return x in self._includes
+        if self._include_all:
+            return x not in self._excludes
+        return (x in self._includes) and (x not in self._excludes)
+
+    @staticmethod
+    def from_env(
+        includes_env_var: str,
+        excludes_env_var: Optional[str] = None):
+      includes = _parse_env_list(os.getenv(includes_env_var) or '')
+      excludes = set()
+      if excludes_env_var:
+        excludes = _parse_env_list(os.getenv(excludes_env_var) or '')
+      return AllowList(
+        includes=includes,
+        excludes=excludes)

instrumentation-genai/opentelemetry-instrumentation-google-genai/src/opentelemetry/instrumentation/google_genai/generate_content.py

@@ -39,6 +39,7 @@
 )
 from opentelemetry.semconv.attributes import error_attributes
 
+from .allowlist_util import AllowList
 from .custom_semconv import CUSTOM_LLM_REQUEST_PREFIX
 from .dict_util import flatten_dict
 from .flags import is_content_recording_enabled
@@ -140,7 +141,7 @@ def _to_dict(value: object):
 
 
 def _add_request_options_to_span(
-    span, config: Optional[GenerateContentConfigOrDict]
+    span, config: Optional[GenerateContentConfigOrDict], allow_list: AllowList
 ):
     if config is None:
         return
@@ -152,7 +153,9 @@ def _add_request_options_to_span(
     # Automatically derive attributes from the contents of the
     # config object. This ensures that all relevant parameters
     # are captured in the telemetry data (except for those
-    # that are excluded via "exclude_keys").
+    # that are excluded via "exclude_keys"). Dynamic attributes (those
+    # starting with "gcp.gen_ai." instead of simply "gen_ai.request.")
+    # are filtered with the "allow_list" before inclusion in the span.
     attributes = flatten_dict(
         _to_dict(config),
         # A custom prefix is used, because the names/structure of the
@@ -162,9 +165,6 @@ def _add_request_options_to_span(
             # System instruction can be overly long for a span attribute.
             # Additionally, it is recorded as an event (log), instead.
             "gcp.gen_ai.request.system_instruction",
-            # Headers could include sensitive information, therefore it is
-            # best that we not record these options.
-            "gcp.gen_ai.request.http_options.headers",
         ],
         # Although a custom prefix is used by default, some of the attributes
         # are captured in common, standard, Semantic Conventions. For the
@@ -184,7 +184,11 @@ def _add_request_options_to_span(
             "gcp.gen_ai.request.seed": gen_ai_attributes.GEN_AI_REQUEST_SEED,
         },
     )
-    span.set_attributes(attributes)
+    for key, value in attributes.items():
+        if key.startswith(CUSTOM_LLM_REQUEST_PREFIX) and not allow_list.allowed(key):
+            # The allowlist is used to control inclusion of the dynamic keys.
+            continue
+        span.set_attribute(key, value)
 
 
 def _get_response_property(response: GenerateContentResponse, path: str):
@@ -206,6 +210,7 @@ def __init__(
         models_object: Union[Models, AsyncModels],
         otel_wrapper: OTelWrapper,
         model: str,
+        generate_content_config_key_allowlist: Optional[AllowList] = None,
     ):
         self._start_time = time.time_ns()
         self._otel_wrapper = otel_wrapper
@@ -218,6 +223,7 @@ def __init__(
         self._content_recording_enabled = is_content_recording_enabled()
         self._response_index = 0
         self._candidate_index = 0
+        self._generate_content_config_key_allowlist = generate_content_config_key_allowlist or AllowList()
 
     def start_span_as_current_span(
         self, model_name, function_name, end_on_exit=True
@@ -240,7 +246,7 @@ def process_request(
         config: Optional[GenerateContentConfigOrDict],
     ):
         span = trace.get_current_span()
-        _add_request_options_to_span(span, config)
+        _add_request_options_to_span(span, config, self._generate_content_config_key_allowlist)
         self._maybe_log_system_instruction(config=config)
         self._maybe_log_user_prompt(contents)
 
@@ -501,7 +507,7 @@ def _record_duration_metric(self):
 
 
 def _create_instrumented_generate_content(
-    snapshot: _MethodsSnapshot, otel_wrapper: OTelWrapper
+    snapshot: _MethodsSnapshot, otel_wrapper: OTelWrapper, generate_content_config_key_allowlist: Optional[AllowList] = None
 ):
     wrapped_func = snapshot.generate_content
 
@@ -515,7 +521,7 @@ def instrumented_generate_content(
         **kwargs: Any,
     ) -> GenerateContentResponse:
         helper = _GenerateContentInstrumentationHelper(
-            self, otel_wrapper, model
+            self, otel_wrapper, model, generate_content_config_key_allowlist=generate_content_config_key_allowlist,
         )
         with helper.start_span_as_current_span(
             model, "google.genai.Models.generate_content"
@@ -541,7 +547,7 @@ def instrumented_generate_content(
 
 
 def _create_instrumented_generate_content_stream(
-    snapshot: _MethodsSnapshot, otel_wrapper: OTelWrapper
+    snapshot: _MethodsSnapshot, otel_wrapper: OTelWrapper, generate_content_config_key_allowlist: Optional[AllowList] = None
 ):
     wrapped_func = snapshot.generate_content_stream
 
@@ -555,7 +561,7 @@ def instrumented_generate_content_stream(
         **kwargs: Any,
     ) -> Iterator[GenerateContentResponse]:
         helper = _GenerateContentInstrumentationHelper(
-            self, otel_wrapper, model
+            self, otel_wrapper, model, generate_content_config_key_allowlist=generate_content_config_key_allowlist
         )
         with helper.start_span_as_current_span(
             model, "google.genai.Models.generate_content_stream"
@@ -581,7 +587,7 @@ def instrumented_generate_content_stream(
 
 
 def _create_instrumented_async_generate_content(
-    snapshot: _MethodsSnapshot, otel_wrapper: OTelWrapper
+    snapshot: _MethodsSnapshot, otel_wrapper: OTelWrapper, generate_content_config_key_allowlist: Optional[AllowList] = None
 ):
     wrapped_func = snapshot.async_generate_content
 
@@ -595,7 +601,7 @@ async def instrumented_generate_content(
         **kwargs: Any,
     ) -> GenerateContentResponse:
         helper = _GenerateContentInstrumentationHelper(
-            self, otel_wrapper, model
+            self, otel_wrapper, model, generate_content_config_key_allowlist=generate_content_config_key_allowlist,
         )
         with helper.start_span_as_current_span(
             model, "google.genai.AsyncModels.generate_content"
@@ -622,7 +628,7 @@ async def instrumented_generate_content(
 
 # Disabling type checking because this is not yet implemented and tested fully.
 def _create_instrumented_async_generate_content_stream(  # type: ignore
-    snapshot: _MethodsSnapshot, otel_wrapper: OTelWrapper
+    snapshot: _MethodsSnapshot, otel_wrapper: OTelWrapper, generate_content_config_key_allowlist: Optional[AllowList] = None
 ):
     wrapped_func = snapshot.async_generate_content_stream
 
@@ -636,7 +642,7 @@ async def instrumented_generate_content_stream(
         **kwargs: Any,
     ) -> Awaitable[AsyncIterator[GenerateContentResponse]]:  # type: ignore
         helper = _GenerateContentInstrumentationHelper(
-            self, otel_wrapper, model
+            self, otel_wrapper, model, generate_content_config_key_allowlist=generate_content_config_key_allowlist
         )
         with helper.start_span_as_current_span(
             model,
@@ -680,20 +686,22 @@ def uninstrument_generate_content(snapshot: object):
     snapshot.restore()
 
 
-def instrument_generate_content(otel_wrapper: OTelWrapper) -> object:
+def instrument_generate_content(
+    otel_wrapper: OTelWrapper,
+    generate_content_config_key_allowlist: Optional[AllowList]=None) -> object:
     snapshot = _MethodsSnapshot()
     Models.generate_content = _create_instrumented_generate_content(
-        snapshot, otel_wrapper
+        snapshot, otel_wrapper, generate_content_config_key_allowlist=generate_content_config_key_allowlist,
     )
     Models.generate_content_stream = (
-        _create_instrumented_generate_content_stream(snapshot, otel_wrapper)
+        _create_instrumented_generate_content_stream(snapshot, otel_wrapper, generate_content_config_key_allowlist=generate_content_config_key_allowlist)
     )
     AsyncModels.generate_content = _create_instrumented_async_generate_content(
-        snapshot, otel_wrapper
+        snapshot, otel_wrapper, generate_content_config_key_allowlist=generate_content_config_key_allowlist
     )
     AsyncModels.generate_content_stream = (
         _create_instrumented_async_generate_content_stream(
-            snapshot, otel_wrapper
+            snapshot, otel_wrapper, generate_content_config_key_allowlist=generate_content_config_key_allowlist,
         )
     )
     return snapshot

instrumentation-genai/opentelemetry-instrumentation-google-genai/src/opentelemetry/instrumentation/google_genai/instrumentor.py

@@ -12,13 +12,14 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from typing import Any, Collection
+from typing import Any, Collection, Optional
 
 from opentelemetry._events import get_event_logger_provider
 from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
 from opentelemetry.metrics import get_meter_provider
 from opentelemetry.trace import get_tracer_provider
 
+from .allowlist_util import AllowList
 from .generate_content import (
     instrument_generate_content,
     uninstrument_generate_content,
@@ -27,8 +28,11 @@
 
 
 class GoogleGenAiSdkInstrumentor(BaseInstrumentor):
-    def __init__(self):
+    def __init__(self, generate_content_config_key_allowlist: Optional[AllowList] = None):
         self._generate_content_snapshot = None
+        self._generate_content_config_key_allowlist = (generate_content_config_key_allowlist or AllowList.from_env(
+            'OTEL_GOOGLE_GENAI_GENERATE_CONTENT_CONFIG_INCLUDES',
+            excludes_env_var='OTEL_GOOGLE_GENAI_GENERATE_CONTENT_CONFIG_EXCLUDES'))
 
     # Inherited, abstract function from 'BaseInstrumentor'. Even though 'self' is
     # not used in the definition, a method is required per the API contract.
@@ -49,8 +53,8 @@ def _instrument(self, **kwargs: Any):
             meter_provider=meter_provider,
         )
         self._generate_content_snapshot = instrument_generate_content(
-            otel_wrapper
-        )
+            otel_wrapper,
+            generate_content_config_key_allowlist=self._generate_content_config_key_allowlist)
 
     def _uninstrument(self, **kwargs: Any):
         uninstrument_generate_content(self._generate_content_snapshot)

instrumentation-genai/opentelemetry-instrumentation-google-genai/tests/common/base.py

@@ -33,11 +33,15 @@ def setUp(self):
         self._client = None
         self._uses_vertex = False
         self._credentials = FakeCredentials()
+        self._instrumentor_args = {}
 
     def _lazy_init(self):
-        self._instrumentation_context = InstrumentationContext()
+        self._instrumentation_context = InstrumentationContext(**self._instrumentor_args)
         self._instrumentation_context.install()
 
+    def set_instrumentor_constructor_kwarg(self, key, value):
+        self._instrumentor_args[key] = value
+
     @property
     def client(self):
         if self._client is None:

instrumentation-genai/opentelemetry-instrumentation-google-genai/tests/common/instrumentation_context.py

@@ -18,8 +18,8 @@
 
 
 class InstrumentationContext:
-    def __init__(self):
-        self._instrumentor = GoogleGenAiSdkInstrumentor()
+    def __init__(self, **kwargs):
+        self._instrumentor = GoogleGenAiSdkInstrumentor(**kwargs)
 
     def install(self):
         self._instrumentor.instrument()

instrumentation-genai/opentelemetry-instrumentation-google-genai/tests/generate_content/test_config_span_attributes.py

@@ -12,8 +12,11 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from google.genai.types import GenerateContentConfig
+import os
+from unittest import mock
 
+from opentelemetry.instrumentation.google_genai.allowlist_util import AllowList
+from google.genai.types import GenerateContentConfig
 from .base import TestCase
 
 
@@ -93,6 +96,7 @@ def test_option_reflected_to_span_attribute_top_p(self):
         span = self.generate_and_get_span(config={"top_p": 10})
         self.assertEqual(span.attributes["gen_ai.request.top_p"], 10)
 
+    @mock.patch.dict(os.environ, {"OTEL_GOOGLE_GENAI_GENERATE_CONTENT_CONFIG_INCLUDES": "*"})
     def test_option_not_reflected_to_span_attribute_system_instruction(self):
         span = self.generate_and_get_span(
             config={"system_instruction": "Yadda yadda yadda"}
@@ -106,27 +110,38 @@ def test_option_not_reflected_to_span_attribute_system_instruction(self):
             if isinstance(value, str):
                 self.assertNotIn("Yadda yadda yadda", value)
 
-    def test_option_not_reflected_to_span_attribute_http_headers(self):
+    @mock.patch.dict(os.environ, {"OTEL_GOOGLE_GENAI_GENERATE_CONTENT_CONFIG_INCLUDES": "*"})
+    def test_option_reflected_to_span_attribute_automatic_func_calling(self):
         span = self.generate_and_get_span(
             config={
-                "http_options": {
-                    "base_url": "my.backend.override",
-                    "headers": {
-                        "sensitive": 12345,
-                    },
+                "automatic_function_calling": {
+                    "ignore_call_history": True,
                 }
             }
         )
-        self.assertEqual(
-            span.attributes["gcp.gen_ai.request.http_options.base_url"],
-            "my.backend.override",
+        self.assertTrue(
+            span.attributes[
+                "gcp.gen_ai.request.automatic_function_calling.ignore_call_history"
+            ]
+        )
+
+    def test_dynamic_config_options_not_included_without_allow_list(self):
+        span = self.generate_and_get_span(
+            config={
+                "automatic_function_calling": {
+                    "ignore_call_history": True,
+                }
+            }
         )
         self.assertNotIn(
-            "gcp.gen_ai.request.http_options.headers.sensitive",
+            "gcp.gen_ai.request.automatic_function_calling.ignore_call_history",
             span.attributes,
         )
 
-    def test_option_reflected_to_span_attribute_automatic_func_calling(self):
+    def test_can_supply_allow_list_via_instrumentor_constructor(self):
+        self.set_instrumentor_constructor_kwarg(
+            "generate_content_config_key_allowlist",
+            AllowList(includes=["*"]))
         span = self.generate_and_get_span(
             config={
                 "automatic_function_calling": {