Uh oh!
There was an error while loading. Please reload this page.
feat(routing): tag/metadata conditional routing - #686
Merged
Conversation
📝 WalkthroughWalkthroughThis PR adds tag-based routing to RoutingTarget, allowing targets to declare tags and a "default" fallback tag. Client requests can supply routing tags via a new header, which propagate through dispatch call sites into ChangesTag-based routing
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ClientContext
participant Dispatch
participant resolve_attempt_models
participant RoutingRegistry
Client->>ClientContext: request with x-aisix-routing-tags header
ClientContext->>ClientContext: parse_routing_tags(header)
ClientContext-->>Dispatch: routing_tags: Vec<String>
Dispatch->>resolve_attempt_models: resolve_attempt_models(..., request_tags)
resolve_attempt_models->>resolve_attempt_models: eligible_targets(routing.targets, request_tags)
alt no eligible targets
resolve_attempt_models-->>Dispatch: ProxyError::InvalidRequest
else eligible targets found
resolve_attempt_models->>RoutingRegistry: pick_targets(eligible subset)
RoutingRegistry-->>resolve_attempt_models: selected targets
resolve_attempt_models-->>Dispatch: attempt models
end
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/e2e/src/cases/tag-routing-e2e.test.ts (1)
94-136: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing e2e coverage for the "no match, no default" error path.
The PR states that when tags match nothing and no
defaulttarget exists, an error is returned, but this test only exercises configurations that always include atag-defaulttarget. Consider adding a scenario with nodefault-tagged target and a request tag that matches nothing, asserting the request fails (e.g., a 400/invalid-request response).As per coding guidelines,
**/*.{test,spec}.{js,ts,jsx,tsx}: "Tests must cover boundary cases (empty values, min/max), invalid inputs, combination scenarios, and extreme cases".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/src/cases/tag-routing-e2e.test.ts` around lines 94 - 136, The e2e test in tag-routing should also cover the failure path where no target matches and no `default` target exists. Extend the existing `selects the tagged target, defaulting when unmatched or untagged` scenario, or add a new case рядом with `askWithTags`/`admin.createModel`, that creates a routing config without any `default`-tagged target, sends a request with an unmatched tag like “apac”, and asserts the request is rejected with an invalid-request/400-style error. Use the existing `startOpenAiUpstream`, `createOpenAiModel`, and `waitConfigPropagation` helpers so the new boundary case stays consistent with the current test setup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/aisix-proxy/src/client_ip.rs`:
- Around line 180-187: The routing-tag parsing in parse_routing_tags currently
preserves case, which can cause exact-match lookups in
RoutingTarget::matches_request_tags and is_default_target to miss values like EU
vs eu. Normalize each parsed tag at the header parse boundary by converting to a
consistent case before collecting, so downstream tag matching behaves
case-insensitively and does not incorrectly fall back to default.
---
Nitpick comments:
In `@tests/e2e/src/cases/tag-routing-e2e.test.ts`:
- Around line 94-136: The e2e test in tag-routing should also cover the failure
path where no target matches and no `default` target exists. Extend the existing
`selects the tagged target, defaulting when unmatched or untagged` scenario, or
add a new case рядом with `askWithTags`/`admin.createModel`, that creates a
routing config without any `default`-tagged target, sends a request with an
unmatched tag like “apac”, and asserts the request is rejected with an
invalid-request/400-style error. Use the existing `startOpenAiUpstream`,
`createOpenAiModel`, and `waitConfigPropagation` helpers so the new boundary
case stays consistent with the current test setup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3f2c66c3-d86a-4dc7-a6c3-d356a838b69f
📒 Files selected for processing (10)
crates/aisix-core/src/models/routing.rscrates/aisix-proxy/src/chat.rscrates/aisix-proxy/src/client_ip.rscrates/aisix-proxy/src/count_tokens.rscrates/aisix-proxy/src/messages.rscrates/aisix-proxy/src/responses.rscrates/aisix-proxy/src/routing.rsschemas/resources/model.schema.jsonschemas/resources/routing.schema.jsontests/e2e/src/cases/tag-routing-e2e.test.ts
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds tag/metadata-conditional routing: a routing model's targets can carry
tags, and a request selects among them by the routing tags it carries. Closes the conditional/tag-routing gap vs LiteLLM / Portkey / Kong (routing gap list #873).How
Request tags come from an out-of-band header,
x-aisix-routing-tags(comma-separated), resolved once by the existingClientContextextractor alongside client IP / User-Agent. They're read from the headers, never the body, so they never reach the upstream request payload.Filtering happens in the shared
resolve_attempt_models, as a pre-filter that narrows the target set before the configured strategy orders it — so tag routing composes with round-robin / weighted / failover / least-*, rather than being a new strategy.eligible_targetsmirrors LiteLLM'stag_based_routing:"default"-tagged targets."default"-tagged targets if any, else all.defaultconfigured → a "no target matches tags" error.RoutingTargetgains an optionaltags: Vec<String>(additive, serde-default, so existing configs are unaffected).count_tokens'sdispatchnow takes the wholeClientContext(it previously took justsource_ip), matching the other three routing-capable endpoints.LiteLLM baseline
Semantics follow
litellm/router_strategy/tag_based_routing.py(match-any intersect +defaultfallback). One deliberate divergence: LiteLLM sources tags from the request body (metadata.tags); we use a header. Our bridges serialize the whole request body (including passthroughextra) to the upstream, so a bodymetadatafield would be forwarded and rejected by providers that constrainmetadata(OpenAI requires string values). A header keeps the routing signal out of the upstream payload without a body-stripping layer. The matching/fallback behavior is unchanged from LiteLLM.Tests
RoutingTargettag predicates +tagsdeserialization.eligible_targetsacross all branches (no-tags passthrough, match-any, tagged-no-match→default, untagged→default, untagged-no-default→all, tagged-no-match-no-default→empty), and header parsing.tag-routing-e2e.test.ts): a routing model witheu/us/defaulttargets —x-aisix-routing-tagsselects the matching upstream, an untagged request and an unmatched tag both fall back to default, and the header is asserted absent from the upstream body.Notes
Regenerated the committed resource schemas (
routing/model). User-facing docs ship as one consolidatedapi7/docsrouting-strategies page for the whole routing family.Fixes api7/AISIX-Cloud#927
Summary by CodeRabbit
New Features
Bug Fixes