-
Notifications
You must be signed in to change notification settings - Fork 67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add instrumentation for simple convenience chat calls #107
base: main
Are you sure you want to change the base?
Conversation
_duration = Stopwatch.StartNew(); | ||
_commonTags = new TagList | ||
{ | ||
{ Constants.GenAiSystemKey, Constants.GenAiSystemValue }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this fully static? Perhaps a better fit for Resource?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resource attributes are per-all-tracers + only otel SDK can set them up. Instrumentation scope attributes are not available on activity sources and in many other languages.
So it'd be nice to optimize it in the future, but it'd need changes in otel beyond gen-ai or even semconv.
|
||
1. Set instrumentation feature-flag using one of the following options: | ||
|
||
- set the `AZURE_EXPERIMENTAL_ENABLE_ACTIVITY_SOURCE` environment variable to `"true"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious, why its named "AZURE_..." ? Is this library connected to Azure SDKs in general?
calls made by your application including those done by the OpenAI SDK. | ||
Check out [OpenTelemetry documentation](https://opentelemetry.io/docs/languages/net/getting-started/) for more details. | ||
|
||
### Available sources and meters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it might not be obvious what are "sources" and "meters"..
{ | ||
private static readonly ActivitySource s_chatSource = new ActivitySource("OpenAI.ChatClient"); | ||
private static readonly Meter s_chatMeter = new Meter("OpenAI.ChatClient"); | ||
private static readonly Histogram<double> s_duration = s_chatMeter.CreateHistogram<double>(Constants.GenAiClientOperationDurationMetricName, "s", "Measures GenAI operation duration."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add a TODO to provide the explicit buckets, when DS 9.0 can be used.
if (inputTokensUsage != null) | ||
{ | ||
// tags is a struct, let's copy them | ||
TagList inputUsageTags = tags; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Token usage metrics has no attribute for error.type, but tags here will have error.type.
|
||
public TestActivityListener(string sourceName) | ||
{ | ||
_listener = new ActivityListener() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if the tests can take OTel dependency sdk, then consider using InMemoryExporter for these tests.
This PR builds foundation for OpenAI SDK tracing and metrics instrumentation (using Otel-compatible .NET primitives).
It's limited to convenience
ChatClient
methods without streaming. The PR implements instrumentation according to OpenTelemetry GenAI semantic conventions.The intention is to add instrumentation to other methods and client types and evolve it along with OTel GenAI semantic conventions.
TODO (in this PR):
TODO (in next PRs):