Skip to content
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

feat: write open telemetry semantic conventions for AI metrics and span attributes #1601

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

bryanatkinson
Copy link
Contributor

Write Metrics and Span attributes matching the Semantic Conventions for Generative AI systems

  • Pull the base metric Histogram helper out of the GCP plugin
  • Write these conventions regardless of the telemetry export destination
  • Provide configuration options to disable

Checklist (if applicable):

};
return timedResponse;
});
return runner(input, getStreamingCallback(registry)).then(
Copy link
Member

Choose a reason for hiding this comment

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

Should we just use the await syntax here, since we're introducing it below (await writeSemConv...) so that we're consistent?

export interface SemConvOptions {
writeMetrics: boolean;
writeSpanAttributes: boolean;
}
Copy link
Member

Choose a reason for hiding this comment

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

What's the use case for turning this on and off at this level?

I am wondering if we should always write metrics, and then let the client/exporter control whether or not the metric actually gets sent anywhere. I think this is how could work? But correct me if I am off base.

Copy link
Member

Choose a reason for hiding this comment

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

Couple other thoughts...

  1. Semantic Convention is a generic term, there are many conventions for many things (e.g. HTTP) so if this is a flag we want, it should be specific as to what convention it refers to.
  2. I imagine we should have a generic way of allow-listing, deny-listing metrics that are exported, versus a metric specific flag.

system: 'vertex_ai',
requestModel: request.config?.version || model.version || name,
responseModel: response['modelVersion'],
operationName: 'image_generation',
Copy link
Member

Choose a reason for hiding this comment

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

should this be the actual endpoint being called? if so, I think its predict

system: 'google_ai',
requestModel: modelName,
responseModel: response['modelVersion'],
operationName: 'chat',
Copy link
Member

Choose a reason for hiding this comment

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

Is startChat in the SDK a veneer on top of generateContent w/ session management? I am iffy what the operationName should be.

  1. Genkit Operation
  2. Gemini SDK operation
  3. Underlying Gemini API operation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants