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

fix analytics with new format #12889

Merged
merged 14 commits into from
Apr 23, 2024
Merged

fix analytics with new format #12889

merged 14 commits into from
Apr 23, 2024

Conversation

AntoineJac
Copy link
Contributor

@AntoineJac AntoineJac commented Apr 19, 2024

Summary

This PR is a fixed of #12583
We need to refactor as many observability providers do not simplify ingest array. Here is a new nested format:

Current model is:

  "ai": {
    "mistral": {
      "request_total_tokens": 648,
      "number_of_instances": 1,
      "instances": [
        {
          "meta": {
            "request_model": "mistral-tiny",
            "plugin_id": "1c783122-0223-4a03-9f1d-c71d2baab01e",
            "response_model": "mistral-tiny",
            "provider_name": "mistral"
          },
          "usage": {
            "prompt_token": 392,
            "total_tokens": 648,
            "completion_token": 256
          }
        }
      ],
      "request_prompt_tokens": 392,
      "request_completion_tokens": 256
    },
    "azure": {
      "request_total_tokens": 145,
      "number_of_instances": 1,
      "instances": [
        {
          "meta": {
            "request_model": "gpt-35-turbo",
            "plugin_id": "5df193be-47a3-4f1b-8c37-37e31af0568b",
            "response_model": "gpt-35-turbo",
            "provider_name": "azure"
          },
          "usage": {
            "prompt_token": 89,
            "total_tokens": 145,
            "completion_token": 56
          }
        }
      ],
      "request_prompt_tokens": 89,
      "request_completion_tokens": 56
    },
    "cohere": {
      "request_total_tokens": 149,
      "number_of_instances": 1,
      "instances": [
        {
          "meta": {
            "request_model": "command",
            "plugin_id": "30a63ab9-03c6-447a-9b25-195eb0acaeb2",
            "response_model": "command",
            "provider_name": "cohere"
          },
          "usage": {
            "prompt_token": 78,
            "total_tokens": 149,
            "completion_token": 71
          }
        }
      ],
      "request_prompt_tokens": 78,
      "request_completion_tokens": 71
    }
  }

New model if using several plugins:

  "ai": {
    "ai-request-transformer": {
      "meta": {
        "request_model": "gpt-35-turbo",
        "provider_name": "azure",
        "response_model": "gpt-35-turbo",
        "plugin_id": "5df193be-47a3-4f1b-8c37-37e31af0568b"
      },
      "payload": {},
      "usage": {
        "prompt_token": 89,
        "completion_token": 56,
        "total_tokens": 145
      }
    },
    "ai-response-transformer": {
      "meta": {
        "request_model": "mistral-tiny",
        "provider_name": "mistral",
        "response_model": "mistral-tiny",
        "plugin_id": "1c783122-0223-4a03-9f1d-c71d2baab01e"
      },
      "payload": {},
      "usage": {
        "prompt_token": 168,
        "completion_token": 180,
        "total_tokens": 348
      }
    },
    "ai-proxy": {
      "meta": {
        "request_model": "gpt-35-turbo",
        "provider_name": "azure",
        "response_model": "gpt-35-turbo",
        "plugin_id": "546c3856-24b3-469a-bd6c-f6083babd2cd"
      },
      "payload": {},
      "usage": {
        "prompt_token": 28,
        "completion_token": 14,
        "total_tokens": 42
      }
    }
  },

Checklist

Issue reference

Fix #[issue number]

KAG-3759
KAG-4124

EDIT: format JSON snippets

Copy link
Contributor

@gruceo gruceo left a comment

Choose a reason for hiding this comment

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

looks good, can you change the commit description according to the convention?

@brentos brentos self-requested a review April 22, 2024 22:30
@AntoineJac
Copy link
Contributor Author

@gruceo can you please merge to Kong and Kong-ee if it looks ok? Thanks

@tysoekong
Copy link
Contributor

I can handle the Kong-EE cherry-picking and stuff, I have been set on to this to get everything done before next week.

Just need this merging to allow proceeding the other remaining PRs.

@tysoekong
Copy link
Contributor

tysoekong commented Apr 23, 2024

Set rebase when merging master back in; my fix conflicts now...

@tysoekong
Copy link
Contributor

Tests should pass now

@tysoekong tysoekong force-pushed the feat/ai-format-analytics-fix branch from 995d5b1 to 97ce4fb Compare April 23, 2024 12:17
@tysoekong
Copy link
Contributor

Wait I have pulled in commits now in this PR

@tysoekong tysoekong force-pushed the feat/ai-format-analytics-fix branch from 97ce4fb to bc4967d Compare April 23, 2024 12:42
@ttyS0e
Copy link
Contributor

ttyS0e commented Apr 23, 2024

@gruceo ready!

Copy link
Contributor

@gruceo gruceo left a comment

Choose a reason for hiding this comment

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

LGTM, wait for @brentos to approve then we can merge.

@gruceo
Copy link
Contributor

gruceo commented Apr 23, 2024

@tysoekong @ttyS0e there are 14 commits in this PR, so we should squash.

@tysoekong
Copy link
Contributor

@brentos Do you approve also?

Copy link
Contributor

@brentos brentos left a comment

Choose a reason for hiding this comment

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

LGTM!

@tysoekong
Copy link
Contributor

@gruceo Will it let you push squash and merge?

@gruceo gruceo merged commit 87bdb50 into master Apr 23, 2024
25 checks passed
@gruceo gruceo deleted the feat/ai-format-analytics-fix branch April 23, 2024 18:10
@team-gateway-bot
Copy link
Collaborator

Successfully created cherry-pick PR for master:

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

Successfully merging this pull request may close these issues.

7 participants