From 816212bd21a1a32f3b642358fbab644dfedd7bc5 Mon Sep 17 00:00:00 2001 From: Aaron Abbott Date: Mon, 13 Jan 2025 21:18:45 +0000 Subject: [PATCH 01/11] Added Vertex AI spans for request parameters --- .../CHANGELOG.md | 2 + .../instrumentation/vertexai/__init__.py | 23 ++- .../instrumentation/vertexai/patch.py | 101 +++++++++++ .../instrumentation/vertexai/utils.py | 162 ++++++++++++++++++ ...at_completion_extra_call_level_params.yaml | 82 +++++++++ ..._completion_extra_client_level_params.yaml | 82 +++++++++ .../test_vertexai_generate_content.yaml | 70 ++++++++ .../tests/conftest.py | 139 +++++++++++++-- .../tests/test_chat_completions.py | 110 ++++++++++++ .../tests/test_placeholder.py | 20 --- tox.ini | 2 +- 11 files changed, 752 insertions(+), 41 deletions(-) create mode 100644 instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py create mode 100644 instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_chat_completion_extra_call_level_params.yaml create mode 100644 instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_chat_completion_extra_client_level_params.yaml create mode 100644 instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_vertexai_generate_content.yaml create mode 100644 instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_chat_completions.py delete mode 100644 instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_placeholder.py diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/CHANGELOG.md b/instrumentation-genai/opentelemetry-instrumentation-vertexai/CHANGELOG.md index 4d786c7840..4e43fbff19 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/CHANGELOG.md +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/CHANGELOG.md @@ -7,5 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +- Added Vertex AI spans for request parameters + ([#3192](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3192)) - Initial VertexAI instrumentation ([#3123](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3123)) diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/__init__.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/__init__.py index 9437184ff0..7f7b88ff42 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/__init__.py +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/__init__.py @@ -41,9 +41,17 @@ from typing import Any, Collection +from wrapt import ( + wrap_function_wrapper, # type: ignore[reportUnknownVariableType] +) + from opentelemetry._events import get_event_logger from opentelemetry.instrumentation.instrumentor import BaseInstrumentor from opentelemetry.instrumentation.vertexai.package import _instruments +from opentelemetry.instrumentation.vertexai.patch import ( + generate_content_create, +) +from opentelemetry.instrumentation.vertexai.utils import is_content_enabled from opentelemetry.semconv.schemas import Schemas from opentelemetry.trace import get_tracer @@ -55,20 +63,29 @@ def instrumentation_dependencies(self) -> Collection[str]: def _instrument(self, **kwargs: Any): """Enable VertexAI instrumentation.""" tracer_provider = kwargs.get("tracer_provider") - _tracer = get_tracer( + tracer = get_tracer( __name__, "", tracer_provider, schema_url=Schemas.V1_28_0.value, ) event_logger_provider = kwargs.get("event_logger_provider") - _event_logger = get_event_logger( + event_logger = get_event_logger( __name__, "", schema_url=Schemas.V1_28_0.value, event_logger_provider=event_logger_provider, ) - # TODO: implemented in later PR + + wrap_function_wrapper( + module="vertexai.generative_models._generative_models", + # Patching this base class also instruments the vertexai.preview.generative_models + # package + name="_GenerativeModel.generate_content", + wrapper=generate_content_create( + tracer, event_logger, is_content_enabled() + ), + ) def _uninstrument(self, **kwargs: Any) -> None: """TODO: implemented in later PR""" diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py index b0a6f42841..152d24acca 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py @@ -11,3 +11,104 @@ # 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. + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any, Callable, Iterable, Optional + +from opentelemetry._events import EventLogger +from opentelemetry.instrumentation.vertexai.utils import ( + GenerateContentParams, + get_genai_request_attributes, + get_span_name, + handle_span_exception, +) +from opentelemetry.trace import SpanKind, Tracer + +if TYPE_CHECKING: + from vertexai.generative_models import ( + GenerationResponse, + Tool, + ToolConfig, + ) + from vertexai.generative_models._generative_models import ( + ContentsType, + GenerationConfigType, + SafetySettingsType, + _GenerativeModel, + ) + + +def generate_content_create( + tracer: Tracer, event_logger: EventLogger, capture_content: bool +): + """Wrap the `generate_content` method of the `GenerativeModel` class to trace it.""" + + def traced_method( + wrapped: Callable[ + ..., GenerationResponse | Iterable[GenerationResponse] + ], + instance: _GenerativeModel, + args: Any, + kwargs: Any, + ): + # Use exact parameter signature to handle named vs positional args robustly + def extract_params( + contents: ContentsType, + *, + generation_config: Optional[GenerationConfigType] = None, + safety_settings: Optional[SafetySettingsType] = None, + tools: Optional[list[Tool]] = None, + tool_config: Optional[ToolConfig] = None, + labels: Optional[dict[str, str]] = None, + stream: bool = False, + ) -> GenerateContentParams: + return GenerateContentParams( + contents=contents, + generation_config=generation_config, + safety_settings=safety_settings, + tools=tools, + tool_config=tool_config, + labels=labels, + stream=stream, + ) + + params = extract_params(*args, **kwargs) + + span_attributes = get_genai_request_attributes(instance, params) + + span_name = get_span_name(span_attributes) + with tracer.start_as_current_span( + name=span_name, + kind=SpanKind.CLIENT, + attributes=span_attributes, + end_on_exit=False, + ) as span: + # TODO: emit request events + # if span.is_recording(): + # for message in kwargs.get("messages", []): + # event_logger.emit( + # message_to_event(message, capture_content) + # ) + + try: + result = wrapped(*args, **kwargs) + # TODO: handle streaming + # if is_streaming(kwargs): + # return StreamWrapper( + # result, span, event_logger, capture_content + # ) + + # TODO: add response attributes and events + # if span.is_recording(): + # _set_response_attributes( + # span, result, event_logger, capture_content + # ) + span.end() + return result + + except Exception as error: + handle_span_exception(span, error) + raise + + return traced_method diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py new file mode 100644 index 0000000000..aab4c9055c --- /dev/null +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py @@ -0,0 +1,162 @@ +# 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. + +from __future__ import annotations + +from dataclasses import dataclass +from os import environ +from typing import ( + TYPE_CHECKING, + Dict, + List, + Mapping, + Optional, + TypedDict, + cast, +) + +from opentelemetry.semconv._incubating.attributes import ( + gen_ai_attributes as GenAIAttributes, +) +from opentelemetry.semconv.attributes import ( + error_attributes as ErrorAttributes, +) +from opentelemetry.trace import Span +from opentelemetry.trace.status import Status, StatusCode +from opentelemetry.util.types import AttributeValue + +if TYPE_CHECKING: + from vertexai.generative_models import Tool, ToolConfig + from vertexai.generative_models._generative_models import ( + ContentsType, + GenerationConfigType, + SafetySettingsType, + _GenerativeModel, + ) + + +@dataclass(frozen=True) +class GenerateContentParams: + contents: ContentsType + generation_config: Optional[GenerationConfigType] + safety_settings: Optional[SafetySettingsType] + tools: Optional[List["Tool"]] + tool_config: Optional["ToolConfig"] + labels: Optional[Dict[str, str]] + stream: bool + + +class GenerationConfigDict(TypedDict, total=False): + temperature: Optional[float] + top_p: Optional[float] + top_k: Optional[int] + max_output_tokens: Optional[int] + stop_sequences: Optional[List[str]] + presence_penalty: Optional[float] + frequency_penalty: Optional[float] + seed: Optional[int] + # And more fields which aren't needed yet + + +def get_genai_request_attributes( + # TODO: use types + instance: _GenerativeModel, + params: GenerateContentParams, + operation_name: GenAIAttributes.GenAiOperationNameValues = GenAIAttributes.GenAiOperationNameValues.CHAT, +): + model = _get_model_name(instance) + generation_config = _get_generation_config(instance, params) + attributes = { + GenAIAttributes.GEN_AI_OPERATION_NAME: operation_name.value, + GenAIAttributes.GEN_AI_SYSTEM: GenAIAttributes.GenAiSystemValues.VERTEX_AI.value, + GenAIAttributes.GEN_AI_REQUEST_MODEL: model, + GenAIAttributes.GEN_AI_REQUEST_TEMPERATURE: generation_config.get( + "temperature" + ), + GenAIAttributes.GEN_AI_REQUEST_TOP_P: generation_config.get("top_p"), + GenAIAttributes.GEN_AI_REQUEST_MAX_TOKENS: generation_config.get( + "max_output_tokens" + ), + GenAIAttributes.GEN_AI_REQUEST_PRESENCE_PENALTY: generation_config.get( + "presence_penalty" + ), + GenAIAttributes.GEN_AI_REQUEST_FREQUENCY_PENALTY: generation_config.get( + "frequency_penalty" + ), + GenAIAttributes.GEN_AI_OPENAI_REQUEST_SEED: generation_config.get( + "seed" + ), + GenAIAttributes.GEN_AI_REQUEST_STOP_SEQUENCES: generation_config.get( + "stop_sequences" + ), + } + + # filter out None values + return {k: v for k, v in attributes.items() if v is not None} + + +def _get_generation_config( + instance: _GenerativeModel, + params: GenerateContentParams, +) -> GenerationConfigDict: + generation_config = params.generation_config or instance._generation_config + if generation_config is None: + return {} + if isinstance(generation_config, dict): + return cast(GenerationConfigDict, generation_config) + return cast(GenerationConfigDict, generation_config.to_dict()) + + +_RESOURCE_PREFIX = "publishers/google/models/" + + +def _get_model_name(instance: _GenerativeModel) -> str: + model_name = instance._model_name + + # Can use str.removeprefix() once 3.8 is dropped + if model_name.startswith(_RESOURCE_PREFIX): + model_name = model_name[len(_RESOURCE_PREFIX) :] + return model_name + + +# TODO: Everything below here should be replaced with +# opentelemetry.instrumentation.genai_utils instead once it is released. +# https://github.com/open-telemetry/opentelemetry-python-contrib/issues/3191 + +OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT = ( + "OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT" +) + + +def is_content_enabled() -> bool: + capture_content = environ.get( + OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT, "false" + ) + + return capture_content.lower() == "true" + + +def get_span_name(span_attributes: Mapping[str, AttributeValue]): + name = span_attributes.get(GenAIAttributes.GEN_AI_OPERATION_NAME, "") + model = span_attributes.get(GenAIAttributes.GEN_AI_REQUEST_MODEL, "") + return f"{name} {model}" + + +def handle_span_exception(span: Span, error: Exception): + span.set_status(Status(StatusCode.ERROR, str(error))) + if span.is_recording(): + span.set_attribute( + ErrorAttributes.ERROR_TYPE, type(error).__qualname__ + ) + span.end() diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_chat_completion_extra_call_level_params.yaml b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_chat_completion_extra_call_level_params.yaml new file mode 100644 index 0000000000..7a2326c15e --- /dev/null +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_chat_completion_extra_call_level_params.yaml @@ -0,0 +1,82 @@ +interactions: +- request: + body: |- + { + "contents": [ + { + "role": "user", + "parts": [ + { + "text": "Say this is a test" + } + ] + } + ], + "generationConfig": { + "temperature": 0.2, + "topP": 0.95, + "topK": 2.0, + "maxOutputTokens": 5, + "stopSequences": [ + "\n\n\n" + ], + "presencePenalty": -1.5, + "frequencyPenalty": 1.0, + "seed": 12345 + } + } + headers: + Accept: + - '*/*' + Accept-Encoding: + - gzip, deflate + Connection: + - keep-alive + Content-Length: + - '376' + Content-Type: + - application/json + User-Agent: + - python-requests/2.32.3 + method: POST + uri: https://us-central1-aiplatform.googleapis.com/v1/projects/fake-project/locations/us-central1/publishers/google/models/gemini-1.5-flash-002:generateContent?%24alt=json%3Benum-encoding%3Dint + response: + body: + string: |- + { + "candidates": [ + { + "content": { + "role": "model", + "parts": [ + { + "text": "Okay, I understand." + } + ] + }, + "finishReason": 2, + "avgLogprobs": -0.006723951548337936 + } + ], + "usageMetadata": { + "promptTokenCount": 5, + "candidatesTokenCount": 5, + "totalTokenCount": 10 + }, + "modelVersion": "gemini-1.5-flash-002" + } + headers: + Content-Type: + - application/json; charset=UTF-8 + Transfer-Encoding: + - chunked + Vary: + - Origin + - X-Origin + - Referer + content-length: + - '407' + status: + code: 200 + message: OK +version: 1 diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_chat_completion_extra_client_level_params.yaml b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_chat_completion_extra_client_level_params.yaml new file mode 100644 index 0000000000..7a2326c15e --- /dev/null +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_chat_completion_extra_client_level_params.yaml @@ -0,0 +1,82 @@ +interactions: +- request: + body: |- + { + "contents": [ + { + "role": "user", + "parts": [ + { + "text": "Say this is a test" + } + ] + } + ], + "generationConfig": { + "temperature": 0.2, + "topP": 0.95, + "topK": 2.0, + "maxOutputTokens": 5, + "stopSequences": [ + "\n\n\n" + ], + "presencePenalty": -1.5, + "frequencyPenalty": 1.0, + "seed": 12345 + } + } + headers: + Accept: + - '*/*' + Accept-Encoding: + - gzip, deflate + Connection: + - keep-alive + Content-Length: + - '376' + Content-Type: + - application/json + User-Agent: + - python-requests/2.32.3 + method: POST + uri: https://us-central1-aiplatform.googleapis.com/v1/projects/fake-project/locations/us-central1/publishers/google/models/gemini-1.5-flash-002:generateContent?%24alt=json%3Benum-encoding%3Dint + response: + body: + string: |- + { + "candidates": [ + { + "content": { + "role": "model", + "parts": [ + { + "text": "Okay, I understand." + } + ] + }, + "finishReason": 2, + "avgLogprobs": -0.006723951548337936 + } + ], + "usageMetadata": { + "promptTokenCount": 5, + "candidatesTokenCount": 5, + "totalTokenCount": 10 + }, + "modelVersion": "gemini-1.5-flash-002" + } + headers: + Content-Type: + - application/json; charset=UTF-8 + Transfer-Encoding: + - chunked + Vary: + - Origin + - X-Origin + - Referer + content-length: + - '407' + status: + code: 200 + message: OK +version: 1 diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_vertexai_generate_content.yaml b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_vertexai_generate_content.yaml new file mode 100644 index 0000000000..a1c1b45219 --- /dev/null +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_vertexai_generate_content.yaml @@ -0,0 +1,70 @@ +interactions: +- request: + body: |- + { + "contents": [ + { + "role": "user", + "parts": [ + { + "text": "Say this is a test" + } + ] + } + ] + } + headers: + Accept: + - '*/*' + Accept-Encoding: + - gzip, deflate + Connection: + - keep-alive + Content-Length: + - '141' + Content-Type: + - application/json + User-Agent: + - python-requests/2.32.3 + method: POST + uri: https://us-central1-aiplatform.googleapis.com/v1/projects/fake-project/locations/us-central1/publishers/google/models/gemini-1.5-flash-002:generateContent?%24alt=json%3Benum-encoding%3Dint + response: + body: + string: |- + { + "candidates": [ + { + "content": { + "role": "model", + "parts": [ + { + "text": "Okay, I understand. I'm ready for your test. Please proceed.\n" + } + ] + }, + "finishReason": 1, + "avgLogprobs": -0.005634686664531105 + } + ], + "usageMetadata": { + "promptTokenCount": 5, + "candidatesTokenCount": 19, + "totalTokenCount": 24 + }, + "modelVersion": "gemini-1.5-flash-002" + } + headers: + Content-Type: + - application/json; charset=UTF-8 + Transfer-Encoding: + - chunked + Vary: + - Origin + - X-Origin + - Referer + content-length: + - '453' + status: + code: 200 + message: OK +version: 1 diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/conftest.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/conftest.py index 8337188ece..fefa895417 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/conftest.py +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/conftest.py @@ -1,22 +1,42 @@ """Unit tests configuration module.""" import json +import os +import re +from typing import Any, Mapping, MutableMapping import pytest +import vertexai import yaml - +from google.auth.credentials import AnonymousCredentials +from vcr import VCR +from vcr.record_mode import RecordMode +from vcr.request import Request + +from opentelemetry.instrumentation.vertexai import VertexAIInstrumentor +from opentelemetry.instrumentation.vertexai.utils import ( + OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT, +) from opentelemetry.sdk._events import EventLoggerProvider from opentelemetry.sdk._logs import LoggerProvider from opentelemetry.sdk._logs.export import ( InMemoryLogExporter, SimpleLogRecordProcessor, ) +from opentelemetry.sdk.metrics import ( + MeterProvider, +) +from opentelemetry.sdk.metrics.export import ( + InMemoryMetricReader, +) from opentelemetry.sdk.trace import TracerProvider from opentelemetry.sdk.trace.export import SimpleSpanProcessor from opentelemetry.sdk.trace.export.in_memory_span_exporter import ( InMemorySpanExporter, ) +FAKE_PROJECT = "fake-project" + @pytest.fixture(scope="function", name="span_exporter") def fixture_span_exporter(): @@ -30,6 +50,12 @@ def fixture_log_exporter(): yield exporter +@pytest.fixture(scope="function", name="metric_reader") +def fixture_metric_reader(): + exporter = InMemoryMetricReader() + yield exporter + + @pytest.fixture(scope="function", name="tracer_provider") def fixture_tracer_provider(span_exporter): provider = TracerProvider() @@ -46,17 +72,105 @@ def fixture_event_logger_provider(log_exporter): return event_logger_provider +@pytest.fixture(scope="function", name="meter_provider") +def fixture_meter_provider(metric_reader): + return MeterProvider( + metric_readers=[metric_reader], + ) + + +@pytest.fixture(autouse=True) +def vertexai_init(vcr: VCR) -> None: + # Unfortunately I couldn't find a nice way to globally reset the global_config for each + # test because different vertex submodules reference the global instance directly + # https://github.com/googleapis/python-aiplatform/blob/v1.74.0/google/cloud/aiplatform/initializer.py#L687 + # so this config will leak if we don't call init() for each test. + + # When not recording (in CI), don't do any auth. That prevents trying to read application + # default credentials from the filesystem or metadata server and oauth token exchange. This + # is not the interesting part of our instrumentation to test. + vertex_init_kwargs = {"api_transport": "rest"} + if vcr.record_mode == RecordMode.NONE: + vertex_init_kwargs["credentials"] = AnonymousCredentials() + vertex_init_kwargs["project"] = FAKE_PROJECT + vertexai.init(**vertex_init_kwargs) + + +@pytest.fixture +def instrument_no_content( + tracer_provider, event_logger_provider, meter_provider +): + os.environ.update( + {OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT: "False"} + ) + + instrumentor = VertexAIInstrumentor() + instrumentor.instrument( + tracer_provider=tracer_provider, + event_logger_provider=event_logger_provider, + meter_provider=meter_provider, + ) + + yield instrumentor + os.environ.pop(OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT, None) + instrumentor.uninstrument() + + +@pytest.fixture +def instrument_with_content( + tracer_provider, event_logger_provider, meter_provider +): + os.environ.update( + {OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT: "True"} + ) + instrumentor = VertexAIInstrumentor() + instrumentor.instrument( + tracer_provider=tracer_provider, + event_logger_provider=event_logger_provider, + meter_provider=meter_provider, + ) + + yield instrumentor + os.environ.pop(OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT, None) + instrumentor.uninstrument() + + @pytest.fixture(scope="module") def vcr_config(): + filter_header_regexes = [ + r"X-.*", + "Server", + "Date", + "Expires", + "Authorization", + ] + + def filter_headers(headers: Mapping[str, str]) -> Mapping[str, str]: + return { + key: val + for key, val in headers.items() + if not any( + re.match(filter_re, key, re.IGNORECASE) + for filter_re in filter_header_regexes + ) + } + + def before_record_cb(request: Request): + request.headers = filter_headers(request.headers) + request.uri = re.sub( + r"/projects/[^/]+/", "/projects/fake-project/", request.uri + ) + return request + + def before_response_cb(response: MutableMapping[str, Any]): + response["headers"] = filter_headers(response["headers"]) + return response + return { - "filter_headers": [ - ("cookie", "test_cookie"), - ("authorization", "Bearer test_vertexai_api_key"), - ("vertexai-organization", "test_vertexai_org_id"), - ("vertexai-project", "test_vertexai_project_id"), - ], "decode_compressed_response": True, - "before_record_response": scrub_response_headers, + "before_record_request": before_record_cb, + "before_record_response": before_response_cb, + "ignore_hosts": ["oauth2.googleapis.com"], } @@ -125,12 +239,3 @@ def deserialize(cassette_string): def fixture_vcr(vcr): vcr.register_serializer("yaml", PrettyPrintJSONBody) return vcr - - -def scrub_response_headers(response): - """ - This scrubs sensitive response headers. Note they are case-sensitive! - """ - response["headers"]["vertexai-organization"] = "test_vertexai_org_id" - response["headers"]["Set-Cookie"] = "test_set_cookie" - return response diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_chat_completions.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_chat_completions.py new file mode 100644 index 0000000000..931d09b756 --- /dev/null +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_chat_completions.py @@ -0,0 +1,110 @@ +import pytest +from vertexai.generative_models import ( + Content, + GenerationConfig, + GenerativeModel, + Part, +) + +from opentelemetry.instrumentation.vertexai import VertexAIInstrumentor +from opentelemetry.sdk.trace.export.in_memory_span_exporter import ( + InMemorySpanExporter, +) + + +@pytest.mark.vcr +def test_vertexai_generate_content( + span_exporter: InMemorySpanExporter, + instrument_with_content: VertexAIInstrumentor, +): + model = GenerativeModel("gemini-1.5-flash-002") + model.generate_content( + [ + Content(role="user", parts=[Part.from_text("Say this is a test")]), + ] + ) + + spans = span_exporter.get_finished_spans() + assert len(spans) == 1 + assert spans[0].name == "chat gemini-1.5-flash-002" + assert dict(spans[0].attributes) == { + "gen_ai.operation.name": "chat", + "gen_ai.request.model": "gemini-1.5-flash-002", + "gen_ai.system": "vertex_ai", + } + + +@pytest.mark.vcr() +def test_chat_completion_extra_client_level_params( + span_exporter, instrument_no_content +): + generation_config = GenerationConfig( + top_k=2, + top_p=0.95, + temperature=0.2, + stop_sequences=["\n\n\n"], + max_output_tokens=5, + presence_penalty=-1.5, + frequency_penalty=1.0, + seed=12345, + ) + model = GenerativeModel("gemini-1.5-flash-002") + model.generate_content( + [ + Content(role="user", parts=[Part.from_text("Say this is a test")]), + ], + generation_config=generation_config, + ) + + spans = span_exporter.get_finished_spans() + assert len(spans) == 1 + assert dict(spans[0].attributes) == { + "gen_ai.openai.request.seed": 12345, + "gen_ai.operation.name": "chat", + "gen_ai.request.frequency_penalty": 1.0, + "gen_ai.request.max_tokens": 5, + "gen_ai.request.model": "gemini-1.5-flash-002", + "gen_ai.request.presence_penalty": -1.5, + "gen_ai.request.stop_sequences": ("\n\n\n",), + "gen_ai.request.temperature": 0.2, + "gen_ai.request.top_p": 0.95, + "gen_ai.system": "vertex_ai", + } + + +@pytest.mark.vcr() +def test_chat_completion_extra_call_level_params( + span_exporter, instrument_no_content +): + generation_config = GenerationConfig( + top_k=2, + top_p=0.95, + temperature=0.2, + stop_sequences=["\n\n\n"], + max_output_tokens=5, + presence_penalty=-1.5, + frequency_penalty=1.0, + seed=12345, + ) + model = GenerativeModel("gemini-1.5-flash-002") + model.generate_content( + [ + Content(role="user", parts=[Part.from_text("Say this is a test")]), + ], + generation_config=generation_config, + ) + + spans = span_exporter.get_finished_spans() + assert len(spans) == 1 + assert dict(spans[0].attributes) == { + "gen_ai.openai.request.seed": 12345, + "gen_ai.operation.name": "chat", + "gen_ai.request.frequency_penalty": 1.0, + "gen_ai.request.max_tokens": 5, + "gen_ai.request.model": "gemini-1.5-flash-002", + "gen_ai.request.presence_penalty": -1.5, + "gen_ai.request.stop_sequences": ("\n\n\n",), + "gen_ai.request.temperature": 0.2, + "gen_ai.request.top_p": 0.95, + "gen_ai.system": "vertex_ai", + } diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_placeholder.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_placeholder.py deleted file mode 100644 index c910bfa0bf..0000000000 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_placeholder.py +++ /dev/null @@ -1,20 +0,0 @@ -# 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. - -# TODO: adapt tests from OpenLLMetry here along with tests from -# instrumentation-genai/opentelemetry-instrumentation-openai-v2/tests/test_chat_completions.py - - -def test_placeholder(): - assert True diff --git a/tox.ini b/tox.ini index 13dd9fafb6..583c521f76 100644 --- a/tox.ini +++ b/tox.ini @@ -789,7 +789,7 @@ commands = test-instrumentation-openai-v2: pytest {toxinidir}/instrumentation-genai/opentelemetry-instrumentation-openai-v2/tests {posargs} lint-instrumentation-openai-v2: sh -c "cd instrumentation-genai && pylint --rcfile ../.pylintrc opentelemetry-instrumentation-openai-v2" - test-instrumentation-vertexai: pytest {toxinidir}/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests {posargs} + test-instrumentation-vertexai: pytest {toxinidir}/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests --vcr-record=none {posargs} lint-instrumentation-vertexai: sh -c "cd instrumentation-genai && pylint --rcfile ../.pylintrc opentelemetry-instrumentation-vertexai" test-instrumentation-sio-pika: pytest {toxinidir}/instrumentation/opentelemetry-instrumentation-pika/tests {posargs} From 5dfc2824d56230f8630751d0dc0197ea00608c9c Mon Sep 17 00:00:00 2001 From: Aaron Abbott Date: Thu, 16 Jan 2025 20:09:58 +0000 Subject: [PATCH 02/11] small fixes, get CI passing --- .../instrumentation/vertexai/patch.py | 5 ++++- .../instrumentation/vertexai/utils.py | 5 ----- .../tests/conftest.py | 16 +++++++--------- 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py index 152d24acca..7e0e0ed617 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py @@ -52,7 +52,9 @@ def traced_method( args: Any, kwargs: Any, ): - # Use exact parameter signature to handle named vs positional args robustly + # Use parameter signature from + # https://github.com/googleapis/python-aiplatform/blob/v1.76.0/vertexai/generative_models/_generative_models.py#L595 + # to handle named vs positional args robustly def extract_params( contents: ContentsType, *, @@ -62,6 +64,7 @@ def extract_params( tool_config: Optional[ToolConfig] = None, labels: Optional[dict[str, str]] = None, stream: bool = False, + **_kwargs: Any, ) -> GenerateContentParams: return GenerateContentParams( contents=contents, diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py index aab4c9055c..6cf1df48a8 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py @@ -70,7 +70,6 @@ class GenerationConfigDict(TypedDict, total=False): def get_genai_request_attributes( - # TODO: use types instance: _GenerativeModel, params: GenerateContentParams, operation_name: GenAIAttributes.GenAiOperationNameValues = GenAIAttributes.GenAiOperationNameValues.CHAT, @@ -130,10 +129,6 @@ def _get_model_name(instance: _GenerativeModel) -> str: return model_name -# TODO: Everything below here should be replaced with -# opentelemetry.instrumentation.genai_utils instead once it is released. -# https://github.com/open-telemetry/opentelemetry-python-contrib/issues/3191 - OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT = ( "OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT" ) diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/conftest.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/conftest.py index fefa895417..b76a108805 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/conftest.py +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/conftest.py @@ -81,19 +81,17 @@ def fixture_meter_provider(metric_reader): @pytest.fixture(autouse=True) def vertexai_init(vcr: VCR) -> None: - # Unfortunately I couldn't find a nice way to globally reset the global_config for each - # test because different vertex submodules reference the global instance directly - # https://github.com/googleapis/python-aiplatform/blob/v1.74.0/google/cloud/aiplatform/initializer.py#L687 - # so this config will leak if we don't call init() for each test. - # When not recording (in CI), don't do any auth. That prevents trying to read application # default credentials from the filesystem or metadata server and oauth token exchange. This # is not the interesting part of our instrumentation to test. - vertex_init_kwargs = {"api_transport": "rest"} + credentials = None + project = None if vcr.record_mode == RecordMode.NONE: - vertex_init_kwargs["credentials"] = AnonymousCredentials() - vertex_init_kwargs["project"] = FAKE_PROJECT - vertexai.init(**vertex_init_kwargs) + credentials = AnonymousCredentials() + project = FAKE_PROJECT + vertexai.init( + api_transport="rest", credentials=credentials, project=project + ) @pytest.fixture From 302c2a4c7acddf9067fbef3376c95075c58eb7e6 Mon Sep 17 00:00:00 2001 From: Aaron Abbott Date: Thu, 16 Jan 2025 23:26:41 +0000 Subject: [PATCH 03/11] Use standard OTel tracing error handling --- .../instrumentation/vertexai/patch.py | 34 ++++------ .../instrumentation/vertexai/utils.py | 14 ---- .../test_vertexai_generate_content.yaml | 2 +- .../test_vertexai_generate_content_error.yaml | 64 +++++++++++++++++++ .../tests/test_chat_completions.py | 38 +++++++++++ 5 files changed, 116 insertions(+), 36 deletions(-) create mode 100644 instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_vertexai_generate_content_error.yaml diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py index 7e0e0ed617..8aaa884ce6 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py @@ -21,7 +21,6 @@ GenerateContentParams, get_genai_request_attributes, get_span_name, - handle_span_exception, ) from opentelemetry.trace import SpanKind, Tracer @@ -85,8 +84,7 @@ def extract_params( name=span_name, kind=SpanKind.CLIENT, attributes=span_attributes, - end_on_exit=False, - ) as span: + ) as _span: # TODO: emit request events # if span.is_recording(): # for message in kwargs.get("messages", []): @@ -94,24 +92,18 @@ def extract_params( # message_to_event(message, capture_content) # ) - try: - result = wrapped(*args, **kwargs) - # TODO: handle streaming - # if is_streaming(kwargs): - # return StreamWrapper( - # result, span, event_logger, capture_content - # ) + result = wrapped(*args, **kwargs) + # TODO: handle streaming + # if is_streaming(kwargs): + # return StreamWrapper( + # result, span, event_logger, capture_content + # ) - # TODO: add response attributes and events - # if span.is_recording(): - # _set_response_attributes( - # span, result, event_logger, capture_content - # ) - span.end() - return result - - except Exception as error: - handle_span_exception(span, error) - raise + # TODO: add response attributes and events + # if span.is_recording(): + # _set_response_attributes( + # span, result, event_logger, capture_content + # ) + return result return traced_method diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py index 6cf1df48a8..09277a8b65 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py @@ -29,11 +29,6 @@ from opentelemetry.semconv._incubating.attributes import ( gen_ai_attributes as GenAIAttributes, ) -from opentelemetry.semconv.attributes import ( - error_attributes as ErrorAttributes, -) -from opentelemetry.trace import Span -from opentelemetry.trace.status import Status, StatusCode from opentelemetry.util.types import AttributeValue if TYPE_CHECKING: @@ -146,12 +141,3 @@ def get_span_name(span_attributes: Mapping[str, AttributeValue]): name = span_attributes.get(GenAIAttributes.GEN_AI_OPERATION_NAME, "") model = span_attributes.get(GenAIAttributes.GEN_AI_REQUEST_MODEL, "") return f"{name} {model}" - - -def handle_span_exception(span: Span, error: Exception): - span.set_status(Status(StatusCode.ERROR, str(error))) - if span.is_recording(): - span.set_attribute( - ErrorAttributes.ERROR_TYPE, type(error).__qualname__ - ) - span.end() diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_vertexai_generate_content.yaml b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_vertexai_generate_content.yaml index a1c1b45219..ea733fd1c0 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_vertexai_generate_content.yaml +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_vertexai_generate_content.yaml @@ -43,7 +43,7 @@ interactions: ] }, "finishReason": 1, - "avgLogprobs": -0.005634686664531105 + "avgLogprobs": -0.0054909539850134595 } ], "usageMetadata": { diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_vertexai_generate_content_error.yaml b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_vertexai_generate_content_error.yaml new file mode 100644 index 0000000000..7c9c6e7ac2 --- /dev/null +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_vertexai_generate_content_error.yaml @@ -0,0 +1,64 @@ +interactions: +- request: + body: |- + { + "contents": [ + { + "role": "user", + "parts": [ + { + "text": "Say this is a test" + } + ] + } + ], + "generationConfig": { + "temperature": 1000.0 + } + } + headers: + Accept: + - '*/*' + Accept-Encoding: + - gzip, deflate + Connection: + - keep-alive + Content-Length: + - '196' + Content-Type: + - application/json + User-Agent: + - python-requests/2.32.3 + method: POST + uri: https://us-central1-aiplatform.googleapis.com/v1/projects/fake-project/locations/us-central1/publishers/google/models/gemini-1.5-flash-002:generateContent?%24alt=json%3Benum-encoding%3Dint + response: + body: + string: |- + { + "error": { + "code": 400, + "message": "Unable to submit request because it has a temperature value of 1000 but the supported range is from 0 (inclusive) to 2.0001 (exclusive). Update the value and try again.", + "status": "INVALID_ARGUMENT", + "details": [ + { + "@type": "type.googleapis.com/google.rpc.DebugInfo", + "detail": "[ORIGINAL ERROR] generic::invalid_argument: Unable to submit request because it has a temperature value of 1000 but the supported range is from 0 (inclusive) to 2.0001 (exclusive). Update the value and try again. [google.rpc.error_details_ext] { message: \"Unable to submit request because it has a temperature value of 1000 but the supported range is from 0 (inclusive) to 2.0001 (exclusive). Update the value and try again.\" }" + } + ] + } + } + headers: + Content-Type: + - application/json; charset=UTF-8 + Transfer-Encoding: + - chunked + Vary: + - Origin + - X-Origin + - Referer + content-length: + - '809' + status: + code: 400 + message: Bad Request +version: 1 diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_chat_completions.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_chat_completions.py index 931d09b756..accadeb856 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_chat_completions.py +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_chat_completions.py @@ -1,4 +1,5 @@ import pytest +from google.api_core.exceptions import BadRequest from vertexai.generative_models import ( Content, GenerationConfig, @@ -10,6 +11,7 @@ from opentelemetry.sdk.trace.export.in_memory_span_exporter import ( InMemorySpanExporter, ) +from opentelemetry.trace import StatusCode @pytest.mark.vcr @@ -34,6 +36,42 @@ def test_vertexai_generate_content( } +@pytest.mark.vcr +def test_vertexai_generate_content_error( + span_exporter: InMemorySpanExporter, + instrument_with_content: VertexAIInstrumentor, +): + model = GenerativeModel("gemini-1.5-flash-002") + try: + # Temperature out of range causes error + model.generate_content( + [ + Content( + role="user", parts=[Part.from_text("Say this is a test")] + ) + ], + generation_config=GenerationConfig(temperature=1000), + ) + except BadRequest: + pass + + spans = span_exporter.get_finished_spans() + assert len(spans) == 1 + assert spans[0].name == "chat gemini-1.5-flash-002" + assert dict(spans[0].attributes) == { + "gen_ai.operation.name": "chat", + "gen_ai.request.model": "gemini-1.5-flash-002", + "gen_ai.request.temperature": 1000.0, + "gen_ai.system": "vertex_ai", + } + # Sets error status + assert spans[0].status.status_code == StatusCode.ERROR + + # Records exception event + assert len(spans[0].events) == 1 + assert spans[0].events[0].name == "exception" + + @pytest.mark.vcr() def test_chat_completion_extra_client_level_params( span_exporter, instrument_no_content From 66ed1de106d03680b302279af8967d85b507ee5c Mon Sep 17 00:00:00 2001 From: Aaron Abbott Date: Thu, 16 Jan 2025 23:30:51 +0000 Subject: [PATCH 04/11] move nested util --- .../instrumentation/vertexai/patch.py | 52 +++++++++---------- .../test_vertexai_generate_content_error.yaml | 7 +-- 2 files changed, 27 insertions(+), 32 deletions(-) diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py index 8aaa884ce6..d7736b2d3c 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py @@ -38,6 +38,31 @@ ) +# Use parameter signature from +# https://github.com/googleapis/python-aiplatform/blob/v1.76.0/vertexai/generative_models/_generative_models.py#L595 +# to handle named vs positional args robustly +def _extract_params( + contents: ContentsType, + *, + generation_config: Optional[GenerationConfigType] = None, + safety_settings: Optional[SafetySettingsType] = None, + tools: Optional[list[Tool]] = None, + tool_config: Optional[ToolConfig] = None, + labels: Optional[dict[str, str]] = None, + stream: bool = False, + **_kwargs: Any, +) -> GenerateContentParams: + return GenerateContentParams( + contents=contents, + generation_config=generation_config, + safety_settings=safety_settings, + tools=tools, + tool_config=tool_config, + labels=labels, + stream=stream, + ) + + def generate_content_create( tracer: Tracer, event_logger: EventLogger, capture_content: bool ): @@ -51,32 +76,7 @@ def traced_method( args: Any, kwargs: Any, ): - # Use parameter signature from - # https://github.com/googleapis/python-aiplatform/blob/v1.76.0/vertexai/generative_models/_generative_models.py#L595 - # to handle named vs positional args robustly - def extract_params( - contents: ContentsType, - *, - generation_config: Optional[GenerationConfigType] = None, - safety_settings: Optional[SafetySettingsType] = None, - tools: Optional[list[Tool]] = None, - tool_config: Optional[ToolConfig] = None, - labels: Optional[dict[str, str]] = None, - stream: bool = False, - **_kwargs: Any, - ) -> GenerateContentParams: - return GenerateContentParams( - contents=contents, - generation_config=generation_config, - safety_settings=safety_settings, - tools=tools, - tool_config=tool_config, - labels=labels, - stream=stream, - ) - - params = extract_params(*args, **kwargs) - + params = _extract_params(*args, **kwargs) span_attributes = get_genai_request_attributes(instance, params) span_name = get_span_name(span_attributes) diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_vertexai_generate_content_error.yaml b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_vertexai_generate_content_error.yaml index 7c9c6e7ac2..600635f9b2 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_vertexai_generate_content_error.yaml +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_vertexai_generate_content_error.yaml @@ -39,12 +39,7 @@ interactions: "code": 400, "message": "Unable to submit request because it has a temperature value of 1000 but the supported range is from 0 (inclusive) to 2.0001 (exclusive). Update the value and try again.", "status": "INVALID_ARGUMENT", - "details": [ - { - "@type": "type.googleapis.com/google.rpc.DebugInfo", - "detail": "[ORIGINAL ERROR] generic::invalid_argument: Unable to submit request because it has a temperature value of 1000 but the supported range is from 0 (inclusive) to 2.0001 (exclusive). Update the value and try again. [google.rpc.error_details_ext] { message: \"Unable to submit request because it has a temperature value of 1000 but the supported range is from 0 (inclusive) to 2.0001 (exclusive). Update the value and try again.\" }" - } - ] + "details": [] } } headers: From 3512ca3f0fc6fd538ce906b28bb780764a5cd5bd Mon Sep 17 00:00:00 2001 From: Aaron Abbott Date: Fri, 17 Jan 2025 04:30:09 +0000 Subject: [PATCH 05/11] Actually use GAPIC client since thats what we use under the hood Also this is what LangChain uses --- .../instrumentation/vertexai/__init__.py | 13 +- .../instrumentation/vertexai/patch.py | 60 +++++--- .../instrumentation/vertexai/utils.py | 133 ++++++++---------- ..._completion_extra_client_level_params.yaml | 82 ----------- ...=> test_chat_completion_extra_params.yaml} | 2 +- .../tests/test_chat_completions.py | 46 +----- 6 files changed, 107 insertions(+), 229 deletions(-) delete mode 100644 instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_chat_completion_extra_client_level_params.yaml rename instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/{test_chat_completion_extra_call_level_params.yaml => test_chat_completion_extra_params.yaml} (97%) diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/__init__.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/__init__.py index 7f7b88ff42..40d1cb48ac 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/__init__.py +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/__init__.py @@ -78,10 +78,15 @@ def _instrument(self, **kwargs: Any): ) wrap_function_wrapper( - module="vertexai.generative_models._generative_models", - # Patching this base class also instruments the vertexai.preview.generative_models - # package - name="_GenerativeModel.generate_content", + module="google.cloud.aiplatform_v1beta1.services.prediction_service.client", + name="PredictionServiceClient.generate_content", + wrapper=generate_content_create( + tracer, event_logger, is_content_enabled() + ), + ) + wrap_function_wrapper( + module="google.cloud.aiplatform_v1.services.prediction_service.client", + name="PredictionServiceClient.generate_content", wrapper=generate_content_create( tracer, event_logger, is_content_enabled() ), diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py index d7736b2d3c..d2c7b98adc 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py @@ -14,7 +14,15 @@ from __future__ import annotations -from typing import TYPE_CHECKING, Any, Callable, Iterable, Optional +from typing import ( + TYPE_CHECKING, + Any, + Callable, + Iterable, + MutableSequence, + Optional, + Union, +) from opentelemetry._events import EventLogger from opentelemetry.instrumentation.vertexai.utils import ( @@ -25,41 +33,49 @@ from opentelemetry.trace import SpanKind, Tracer if TYPE_CHECKING: + from google.cloud.aiplatform_v1.types import ( + content, + prediction_service, + ) from vertexai.generative_models import ( GenerationResponse, - Tool, - ToolConfig, ) from vertexai.generative_models._generative_models import ( - ContentsType, - GenerationConfigType, - SafetySettingsType, _GenerativeModel, ) # Use parameter signature from -# https://github.com/googleapis/python-aiplatform/blob/v1.76.0/vertexai/generative_models/_generative_models.py#L595 +# https://github.com/googleapis/python-aiplatform/blob/v1.76.0/google/cloud/aiplatform_v1/services/prediction_service/client.py#L2088 # to handle named vs positional args robustly def _extract_params( - contents: ContentsType, + request: Optional[ + Union[prediction_service.GenerateContentRequest, dict[Any, Any]] + ] = None, *, - generation_config: Optional[GenerationConfigType] = None, - safety_settings: Optional[SafetySettingsType] = None, - tools: Optional[list[Tool]] = None, - tool_config: Optional[ToolConfig] = None, - labels: Optional[dict[str, str]] = None, - stream: bool = False, + model: Optional[str] = None, + contents: Optional[MutableSequence[content.Content]] = None, **_kwargs: Any, ) -> GenerateContentParams: + # Request vs the named parameters are mututally exclusive or the RPC will fail + if not request: + return GenerateContentParams( + model=model or "", + contents=contents, + ) + + if isinstance(request, dict): + return GenerateContentParams(**request) + return GenerateContentParams( - contents=contents, - generation_config=generation_config, - safety_settings=safety_settings, - tools=tools, - tool_config=tool_config, - labels=labels, - stream=stream, + model=request.model, + contents=request.contents, + system_instruction=request.system_instruction, + tools=request.tools, + tool_config=request.tool_config, + labels=request.labels, + safety_settings=request.safety_settings, + generation_config=request.generation_config, ) @@ -77,7 +93,7 @@ def traced_method( kwargs: Any, ): params = _extract_params(*args, **kwargs) - span_attributes = get_genai_request_attributes(instance, params) + span_attributes = get_genai_request_attributes(params) span_name = get_span_name(span_attributes) with tracer.start_as_current_span( diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py index 09277a8b65..d80d3b29da 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py @@ -14,16 +14,14 @@ from __future__ import annotations +import re from dataclasses import dataclass from os import environ from typing import ( TYPE_CHECKING, - Dict, - List, Mapping, Optional, - TypedDict, - cast, + Sequence, ) from opentelemetry.semconv._incubating.attributes import ( @@ -32,96 +30,77 @@ from opentelemetry.util.types import AttributeValue if TYPE_CHECKING: - from vertexai.generative_models import Tool, ToolConfig - from vertexai.generative_models._generative_models import ( - ContentsType, - GenerationConfigType, - SafetySettingsType, - _GenerativeModel, - ) + from google.cloud.aiplatform_v1.types import content, tool @dataclass(frozen=True) class GenerateContentParams: - contents: ContentsType - generation_config: Optional[GenerationConfigType] - safety_settings: Optional[SafetySettingsType] - tools: Optional[List["Tool"]] - tool_config: Optional["ToolConfig"] - labels: Optional[Dict[str, str]] - stream: bool - - -class GenerationConfigDict(TypedDict, total=False): - temperature: Optional[float] - top_p: Optional[float] - top_k: Optional[int] - max_output_tokens: Optional[int] - stop_sequences: Optional[List[str]] - presence_penalty: Optional[float] - frequency_penalty: Optional[float] - seed: Optional[int] - # And more fields which aren't needed yet + model: str + contents: Optional[Sequence[content.Content]] = None + system_instruction: Optional[content.Content | None] = None + tools: Optional[Sequence[tool.Tool]] = None + tool_config: Optional[tool.ToolConfig] = None + labels: Optional[Mapping[str, str]] = None + safety_settings: Optional[Sequence[content.SafetySetting]] = None + generation_config: Optional[content.GenerationConfig] = None def get_genai_request_attributes( - instance: _GenerativeModel, params: GenerateContentParams, operation_name: GenAIAttributes.GenAiOperationNameValues = GenAIAttributes.GenAiOperationNameValues.CHAT, ): - model = _get_model_name(instance) - generation_config = _get_generation_config(instance, params) - attributes = { + model = _get_model_name(params.model) + generation_config = params.generation_config + attributes: dict[str, AttributeValue] = { GenAIAttributes.GEN_AI_OPERATION_NAME: operation_name.value, GenAIAttributes.GEN_AI_SYSTEM: GenAIAttributes.GenAiSystemValues.VERTEX_AI.value, GenAIAttributes.GEN_AI_REQUEST_MODEL: model, - GenAIAttributes.GEN_AI_REQUEST_TEMPERATURE: generation_config.get( - "temperature" - ), - GenAIAttributes.GEN_AI_REQUEST_TOP_P: generation_config.get("top_p"), - GenAIAttributes.GEN_AI_REQUEST_MAX_TOKENS: generation_config.get( - "max_output_tokens" - ), - GenAIAttributes.GEN_AI_REQUEST_PRESENCE_PENALTY: generation_config.get( - "presence_penalty" - ), - GenAIAttributes.GEN_AI_REQUEST_FREQUENCY_PENALTY: generation_config.get( - "frequency_penalty" - ), - GenAIAttributes.GEN_AI_OPENAI_REQUEST_SEED: generation_config.get( - "seed" - ), - GenAIAttributes.GEN_AI_REQUEST_STOP_SEQUENCES: generation_config.get( - "stop_sequences" - ), } - # filter out None values - return {k: v for k, v in attributes.items() if v is not None} - - -def _get_generation_config( - instance: _GenerativeModel, - params: GenerateContentParams, -) -> GenerationConfigDict: - generation_config = params.generation_config or instance._generation_config - if generation_config is None: - return {} - if isinstance(generation_config, dict): - return cast(GenerationConfigDict, generation_config) - return cast(GenerationConfigDict, generation_config.to_dict()) - - -_RESOURCE_PREFIX = "publishers/google/models/" - + if not generation_config: + return attributes + + # Check for optional fields + # https://proto-plus-python.readthedocs.io/en/stable/fields.html#optional-fields + if "temperature" in generation_config: + attributes[GenAIAttributes.GEN_AI_REQUEST_TEMPERATURE] = ( + generation_config.temperature + ) + if "top_p" in generation_config: + attributes[GenAIAttributes.GEN_AI_REQUEST_TOP_P] = ( + generation_config.top_p + ) + if "max_output_tokens" in generation_config: + attributes[GenAIAttributes.GEN_AI_REQUEST_MAX_TOKENS] = ( + generation_config.max_output_tokens + ) + if "presence_penalty" in generation_config: + attributes[GenAIAttributes.GEN_AI_REQUEST_PRESENCE_PENALTY] = ( + generation_config.presence_penalty + ) + if "frequency_penalty" in generation_config: + attributes[GenAIAttributes.GEN_AI_REQUEST_FREQUENCY_PENALTY] = ( + generation_config.frequency_penalty + ) + if "seed" in generation_config: + attributes[GenAIAttributes.GEN_AI_OPENAI_REQUEST_SEED] = ( + generation_config.seed + ) + if "stop_sequences" in generation_config: + attributes[GenAIAttributes.GEN_AI_REQUEST_STOP_SEQUENCES] = ( + generation_config.stop_sequences + ) + + return attributes + + +_MODEL_STRIP_RE = re.compile( + r"^projects/(.*)/locations/(.*)/publishers/google/models/" +) -def _get_model_name(instance: _GenerativeModel) -> str: - model_name = instance._model_name - # Can use str.removeprefix() once 3.8 is dropped - if model_name.startswith(_RESOURCE_PREFIX): - model_name = model_name[len(_RESOURCE_PREFIX) :] - return model_name +def _get_model_name(model: str) -> str: + return _MODEL_STRIP_RE.sub("", model) OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT = ( diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_chat_completion_extra_client_level_params.yaml b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_chat_completion_extra_client_level_params.yaml deleted file mode 100644 index 7a2326c15e..0000000000 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_chat_completion_extra_client_level_params.yaml +++ /dev/null @@ -1,82 +0,0 @@ -interactions: -- request: - body: |- - { - "contents": [ - { - "role": "user", - "parts": [ - { - "text": "Say this is a test" - } - ] - } - ], - "generationConfig": { - "temperature": 0.2, - "topP": 0.95, - "topK": 2.0, - "maxOutputTokens": 5, - "stopSequences": [ - "\n\n\n" - ], - "presencePenalty": -1.5, - "frequencyPenalty": 1.0, - "seed": 12345 - } - } - headers: - Accept: - - '*/*' - Accept-Encoding: - - gzip, deflate - Connection: - - keep-alive - Content-Length: - - '376' - Content-Type: - - application/json - User-Agent: - - python-requests/2.32.3 - method: POST - uri: https://us-central1-aiplatform.googleapis.com/v1/projects/fake-project/locations/us-central1/publishers/google/models/gemini-1.5-flash-002:generateContent?%24alt=json%3Benum-encoding%3Dint - response: - body: - string: |- - { - "candidates": [ - { - "content": { - "role": "model", - "parts": [ - { - "text": "Okay, I understand." - } - ] - }, - "finishReason": 2, - "avgLogprobs": -0.006723951548337936 - } - ], - "usageMetadata": { - "promptTokenCount": 5, - "candidatesTokenCount": 5, - "totalTokenCount": 10 - }, - "modelVersion": "gemini-1.5-flash-002" - } - headers: - Content-Type: - - application/json; charset=UTF-8 - Transfer-Encoding: - - chunked - Vary: - - Origin - - X-Origin - - Referer - content-length: - - '407' - status: - code: 200 - message: OK -version: 1 diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_chat_completion_extra_call_level_params.yaml b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_chat_completion_extra_params.yaml similarity index 97% rename from instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_chat_completion_extra_call_level_params.yaml rename to instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_chat_completion_extra_params.yaml index 7a2326c15e..e6547166bc 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_chat_completion_extra_call_level_params.yaml +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_chat_completion_extra_params.yaml @@ -55,7 +55,7 @@ interactions: ] }, "finishReason": 2, - "avgLogprobs": -0.006723951548337936 + "avgLogprobs": -0.006721805781126022 } ], "usageMetadata": { diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_chat_completions.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_chat_completions.py index accadeb856..79cf772a5d 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_chat_completions.py +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_chat_completions.py @@ -73,47 +73,7 @@ def test_vertexai_generate_content_error( @pytest.mark.vcr() -def test_chat_completion_extra_client_level_params( - span_exporter, instrument_no_content -): - generation_config = GenerationConfig( - top_k=2, - top_p=0.95, - temperature=0.2, - stop_sequences=["\n\n\n"], - max_output_tokens=5, - presence_penalty=-1.5, - frequency_penalty=1.0, - seed=12345, - ) - model = GenerativeModel("gemini-1.5-flash-002") - model.generate_content( - [ - Content(role="user", parts=[Part.from_text("Say this is a test")]), - ], - generation_config=generation_config, - ) - - spans = span_exporter.get_finished_spans() - assert len(spans) == 1 - assert dict(spans[0].attributes) == { - "gen_ai.openai.request.seed": 12345, - "gen_ai.operation.name": "chat", - "gen_ai.request.frequency_penalty": 1.0, - "gen_ai.request.max_tokens": 5, - "gen_ai.request.model": "gemini-1.5-flash-002", - "gen_ai.request.presence_penalty": -1.5, - "gen_ai.request.stop_sequences": ("\n\n\n",), - "gen_ai.request.temperature": 0.2, - "gen_ai.request.top_p": 0.95, - "gen_ai.system": "vertex_ai", - } - - -@pytest.mark.vcr() -def test_chat_completion_extra_call_level_params( - span_exporter, instrument_no_content -): +def test_chat_completion_extra_params(span_exporter, instrument_no_content): generation_config = GenerationConfig( top_k=2, top_p=0.95, @@ -142,7 +102,7 @@ def test_chat_completion_extra_call_level_params( "gen_ai.request.model": "gemini-1.5-flash-002", "gen_ai.request.presence_penalty": -1.5, "gen_ai.request.stop_sequences": ("\n\n\n",), - "gen_ai.request.temperature": 0.2, - "gen_ai.request.top_p": 0.95, + "gen_ai.request.temperature": 0.20000000298023224, + "gen_ai.request.top_p": 0.949999988079071, "gen_ai.system": "vertex_ai", } From dd573890f81262b9be028e0d5840d85363cae313 Mon Sep 17 00:00:00 2001 From: Aaron Abbott Date: Fri, 17 Jan 2025 19:33:16 +0000 Subject: [PATCH 06/11] Comment out seed for now --- .../opentelemetry/instrumentation/vertexai/utils.py | 10 ++++++---- .../tests/test_chat_completions.py | 1 - 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py index d80d3b29da..7b715e9f9a 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py @@ -82,10 +82,12 @@ def get_genai_request_attributes( attributes[GenAIAttributes.GEN_AI_REQUEST_FREQUENCY_PENALTY] = ( generation_config.frequency_penalty ) - if "seed" in generation_config: - attributes[GenAIAttributes.GEN_AI_OPENAI_REQUEST_SEED] = ( - generation_config.seed - ) + # Uncomment once GEN_AI_REQUEST_SEED is released in 1.30 + # https://github.com/open-telemetry/semantic-conventions/pull/1710 + # if "seed" in generation_config: + # attributes[GenAIAttributes.GEN_AI_REQUEST_SEED] = ( + # generation_config.seed + # ) if "stop_sequences" in generation_config: attributes[GenAIAttributes.GEN_AI_REQUEST_STOP_SEQUENCES] = ( generation_config.stop_sequences diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_chat_completions.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_chat_completions.py index 79cf772a5d..837caee266 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_chat_completions.py +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_chat_completions.py @@ -95,7 +95,6 @@ def test_chat_completion_extra_params(span_exporter, instrument_no_content): spans = span_exporter.get_finished_spans() assert len(spans) == 1 assert dict(spans[0].attributes) == { - "gen_ai.openai.request.seed": 12345, "gen_ai.operation.name": "chat", "gen_ai.request.frequency_penalty": 1.0, "gen_ai.request.max_tokens": 5, From c84c53222c6a21089c317761575e3361c44ae18f Mon Sep 17 00:00:00 2001 From: Aaron Abbott Date: Fri, 17 Jan 2025 19:44:40 +0000 Subject: [PATCH 07/11] Remove unnecessary dict.get() calls --- .../src/opentelemetry/instrumentation/vertexai/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py index 7b715e9f9a..c0bff12824 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py @@ -119,6 +119,6 @@ def is_content_enabled() -> bool: def get_span_name(span_attributes: Mapping[str, AttributeValue]): - name = span_attributes.get(GenAIAttributes.GEN_AI_OPERATION_NAME, "") - model = span_attributes.get(GenAIAttributes.GEN_AI_REQUEST_MODEL, "") + name = span_attributes[GenAIAttributes.GEN_AI_OPERATION_NAME] + model = span_attributes[GenAIAttributes.GEN_AI_REQUEST_MODEL] return f"{name} {model}" From 874098f934dec6df4f4a141124957a6f329bf831 Mon Sep 17 00:00:00 2001 From: Aaron Abbott Date: Fri, 17 Jan 2025 21:56:29 +0000 Subject: [PATCH 08/11] Typing improvements to check that we support both v1 and v1beta1 --- .../instrumentation/vertexai/patch.py | 35 +++++++++++-------- .../instrumentation/vertexai/utils.py | 29 ++++++++++----- 2 files changed, 42 insertions(+), 22 deletions(-) diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py index d2c7b98adc..8f293ecb00 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py @@ -18,10 +18,7 @@ TYPE_CHECKING, Any, Callable, - Iterable, MutableSequence, - Optional, - Union, ) from opentelemetry._events import EventLogger @@ -33,15 +30,19 @@ from opentelemetry.trace import SpanKind, Tracer if TYPE_CHECKING: + from google.cloud.aiplatform_v1.services.prediction_service import client from google.cloud.aiplatform_v1.types import ( content, prediction_service, ) - from vertexai.generative_models import ( - GenerationResponse, + from google.cloud.aiplatform_v1beta1.services.prediction_service import ( + client as client_v1beta1, ) - from vertexai.generative_models._generative_models import ( - _GenerativeModel, + from google.cloud.aiplatform_v1beta1.types import ( + content as content_v1beta1, + ) + from google.cloud.aiplatform_v1beta1.types import ( + prediction_service as prediction_service_v1beta1, ) @@ -49,12 +50,15 @@ # https://github.com/googleapis/python-aiplatform/blob/v1.76.0/google/cloud/aiplatform_v1/services/prediction_service/client.py#L2088 # to handle named vs positional args robustly def _extract_params( - request: Optional[ - Union[prediction_service.GenerateContentRequest, dict[Any, Any]] - ] = None, + request: prediction_service.GenerateContentRequest + | prediction_service_v1beta1.GenerateContentRequest + | dict[Any, Any] + | None = None, *, - model: Optional[str] = None, - contents: Optional[MutableSequence[content.Content]] = None, + model: str | None = None, + contents: MutableSequence[content.Content] + | MutableSequence[content_v1beta1.Content] + | None = None, **_kwargs: Any, ) -> GenerateContentParams: # Request vs the named parameters are mututally exclusive or the RPC will fail @@ -86,9 +90,12 @@ def generate_content_create( def traced_method( wrapped: Callable[ - ..., GenerationResponse | Iterable[GenerationResponse] + ..., + prediction_service.GenerateContentResponse + | prediction_service_v1beta1.GenerateContentResponse, ], - instance: _GenerativeModel, + instance: client.PredictionServiceClient + | client_v1beta1.PredictionServiceClient, args: Any, kwargs: Any, ): diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py index c0bff12824..183a445f3d 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py @@ -20,7 +20,6 @@ from typing import ( TYPE_CHECKING, Mapping, - Optional, Sequence, ) @@ -31,18 +30,32 @@ if TYPE_CHECKING: from google.cloud.aiplatform_v1.types import content, tool + from google.cloud.aiplatform_v1beta1.types import ( + content as content_v1beta1, + ) + from google.cloud.aiplatform_v1beta1.types import ( + tool as tool_v1beta1, + ) @dataclass(frozen=True) class GenerateContentParams: model: str - contents: Optional[Sequence[content.Content]] = None - system_instruction: Optional[content.Content | None] = None - tools: Optional[Sequence[tool.Tool]] = None - tool_config: Optional[tool.ToolConfig] = None - labels: Optional[Mapping[str, str]] = None - safety_settings: Optional[Sequence[content.SafetySetting]] = None - generation_config: Optional[content.GenerationConfig] = None + contents: ( + Sequence[content.Content] | Sequence[content_v1beta1.Content] | None + ) = None + system_instruction: content.Content | content_v1beta1.Content | None = None + tools: Sequence[tool.Tool] | Sequence[tool_v1beta1.Tool] | None = None + tool_config: tool.ToolConfig | tool_v1beta1.ToolConfig | None = None + labels: Mapping[str, str] | None = None + safety_settings: ( + Sequence[content.SafetySetting] + | Sequence[content_v1beta1.SafetySetting] + | None + ) = None + generation_config: ( + content.GenerationConfig | content_v1beta1.GenerationConfig | None + ) = None def get_genai_request_attributes( From 5bed3360a6da3dbffe52910e25d573cfac602d7a Mon Sep 17 00:00:00 2001 From: Aaron Abbott Date: Tue, 21 Jan 2025 17:11:52 +0000 Subject: [PATCH 09/11] Add more teest cases for error conditions and fix span name bug --- .../instrumentation/vertexai/utils.py | 2 + ...ontent.yaml => test_generate_content.yaml} | 2 +- ...> test_generate_content_extra_params.yaml} | 0 ...generate_content_invalid_temperature.yaml} | 0 .../test_generate_content_missing_model.yaml | 56 +++++++++++++ .../tests/test_chat_completions.py | 82 ++++++++++++++++--- 6 files changed, 131 insertions(+), 11 deletions(-) rename instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/{test_vertexai_generate_content.yaml => test_generate_content.yaml} (96%) rename instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/{test_chat_completion_extra_params.yaml => test_generate_content_extra_params.yaml} (100%) rename instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/{test_vertexai_generate_content_error.yaml => test_generate_content_invalid_temperature.yaml} (100%) create mode 100644 instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_generate_content_missing_model.yaml diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py index 183a445f3d..da7d6d483a 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py @@ -134,4 +134,6 @@ def is_content_enabled() -> bool: def get_span_name(span_attributes: Mapping[str, AttributeValue]): name = span_attributes[GenAIAttributes.GEN_AI_OPERATION_NAME] model = span_attributes[GenAIAttributes.GEN_AI_REQUEST_MODEL] + if not model: + return name return f"{name} {model}" diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_vertexai_generate_content.yaml b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_generate_content.yaml similarity index 96% rename from instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_vertexai_generate_content.yaml rename to instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_generate_content.yaml index ea733fd1c0..69856f9308 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_vertexai_generate_content.yaml +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_generate_content.yaml @@ -43,7 +43,7 @@ interactions: ] }, "finishReason": 1, - "avgLogprobs": -0.0054909539850134595 + "avgLogprobs": -0.005692833348324424 } ], "usageMetadata": { diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_chat_completion_extra_params.yaml b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_generate_content_extra_params.yaml similarity index 100% rename from instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_chat_completion_extra_params.yaml rename to instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_generate_content_extra_params.yaml diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_vertexai_generate_content_error.yaml b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_generate_content_invalid_temperature.yaml similarity index 100% rename from instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_vertexai_generate_content_error.yaml rename to instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_generate_content_invalid_temperature.yaml diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_generate_content_missing_model.yaml b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_generate_content_missing_model.yaml new file mode 100644 index 0000000000..efe3e152ce --- /dev/null +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/cassettes/test_generate_content_missing_model.yaml @@ -0,0 +1,56 @@ +interactions: +- request: + body: |- + { + "contents": [ + { + "role": "user", + "parts": [ + { + "text": "Say this is a test" + } + ] + } + ] + } + headers: + Accept: + - '*/*' + Accept-Encoding: + - gzip, deflate + Connection: + - keep-alive + Content-Length: + - '141' + Content-Type: + - application/json + User-Agent: + - python-requests/2.32.3 + method: POST + uri: https://us-central1-aiplatform.googleapis.com/v1/projects/fake-project/locations/us-central1/publishers/google/models/gemini-does-not-exist:generateContent?%24alt=json%3Benum-encoding%3Dint + response: + body: + string: |- + { + "error": { + "code": 404, + "message": "Publisher Model `projects/otel-starter-project/locations/us-central1/publishers/google/models/gemini-does-not-exist` not found.", + "status": "NOT_FOUND", + "details": [] + } + } + headers: + Content-Type: + - application/json; charset=UTF-8 + Transfer-Encoding: + - chunked + Vary: + - Origin + - X-Origin + - Referer + content-length: + - '672' + status: + code: 404 + message: Not Found +version: 1 diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_chat_completions.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_chat_completions.py index 837caee266..95f918c314 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_chat_completions.py +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_chat_completions.py @@ -1,5 +1,5 @@ import pytest -from google.api_core.exceptions import BadRequest +from google.api_core.exceptions import BadRequest, NotFound from vertexai.generative_models import ( Content, GenerationConfig, @@ -8,6 +8,7 @@ ) from opentelemetry.instrumentation.vertexai import VertexAIInstrumentor +from opentelemetry.sdk.trace import ReadableSpan from opentelemetry.sdk.trace.export.in_memory_span_exporter import ( InMemorySpanExporter, ) @@ -15,7 +16,7 @@ @pytest.mark.vcr -def test_vertexai_generate_content( +def test_generate_content( span_exporter: InMemorySpanExporter, instrument_with_content: VertexAIInstrumentor, ): @@ -37,7 +38,65 @@ def test_vertexai_generate_content( @pytest.mark.vcr -def test_vertexai_generate_content_error( +def test_generate_content_empty_model( + span_exporter: InMemorySpanExporter, + instrument_with_content: VertexAIInstrumentor, +): + model = GenerativeModel("") + try: + model.generate_content( + [ + Content( + role="user", parts=[Part.from_text("Say this is a test")] + ) + ], + ) + except ValueError: + pass + + spans = span_exporter.get_finished_spans() + assert len(spans) == 1 + assert spans[0].name == "chat" + # Captures invalid params + assert dict(spans[0].attributes) == { + "gen_ai.operation.name": "chat", + "gen_ai.request.model": "", + "gen_ai.system": "vertex_ai", + } + assert_span_error(spans[0]) + + +@pytest.mark.vcr +def test_generate_content_missing_model( + span_exporter: InMemorySpanExporter, + instrument_with_content: VertexAIInstrumentor, +): + model = GenerativeModel("gemini-does-not-exist") + try: + model.generate_content( + [ + Content( + role="user", parts=[Part.from_text("Say this is a test")] + ) + ], + ) + except NotFound: + pass + + spans = span_exporter.get_finished_spans() + assert len(spans) == 1 + assert spans[0].name == "chat gemini-does-not-exist" + # Captures invalid params + assert dict(spans[0].attributes) == { + "gen_ai.operation.name": "chat", + "gen_ai.request.model": "gemini-does-not-exist", + "gen_ai.system": "vertex_ai", + } + assert_span_error(spans[0]) + + +@pytest.mark.vcr +def test_generate_content_invalid_temperature( span_exporter: InMemorySpanExporter, instrument_with_content: VertexAIInstrumentor, ): @@ -64,16 +123,11 @@ def test_vertexai_generate_content_error( "gen_ai.request.temperature": 1000.0, "gen_ai.system": "vertex_ai", } - # Sets error status - assert spans[0].status.status_code == StatusCode.ERROR - - # Records exception event - assert len(spans[0].events) == 1 - assert spans[0].events[0].name == "exception" + assert_span_error(spans[0]) @pytest.mark.vcr() -def test_chat_completion_extra_params(span_exporter, instrument_no_content): +def test_generate_content_extra_params(span_exporter, instrument_no_content): generation_config = GenerationConfig( top_k=2, top_p=0.95, @@ -105,3 +159,11 @@ def test_chat_completion_extra_params(span_exporter, instrument_no_content): "gen_ai.request.top_p": 0.949999988079071, "gen_ai.system": "vertex_ai", } + + +def assert_span_error(span: ReadableSpan) -> None: + # Sets error status + assert span.status.status_code == StatusCode.ERROR + # Records exception event + error_events = [e for e in span.events if e.name == "exception"] + assert error_events != [] From 18e8fc2e9f18cce345e243c4c64b56930a878949 Mon Sep 17 00:00:00 2001 From: Aaron Abbott Date: Tue, 21 Jan 2025 18:47:49 +0000 Subject: [PATCH 10/11] fix typing --- .../src/opentelemetry/instrumentation/vertexai/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py index da7d6d483a..96d7125028 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/utils.py @@ -131,9 +131,9 @@ def is_content_enabled() -> bool: return capture_content.lower() == "true" -def get_span_name(span_attributes: Mapping[str, AttributeValue]): +def get_span_name(span_attributes: Mapping[str, AttributeValue]) -> str: name = span_attributes[GenAIAttributes.GEN_AI_OPERATION_NAME] model = span_attributes[GenAIAttributes.GEN_AI_REQUEST_MODEL] if not model: - return name + return f"{name}" return f"{name} {model}" From 28d6ffd1b339513db3d26c6bb4f8d22aadca123b Mon Sep 17 00:00:00 2001 From: Aaron Abbott Date: Wed, 22 Jan 2025 17:29:44 +0000 Subject: [PATCH 11/11] Add todos for error.type --- .../src/opentelemetry/instrumentation/vertexai/patch.py | 2 ++ .../tests/test_chat_completions.py | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py index 8f293ecb00..36a31045b5 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/src/opentelemetry/instrumentation/vertexai/patch.py @@ -115,6 +115,8 @@ def traced_method( # message_to_event(message, capture_content) # ) + # TODO: set error.type attribute + # https://github.com/open-telemetry/semantic-conventions/blob/main/docs/gen-ai/gen-ai-spans.md result = wrapped(*args, **kwargs) # TODO: handle streaming # if is_streaming(kwargs): diff --git a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_chat_completions.py b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_chat_completions.py index 95f918c314..63a2e2c2d1 100644 --- a/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_chat_completions.py +++ b/instrumentation-genai/opentelemetry-instrumentation-vertexai/tests/test_chat_completions.py @@ -164,6 +164,10 @@ def test_generate_content_extra_params(span_exporter, instrument_no_content): def assert_span_error(span: ReadableSpan) -> None: # Sets error status assert span.status.status_code == StatusCode.ERROR + + # TODO: check thate error.type is set + # https://github.com/open-telemetry/semantic-conventions/blob/main/docs/gen-ai/gen-ai-spans.md + # Records exception event error_events = [e for e in span.events if e.name == "exception"] assert error_events != []