Add metrics to the Python OpenAI instrumentation #3180

drewby · 2025-01-10T04:14:56Z

Description

This PR implements the GenAI semantic conventions for the two client metrics so they are collected along with spans when instrumenting a Python application.

Basic implementation of two client metrics defined in the GenAI semantic conventions:

gen_ai.client.token.usage - Documentation
gen_ai.client.operation.duration - Documentation

There is an example added to show end users who to configure the explicit bucket boundaries as defined in the semantic convention spec.

Fixes #3177

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

test_chat_completion_metrics - tests that metrics are correctly emitted when calling OpenAI synchronously.
test_async_chat_completion_metrics - tests that metrics are correctly emitted when calling OpenAI asynchronously.

Does This PR Require a Core Repo Change?

Yes. - Link to PR:
No.

Checklist:

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

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

xrmx

A couple of nits but LGTM. Not sure that setting the views for having proper buckets it's useful but no big deal.

xrmx · 2025-01-10T08:00:34Z

instrumentation-genai/opentelemetry-instrumentation-openai-v2/CHANGELOG.md

@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Add example to `opentelemetry-instrumentation-openai-v2`
  ([#3006](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3006))
 - Support for `AsyncOpenAI/AsyncCompletions` ([#2984](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2984))
+- Add metrics to the Python OpenAI instrumentation ([#3180](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3180))


Suggested change

- Add metrics to the Python OpenAI instrumentation ([#3180](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3180))

- Add metrics ([#3180](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3180))

xrmx · 2025-01-10T08:03:27Z

...opentelemetry-instrumentation-openai-v2/src/opentelemetry/instrumentation/openai_v2/patch.py

@@ -23,6 +24,7 @@
 )
 from opentelemetry.trace import Span, SpanKind, Tracer

+from .meters import Meters  # Import the Meters class


Suggested change

from .meters import Meters # Import the Meters class

from .meters import Meters

xrmx · 2025-01-10T08:21:32Z

instrumentation-genai/opentelemetry-instrumentation-openai-v2/tests/conftest.py

@@ -52,6 +69,62 @@ def fixture_event_logger_provider(log_exporter):
    return event_logger_provider


+@pytest.fixture(scope="function", name="meter_provider")
+def fixture_meter_provider(metric_reader):
+    token_usage_histogram_view = View(


Am not sure adding the views is useful since it's outside of the instrumentation concern, maybe wait for advisory support to get in instead?

I think without setting them you'd not get any value from metrics - all measurements in sec go to the smallest bucket :( So I'd rather have them initially and remove as unnecessary once advisory params are in.

xrmx · 2025-01-10T08:25:14Z

instrumentation-genai/opentelemetry-instrumentation-openai-v2/examples/buckets/README.rst

@@ -0,0 +1,38 @@
+OpenTelemetry OpenAI Instrumentation Example


I'm not sure there is anything openai specific here, should we document the views more generally instead?

lmolkova

Looks great, just a few comments on recording errors

lmolkova · 2025-01-11T04:03:35Z

...pentelemetry-instrumentation-openai-v2/src/opentelemetry/instrumentation/openai_v2/meters.py

+from opentelemetry.semconv._incubating.metrics import gen_ai_metrics
+
+
+class Meters:


nit:

Suggested change

class Meters:

class Instruments:

would be more precise

lmolkova · 2025-01-11T04:04:39Z

...opentelemetry-instrumentation-openai-v2/src/opentelemetry/instrumentation/openai_v2/patch.py

+                    meters,
+                    duration,
+                    result,
+                    span_attributes[GenAIAttributes.GEN_AI_REQUEST_MODEL],


we should pass error type and record it on the histogram

lmolkova · 2025-01-11T04:08:46Z

...opentelemetry-instrumentation-openai-v2/src/opentelemetry/instrumentation/openai_v2/patch.py

+
+        completion_attributes = {
+            **common_attributes,
+            GenAIAttributes.GEN_AI_TOKEN_TYPE: GenAIAttributes.GenAiTokenTypeValues.COMPLETION.value,


I think this one is deprecated, it should be OUTPUT, no?

lmolkova · 2025-01-11T04:09:09Z

...opentelemetry-instrumentation-openai-v2/src/opentelemetry/instrumentation/openai_v2/patch.py

+        GenAIAttributes.GEN_AI_OPERATION_NAME: GenAIAttributes.GenAiOperationNameValues.CHAT.value,
+        GenAIAttributes.GEN_AI_SYSTEM: GenAIAttributes.GenAiSystemValues.OPENAI.value,
+        GenAIAttributes.GEN_AI_REQUEST_MODEL: request_model,
+    }


we also need to record error.type, server.address and server.port similarly to spans

https://github.com/open-telemetry/semantic-conventions/blob/main/docs/gen-ai/gen-ai-metrics.md#metric-gen_aiclientoperationduration

Also we have extra attributes defined for openai - https://github.com/open-telemetry/semantic-conventions/blob/main/docs/gen-ai/openai.md#metric-gen_aiclientoperationduration - can we populate them here too?

lmolkova · 2025-01-11T04:11:20Z

...mentation-genai/opentelemetry-instrumentation-openai-v2/tests/test_async_chat_completions.py

+
+@pytest.mark.vcr()
+@pytest.mark.asyncio()
+async def test_async_chat_completion_metrics(


it'd be nice to add metrics checks for failure cases (or just update existing tests to record and assert metrics too)

lmolkova · 2025-01-11T04:15:01Z

instrumentation-genai/opentelemetry-instrumentation-openai-v2/tests/conftest.py

@@ -52,6 +69,62 @@ def fixture_event_logger_provider(log_exporter):
    return event_logger_provider


+@pytest.fixture(scope="function", name="meter_provider")
+def fixture_meter_provider(metric_reader):
+    token_usage_histogram_view = View(


I think without setting them you'd not get any value from metrics - all measurements in sec go to the smallest bucket :( So I'd rather have them initially and remove as unnecessary once advisory params are in.

drewby added 6 commits January 7, 2025 07:56

Add metrics to instrumentation

1e3fd4d

Use semconv for metric

ca3659f

Add histogram buckets test

11d0a4d

Add histogram explicit boundaries example

fd22316

Merge branch 'open-telemetry:main' into main

aa82c1e

Update documentation

c156fe0

drewby requested a review from a team as a code owner January 10, 2025 04:14

github-actions bot assigned alizenhom, codefromthecrypt, gyliu513, karthikscale3, lmolkova, lzchen and nirga Jan 10, 2025

github-actions bot requested review from alizenhom, codefromthecrypt, gyliu513, karthikscale3, lmolkova, lzchen and nirga January 10, 2025 04:15

drewby added 3 commits January 10, 2025 04:15

Update CHANGELOG

b77a4dc

Fix linting errors

1e0d6b7

Use metric name constant

bf16dfe

xrmx approved these changes Jan 10, 2025

View reviewed changes

xrmx reviewed Jan 10, 2025

View reviewed changes

lmolkova reviewed Jan 11, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add metrics to the Python OpenAI instrumentation #3180

Add metrics to the Python OpenAI instrumentation #3180

drewby commented Jan 10, 2025 •

edited

Loading

xrmx left a comment

xrmx Jan 10, 2025

xrmx Jan 10, 2025

xrmx Jan 10, 2025

lmolkova Jan 11, 2025

xrmx Jan 10, 2025

lmolkova left a comment

lmolkova Jan 11, 2025

lmolkova Jan 11, 2025

lmolkova Jan 11, 2025

lmolkova Jan 11, 2025

lmolkova Jan 11, 2025

lmolkova Jan 11, 2025

	- Add metrics to the Python OpenAI instrumentation ([#3180](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3180))
	- Add metrics ([#3180](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3180))

	from .meters import Meters # Import the Meters class
	from .meters import Meters

		@@ -0,0 +1,38 @@
		OpenTelemetry OpenAI Instrumentation Example

		from opentelemetry.semconv._incubating.metrics import gen_ai_metrics


		class Meters:

Add metrics to the Python OpenAI instrumentation #3180

Are you sure you want to change the base?

Add metrics to the Python OpenAI instrumentation #3180

Conversation

drewby commented Jan 10, 2025 • edited Loading

Description

Type of change

How Has This Been Tested?

Does This PR Require a Core Repo Change?

Checklist:

xrmx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmolkova left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drewby commented Jan 10, 2025 •

edited

Loading