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

Extend GenAI System to include OpenAI compatible platforms #1655

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

Conversation

codefromthecrypt
Copy link

@codefromthecrypt codefromthecrypt commented Dec 6, 2024

This extends GenAI System to include OpenAI compatible platforms. These are usable by OpenAI client libraries:

At the moment, there is no way to attribute signals to Azure OpenAI, so people use "openai". At Elastic, we have different
dashboards for OpenAI (the platform) vs Azure OpenAI, as these are independent services. Lacking a GenAI system name, we have to resort to "openai" which is ambiguous between these two.

Using "server.address" isn't a great option due to subdomains used in the Azure OpenAI service depending on the application name in Azure. While we could use cloud semantics, they aren't guaranteed to be present, and aren't integrated in current code. Even if it were, this it is less ideal to navigate or aggregate on two attributes vs one.

This also adds a value for Google Gemini which has exactly the same concern as it can be accessed either via its native API or via OpenAI libraries.

Changes

Added the following, clarifying only when needed:

  • "az.ai.openai" to follow conventions of "az.ai.inference"
  • "gemini"", not "google.gemini" as "vertexai" is not prefixed either.
  • "perplexity"
  • "xai"
  • "deepseek"
  • "groq"

Merge requirement checklist

@codefromthecrypt
Copy link
Author

codefromthecrypt commented Dec 9, 2024

PS I used the Azure AI Inference client client per instructions to access Azure OpenAI service, and I guess what's happening here more generally, is that gen_ai.system might be conflated with the client itself.

image

Specifically, the Azure AI Inference client can access three different products:

  • GitHub models
  • Azure AI Studio models
  • Azure OpenAI Studio models

In each case the services are different (different subdomains, for example). However, status quo, all would set gen_ai.system=az.ai.inference

So, status quo gen_ai.system might be treated more like the SDK name than the actual system name. Should I update the text like that, as this applies even beyond openai compat?

@karthikscale3
Copy link
Contributor

This is definitely an issue that we have faced as well. Right now auto instrumentation default sets genai.system to openai even if the base_url used is different from the openai one. More broadly it's only looking at the client library being used as opposed to the inference server being used, which is the correct representation. In Langtrace's SDK, we capture the underlying server being used from the base url using regex and set a langtrace attribute called service-name. I think we can move this behavior up to the spec with genai.system. We also need to capture this for all the other LLM providers that support openai client library

  1. xai
  2. deepseek
  3. groq
  4. perplexity

@lmolkova
Copy link
Contributor

Base URL regex is not reliable either - you can run smthing behind the proxy, self-host model in vllm, your provider can host different models behind one API. Instrumentation can do the best effort, but cannot guarantee anything beyond "a system this library was designed for" on the gen_ai.system.

@codefromthecrypt codefromthecrypt changed the title Extend GenAI System to include Azure OpenAI and Gemini Extend GenAI System to include OpenAI compatible platforms Dec 10, 2024
@codefromthecrypt
Copy link
Author

@lmolkova want me to fix the "dead link" on https://blogs.oracle.com/linux/post/understanding-linux-kernel-memory-statistics? it is result code zero which I've seen elsewhere when a link takes too long to load or otherwise

@codefromthecrypt
Copy link
Author

The broken links might not be a new problem. We can work around this by retrying externally, in the make file, but I imagine for now, folks can just manually merge when MLC is flakey on a good link. #332 (comment)

@xrmx
Copy link
Contributor

xrmx commented Dec 10, 2024

The broken links might not be a new problem. We can work around this by retrying externally, in the make file, but I imagine for now, folks can just manually merge when MLC is flakey on a good link. #332 (comment)

It has been here since a few and has not been a problem to have things merged :)

Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Dec 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Needs More Approval
Development

Successfully merging this pull request may close these issues.

4 participants