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

Add genai system-specific conventions for Azure AI Inference #1393

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

Conversation

lmolkova
Copy link
Contributor

@lmolkova lmolkova commented Sep 5, 2024

Adds

  • az.ai.inference system name.
  • md file describing how generic conventions apply to Azure AI Inference

It's being implemented in Azure/azure-sdk-for-js#30800 and Azure/azure-sdk-for-python#36890, other languages to follow

Merge requirement checklist

- ref: gen_ai.request.model
requirement_level: required
requirement_level:
conditionally_required: If available.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Azure AI Inference allows to use specific model-as-a-service and does not always have a model name at request time.

Client interacts with the endpoint, but does not need to know the model.

So, changing level to conditionally_required to accommodate this scenario. Other system-specific conventions may override it and say it's required

Copy link
Member

Choose a reason for hiding this comment

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

Interesting. It used to be that the request would use the "deployment name" which then would have an underlying model configured in the service. Is that still the case?

Copy link
Contributor Author

@lmolkova lmolkova Sep 6, 2024

Choose a reason for hiding this comment

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

if you use serverless models, after you deploy one the only thing the client knows is the endpoint 🤷‍♀️

const client = ModelClient("https://contoso.eastus2.inference.ai.azure.com", credential);
  const response = await client.path("/chat/completions").post({
    body: {
      messages: [
        ...
      ],
    }
  });

@lmolkova lmolkova requested a review from a team September 5, 2024 22:15
model/registry/gen-ai.yaml Outdated Show resolved Hide resolved

## Azure AI Inference

`gen_ai.system` MUST be set to `"az.ai.inference"` and SHOULD be provided **at span creation time**.
Copy link
Contributor Author

@lmolkova lmolkova Sep 5, 2024

Choose a reason for hiding this comment

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

@truptiparkar7 could you please bless this name? Thanks!

ps: alternatives/suggestions are more than welcome

- ref: gen_ai.request.model
requirement_level: required
requirement_level:
conditionally_required: If available.
Copy link
Member

Choose a reason for hiding this comment

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

Interesting. It used to be that the request would use the "deployment name" which then would have an underlying model configured in the service. Is that still the case?

| [`error.type`](/docs/attributes-registry/error.md) | string | Describes a class of error the operation ended with. [2] | `timeout`; `java.net.UnknownHostException`; `server_certificate_invalid`; `500` | `Conditionally Required` if the operation ended in an error | ![Stable](https://img.shields.io/badge/-stable-lightgreen) |
| [`gen_ai.request.model`](/docs/attributes-registry/gen-ai.md) | string | The name of the GenAI model a request is being made to. [3] | `gpt-4` | `Conditionally Required` If available. | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| [`server.port`](/docs/attributes-registry/server.md) | int | GenAI server port. [4] | `80`; `8080`; `443` | `Conditionally Required` If not default (443). | ![Stable](https://img.shields.io/badge/-stable-lightgreen) |
| [`az.namespace`](/docs/attributes-registry/azure.md) | string | [Azure Resource Provider Namespace](https://learn.microsoft.com/azure/azure-resource-manager/management/azure-services-resource-providers) as recognized by the client. [5] | `Microsoft.CognitiveServices` | `Recommended` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
Copy link
Member

Choose a reason for hiding this comment

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

IMO it's a bit difficult to parse what is specific to the vendor conventions and what is repeated from the general conventions. I have the same concern with the openai conventions. Is there a way to highlight, or sort, or otherwise call out what is the additional attributes?

Copy link
Contributor Author

@lmolkova lmolkova Sep 6, 2024

Choose a reason for hiding this comment

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

no way to highlight now, but it's a good feature to have.

My assumption that users who use some specific vendor would come to the corresponding page and learn what this vendor reports (doesn't matter if it's generic or not).

I'm really curious to learn why you need to find what's different.

An easy way to to go read yaml which includes all customizations:

- id: trace.gen_ai.az.ai.inference.client
extends: trace.gen_ai.client.common_attributes
type: attribute_group
brief: >
Describes Azure AI Inference span attributes.
attributes:
- ref: az.namespace
note: >
When `az.namespace` attribute is populated, it MUST be set to `Microsoft.CognitiveServices` for all
operations performed by Azure AI Inference clients.
examples: ["Microsoft.CognitiveServices"]
- ref: gen_ai.usage.input_tokens
brief: >
The number of prompt tokens as reported in the usage prompt_tokens property of the response.
- ref: gen_ai.usage.output_tokens
brief: >
The number of completion tokens as reported in the usage completion_tokens property of the response.
- ref: server.port
requirement_level:
conditionally_required: If not default (443).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

3 participants