feat: add routing engine tracking to logs and telemetry#1593
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Caution Review failedThe pull request is closed. 📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds capture and propagation of routing_engine_used: new context key, DB migration and Log column, search/filter support, telemetry label and metrics, OpenAPI/docs updates, backend logging/telemetry changes to persist and surface value, and frontend UI/filter/icons to display and filter by routing engine. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Router as Governance/Routing
participant Context
participant Logger as Logging Plugin
participant DB
participant Telemetry
participant UI
Client->>Router: incoming request
Router->>Router: decide engine (routing-rule/governance/loadbalancing)
Router->>Context: set BifrostContextKeyRoutingEngineUsed
Router->>Logger: forward request/response
Logger->>Context: read routing_engine_used
Logger->>Logger: create/update Log with RoutingEngineUsed
Logger->>DB: insert/update log (routing_engine_used column)
DB-->>Logger: persist result
Telemetry->>Context: read routing_engine_used
Telemetry->>Telemetry: attach routing_engine_used label and record metrics
UI->>DB: GET /logs?routing_engine_used=...
DB-->>UI: return filtered logs + available routing_engines
UI->>UI: render badges, filters, and charts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
b91ad15 to
32412cb
Compare
|
@coderabbitai full-review |
|
✅ Actions performedFull review triggered. |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/logging/main.go (1)
309-339:⚠️ Potential issue | 🔴 CriticalThe pooled LogMessage reset logic is incomplete—RoutingEngineUsed and several other fields are not cleared.
The
putLogMessage()function inpool.goonly resets a subset of fields (Operation, RequestID, Timestamp, InitialData, UpdateData, StreamResponse) but omits RoutingEngineUsed, ParentRequestID, NumberOfRetries, FallbackIndex, SelectedKeyID, SelectedKeyName, VirtualKeyID, VirtualKeyName, RoutingRuleID, RoutingRuleName, Latency, and SemanticCacheDebug. This allows stale values to persist across pooled message reuses. Update the reset logic to clear all fields.
🤖 Fix all issues with AI agents
In `@docs/openapi/paths/management/logging.yaml`:
- Around line 45-49: Update the query parameter description for
routing_engine_used to list all possible values including "loadbalancing" (in
addition to "routing-rule" and "governance") so both endpoints reflect the
current emitted values; locate the parameter named routing_engine_used in the
OpenAPI paths (the occurrence around the shown diff and the repeated occurrence
referenced later) and change the description to mention the full set (e.g.,
"Comma-separated list of routing engines to filter by (routing-rule, governance,
or loadbalancing)").
In `@docs/openapi/schemas/management/logging.yaml`:
- Around line 40-43: Update the description for the routing_engine_used schema
(field name: routing_engine_used) to list all valid routing engine values
including "loadbalancing" in addition to "routing-rule" and "governance"; apply
the same description change to the other occurrences of routing_engine_used in
this file so all schema entries consistently document the three allowed values
("routing-rule", "governance", "loadbalancing").
In `@ui/app/workspace/logs/views/filters.tsx`:
- Around line 325-327: The label lookup can return undefined for unknown routing
engines causing blank text; update the JSX branch that uses
RoutingEngineUsedLabels[value as keyof typeof RoutingEngineUsedLabels] so it
falls back to the raw value (use a nullish/undefined fallback) — i.e., when
category === "Routing Engines" return RoutingEngineUsedLabels[...] ?? value;
keep the RequestTypeLabels branch as-is and ensure the ternary/conditional
rendering uses the fallback to avoid empty output.
In `@ui/lib/constants/logs.ts`:
- Around line 1-2: Remove the unused import "Route" from
next/dist/build/swc/types in ui/lib/constants/logs.ts; locate the import
statement that references Route and delete it so no unused symbols remain, then
run typecheck/linters to ensure no other references to Route exist and commit
the cleaned file.
In `@ui/lib/types/logs.ts`:
- Line 338: The comment for the routing_engine_used property is incomplete;
update the comment on routing_engine_used?: string[] in ui/lib/types/logs.ts to
include "loadbalancing" alongside "routing-rule" and "governance" so it matches
the backend enum BifrostContextKeyRoutingEngineUsed; locate the
routing_engine_used field and expand its inline comment to list "routing-rule,
governance, loadbalancing".
🧹 Nitpick comments (6)
ui/lib/types/logs.ts (1)
156-157: Unrelated change detected.The
output_formatfield addition toImageMessageDataappears unrelated to the routing engine tracking feature. Consider splitting this into a separate commit or PR for cleaner change tracking.framework/logstore/migrations.go (1)
992-1023: Add an index for routing_engine_used to keep filtering fast.Since routing_engine_used is now a first-class filter, adding an index (and dropping it on rollback) keeps log searches scalable as data grows.
♻️ Suggested migration enhancement
Migrate: func(tx *gorm.DB) error { tx = tx.WithContext(ctx) migrator := tx.Migrator() if !migrator.HasColumn(&Log{}, "routing_engine_used") { if err := migrator.AddColumn(&Log{}, "routing_engine_used"); err != nil { return err } } + if !migrator.HasIndex(&Log{}, "idx_logs_routing_engine_used") { + if err := migrator.CreateIndex(&Log{}, "idx_logs_routing_engine_used"); err != nil { + return err + } + } return nil }, Rollback: func(tx *gorm.DB) error { tx = tx.WithContext(ctx) migrator := tx.Migrator() + if migrator.HasIndex(&Log{}, "idx_logs_routing_engine_used") { + if err := migrator.DropIndex(&Log{}, "idx_logs_routing_engine_used"); err != nil { + return err + } + } if migrator.HasColumn(&Log{}, "routing_engine_used") { if err := migrator.DropColumn(&Log{}, "routing_engine_used"); err != nil { return err } } return nil },plugins/logging/operations.go (1)
16-49: Use bifrost.Ptr for new pointer assignments.The current code uses the
&operator to create pointers, which doesn't align with the bifrost convention. Switch to thePtr()function for consistency. Import the necessary package (eithercore/utilsor usecore/schemaswhich is already imported) and update both assignments.♻️ Proposed adjustment
if parentRequestID != "" { - entry.ParentRequestID = &parentRequestID + entry.ParentRequestID = schemas.Ptr(parentRequestID) } if routingEngineUsed != "" { - entry.RoutingEngineUsed = &routingEngineUsed + entry.RoutingEngineUsed = schemas.Ptr(routingEngineUsed) }The
Ptrfunction is available incore/schemas/utils.go(already imported by this file), so usingschemas.Ptr()directly is the most straightforward path. Alternatively, you can import fromcore/utilsif that aligns with your broader codebase patterns.ui/app/workspace/logs/views/logDetailsSheet.tsx (1)
308-319: Add a safe fallback for unknown routing engines.If a new routing engine value appears (e.g., from other PRs in the stack), this block can render an empty label/color. Consider defaulting to the raw value and a neutral badge style to keep the UI informative.
💡 Suggested fallback
- <Badge className={RoutingEngineUsedColors[log.routing_engine_used as keyof typeof RoutingEngineUsedColors]}> + <Badge + className={ + RoutingEngineUsedColors[log.routing_engine_used as keyof typeof RoutingEngineUsedColors] ?? + "bg-gray-100 text-gray-800" + } + > <div className="flex items-center gap-2"> {RoutingEngineUsedIcons[log.routing_engine_used as keyof typeof RoutingEngineUsedIcons]?.()} - <span>{RoutingEngineUsedLabels[log.routing_engine_used as keyof typeof RoutingEngineUsedLabels]}</span> + <span> + {RoutingEngineUsedLabels[log.routing_engine_used as keyof typeof RoutingEngineUsedLabels] ?? + log.routing_engine_used} + </span> </div> </Badge>plugins/logging/main.go (2)
797-800: Prefer shared context-key constants over inline strings.Using the existing schemas.BifrostContextKeyGovernanceVirtualKeyID/Name constants avoids drift and typos.
🔧 Suggested change
- virtualKeyID := bifrost.GetStringFromContext(ctx, schemas.BifrostContextKey("bf-governance-virtual-key-id")) - virtualKeyName := bifrost.GetStringFromContext(ctx, schemas.BifrostContextKey("bf-governance-virtual-key-name")) + virtualKeyID := bifrost.GetStringFromContext(ctx, schemas.BifrostContextKeyGovernanceVirtualKeyID) + virtualKeyName := bifrost.GetStringFromContext(ctx, schemas.BifrostContextKeyGovernanceVirtualKeyName) ... - virtualKeyID := bifrost.GetStringFromContext(ctx, schemas.BifrostContextKey("bf-governance-virtual-key-id")) - virtualKeyName := bifrost.GetStringFromContext(ctx, schemas.BifrostContextKey("bf-governance-virtual-key-name")) + virtualKeyID := bifrost.GetStringFromContext(ctx, schemas.BifrostContextKeyGovernanceVirtualKeyID) + virtualKeyName := bifrost.GetStringFromContext(ctx, schemas.BifrostContextKeyGovernanceVirtualKeyName)Also applies to: 873-875
365-367: Use bifrost.Ptr for the new routing-engine pointer.Replace the address-of operator with the repo-standard helper for pointer creation.
✅ Suggested change
- initialEntry.RoutingEngineUsed = &msg.RoutingEngineUsed + initialEntry.RoutingEngineUsed = bifrost.Ptr(msg.RoutingEngineUsed)
|
I'll review the implementation across all these areas to ensure consistency, correctness, and adherence to best practices. ✅ Actions performedFull review triggered. |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@plugins/governance/main.go`:
- Line 532: The governance handler currently overwrites the routing attribution
by unconditionally calling
ctx.SetValue(schemas.BifrostContextKeyRoutingEngineUsed, "governance"); change
it to only set that key when no routing engine has already been recorded (i.e.,
check ctx.GetValue / ctx.Value for schemas.BifrostContextKeyRoutingEngineUsed
and only call ctx.SetValue if it is nil/empty). Apply the same conditional guard
to the second occurrence of
ctx.SetValue(schemas.BifrostContextKeyRoutingEngineUsed, "governance") so
loadBalanceProvider (which runs after applyRoutingRules) cannot overwrite a
previously set "routing-rule" attribution.
In `@plugins/logging/main.go`:
- Around line 797-800: The context keys used to fetch MCP virtual key fields are
wrong: update the string constants passed to bifrost.GetStringFromContext (and
wrapped by schemas.BifrostContextKey) so they use the canonical "bifrost-"
prefix (e.g. change "bf-governance-virtual-key-id" and
"bf-governance-virtual-key-name" to "bifrost-governance-virtual-key-id" and
"bifrost-governance-virtual-key-name") wherever they are read (see virtualKeyID,
virtualKeyName and the other occurrence around the block for lines 872-875) so
the MCP virtual key data actually populates.
In `@ui/app/workspace/logs/views/logDetailsSheet.tsx`:
- Around line 308-317: The Routing Engine UI block uses RoutingEngineUsedColors,
RoutingEngineUsedIcons, and RoutingEngineUsedLabels directly which can break
when log.routing_engine_used contains an unknown value; update the rendering to
mirror the RequestType pattern by resolving a safe key (e.g., const key =
log.routing_engine_used as keyof typeof RoutingEngineUsedColors ?? 'unknown')
and then use fallbacks like RoutingEngineUsedColors[key] ?? 'default-class',
RoutingEngineUsedIcons[key]?.() ?? DefaultRoutingEngineIcon(), and
RoutingEngineUsedLabels[key] ?? 'Unknown' so the Badge className, icon render,
and label always have safe defaults.
🧹 Nitpick comments (3)
framework/logstore/migrations.go (1)
992-1023: Create/drop the routing_engine_used index to match the model tag.The Log model declares
idx_logs_routing_engine_used, but the migration only adds the column. Add index creation (and drop on rollback) so query performance matches the schema intent.♻️ Suggested migration enhancement
Migrate: func(tx *gorm.DB) error { tx = tx.WithContext(ctx) migrator := tx.Migrator() if !migrator.HasColumn(&Log{}, "routing_engine_used") { if err := migrator.AddColumn(&Log{}, "routing_engine_used"); err != nil { return err } } + if !migrator.HasIndex(&Log{}, "idx_logs_routing_engine_used") { + if err := migrator.CreateIndex(&Log{}, "idx_logs_routing_engine_used"); err != nil { + return fmt.Errorf("failed to create index on routing_engine_used: %w", err) + } + } return nil }, Rollback: func(tx *gorm.DB) error { tx = tx.WithContext(ctx) migrator := tx.Migrator() + if migrator.HasIndex(&Log{}, "idx_logs_routing_engine_used") { + if err := migrator.DropIndex(&Log{}, "idx_logs_routing_engine_used"); err != nil { + return err + } + } if migrator.HasColumn(&Log{}, "routing_engine_used") { if err := migrator.DropColumn(&Log{}, "routing_engine_used"); err != nil { return err } } return nil },plugins/logging/operations.go (1)
16-49: Preferbifrost.Ptr(...)for pointer creation.
Keeps pointer construction consistent with repo conventions.Based on learnings: “prefer using bifrost.Ptr() to create pointers instead of the address operator (&) … across all code paths.”♻️ Proposed fix
import ( "context" "fmt" "time" "github.com/bytedance/sonic" + bifrost "github.com/maximhq/bifrost/core" "github.com/maximhq/bifrost/core/schemas" "github.com/maximhq/bifrost/framework/logstore" "github.com/maximhq/bifrost/framework/streaming" ) @@ if routingEngineUsed != "" { - entry.RoutingEngineUsed = &routingEngineUsed + entry.RoutingEngineUsed = bifrost.Ptr(routingEngineUsed) }plugins/logging/main.go (1)
365-367: Preferbifrost.Ptr(...)for pointer creation here as well.Based on learnings: “prefer using bifrost.Ptr() to create pointers instead of the address operator (&) … across all code paths.”♻️ Proposed fix
- if msg.RoutingEngineUsed != "" { - initialEntry.RoutingEngineUsed = &msg.RoutingEngineUsed - } + if msg.RoutingEngineUsed != "" { + initialEntry.RoutingEngineUsed = bifrost.Ptr(msg.RoutingEngineUsed) + }
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@core/schemas/bifrost.go`:
- Line 187: The inline comment for the constant
BifrostContextKeyRoutingEngineUsed contains a typographical error with a double
closing parenthesis "DO NOT SET THIS MANUALLY))"; edit the comment attached to
BifrostContextKeyRoutingEngineUsed and remove the extra parenthesis so it reads
"...DO NOT SET THIS MANUALLY)" (or simply "...DO NOT SET THIS MANUALLY") to
correct the typo.
In `@plugins/logging/main.go`:
- Around line 798-799: Replace the hardcoded context key strings used when
reading from context in the logging plugin: instead of
bifrost.GetStringFromContext(ctx,
schemas.BifrostContextKey("bf-governance-virtual-key-id")) and
bifrost.GetStringFromContext(ctx,
schemas.BifrostContextKey("bf-governance-virtual-key-name")), use the declared
constants schemas.BifrostContextKeyGovernanceVirtualKeyID and
schemas.BifrostContextKeyGovernanceVirtualKeyName so virtualKeyID and
virtualKeyName correctly read the values the governance plugin sets; update the
two calls in the same block where virtualKeyID and virtualKeyName are assigned.
- Around line 873-874: Replace the incorrect context key strings used when
reading virtualKeyID and virtualKeyName from context: change the calls to
bifrost.GetStringFromContext that currently use
schemas.BifrostContextKey("bf-governance-virtual-key-id") and
schemas.BifrostContextKey("bf-governance-virtual-key-name") to use the correct
keys with the "bifrost-governance-virtual-key-id" and
"bifrost-governance-virtual-key-name" constants (i.e., update the
schemas.BifrostContextKey arguments for virtualKeyID and virtualKeyName in this
file so they match the same correct constants used elsewhere such as in
PreMCPHook).
In `@ui/app/workspace/logs/views/logDetailsSheet.tsx`:
- Around line 25-26: The badge rendering can produce empty/unstyled output when
a routing engine key is unknown; update usages of RoutingEngineUsedLabels and
RoutingEngineUsedColors in logDetailsSheet.tsx (the code that renders the
routing engine badge) to provide safe fallbacks: use the raw routingEngine value
(or a string like String(routingEngine) as a nullish fallback) for the label and
a default color (e.g., a DEFAULT_ROUTING_ENGINE_COLOR constant or a sensible
hex) for the color via nullish coalescing; ensure both the label lookup
(RoutingEngineUsedLabels[routingEngine] ?? String(routingEngine)) and the color
lookup (RoutingEngineUsedColors[routingEngine] ?? DEFAULT_ROUTING_ENGINE_COLOR)
are applied wherever the badge is created (also update the other occurrences
around the 308-316 area).
🧹 Nitpick comments (2)
plugins/logging/operations.go (1)
16-49: Usebifrost.Ptrfor pointer assignment.To keep pointer creation consistent with repo conventions, prefer
bifrost.Ptr(routingEngineUsed)over&routingEngineUsedhere (and add the import if not already present).♻️ Suggested tweak
- if routingEngineUsed != "" { - entry.RoutingEngineUsed = &routingEngineUsed - } + if routingEngineUsed != "" { + entry.RoutingEngineUsed = bifrost.Ptr(routingEngineUsed) + }Based on learnings: In the maximhq/bifrost repository, prefer using bifrost.Ptr() to create pointers instead of the address operator (&) even when & would be valid syntactically.
framework/logstore/migrations.go (1)
64-66: Consider adding an index forrouting_engine_used.Since this field is filterable, creating
idx_logs_routing_engine_usedduring migration keeps queries snappy on large tables.🛠️ Suggested index addition
Migrate: func(tx *gorm.DB) error { tx = tx.WithContext(ctx) migrator := tx.Migrator() if !migrator.HasColumn(&Log{}, "routing_engine_used") { if err := migrator.AddColumn(&Log{}, "routing_engine_used"); err != nil { return err } } + if !migrator.HasIndex(&Log{}, "idx_logs_routing_engine_used") { + if err := migrator.CreateIndex(&Log{}, "idx_logs_routing_engine_used"); err != nil { + return fmt.Errorf("failed to create index on routing_engine_used: %w", err) + } + } return nil }, Rollback: func(tx *gorm.DB) error { tx = tx.WithContext(ctx) migrator := tx.Migrator() + if migrator.HasIndex(&Log{}, "idx_logs_routing_engine_used") { + if err := migrator.DropIndex(&Log{}, "idx_logs_routing_engine_used"); err != nil { + return err + } + } if migrator.HasColumn(&Log{}, "routing_engine_used") { if err := migrator.DropColumn(&Log{}, "routing_engine_used"); err != nil { return err } } return nil },Also applies to: 992-1015
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@plugins/governance/main.go`:
- Line 532: The routing engine context value is incorrectly set to "governance"
in the load-balancing branch, which collides with governance decisions and
prevents the "loadbalancing" filter from running; update the ctx.SetValue call
that uses schemas.BifrostContextKeyRoutingEngineUsed to set the value to
"loadbalancing" instead of "governance" so this path is correctly labeled and
the weighted provider selection filter can operate.
🧹 Nitpick comments (3)
plugins/logging/operations.go (1)
22-49: Preferbifrost.Ptrfor pointer creation.Use the repo-standard helper instead of
&routingEngineUsedfor consistency.♻️ Suggested change
- entry.RoutingEngineUsed = &routingEngineUsed + entry.RoutingEngineUsed = bifrost.Ptr(routingEngineUsed)📌 Import update (outside this hunk)
+ bifrost "github.com/maximhq/bifrost/core"Based on learnings: “prefer using bifrost.Ptr() to create pointers instead of the address operator (&)… apply this consistently across all code paths.”
transports/bifrost-http/handlers/logging.go (1)
64-141: Consider extracting shared filter parsing logic.The filter extraction code in
getLogs,getLogsStats, andgetLogsHistogramis nearly identical. WhileparseHistogramFiltersexists and handles most filters, it's not used bygetLogsorgetLogsStats. Refactoring to share this logic would reduce duplication and ensure consistency when adding new filters.This is pre-existing technical debt, so it's optional to address in this PR.
Also applies to: 238-315, 327-404
plugins/logging/main.go (1)
365-367: Prefer bifrost.Ptr for pointer creation.
Use the repo-standard helper instead of the address operator.♻️ Proposed fix
- if msg.RoutingEngineUsed != "" { - initialEntry.RoutingEngineUsed = &msg.RoutingEngineUsed - } + if msg.RoutingEngineUsed != "" { + initialEntry.RoutingEngineUsed = bifrost.Ptr(msg.RoutingEngineUsed) + }Based on learnings: "In the maximhq/bifrost repository, prefer using bifrost.Ptr() to create pointers instead of the address operator (&) even when & would be valid syntactically. Apply this consistently across all code paths, including test utilities, to improve consistency and readability."
32412cb to
c81f91e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@plugins/logging/main.go`:
- Around line 365-367: The code currently takes the address of a pooled
LogMessage field (using &msg.RoutingEngineUsed) which can alias into a reused
object; instead, copy the string into a new pointer using bifrost.Ptr() and
assign that to initialEntry.RoutingEngineUsed (i.e., replace
&msg.RoutingEngineUsed with bifrost.Ptr(msg.RoutingEngineUsed)) so the entry
holds an independent value outside the pooled msg returned after the goroutine.
🧹 Nitpick comments (1)
plugins/logging/operations.go (1)
16-49: Use bifrost.Ptr for pointer creation per repo convention.
This keeps pointer handling consistent with other code paths.🛠️ Suggested fix
import ( "context" "fmt" "time" "github.com/bytedance/sonic" + bifrost "github.com/maximhq/bifrost/core" "github.com/maximhq/bifrost/core/schemas" "github.com/maximhq/bifrost/framework/logstore" "github.com/maximhq/bifrost/framework/streaming" ) @@ if routingEngineUsed != "" { - entry.RoutingEngineUsed = &routingEngineUsed + entry.RoutingEngineUsed = bifrost.Ptr(routingEngineUsed) }Based on learnings "In the maximhq/bifrost repository, prefer using bifrost.Ptr() to create pointers instead of the address operator (&) even when & would be valid syntactically."
Merge activity
|
1913341 to
6328738
Compare
c81f91e to
0740c11
Compare
0740c11 to
3f9ea85
Compare

Summary
Add tracking for routing engine used in requests to improve observability and filtering capabilities in logs and telemetry.
Changes
BifrostContextKeyRoutingEngineUsedto track which routing engine handled a requestType of change
Affected areas
How to test
Screenshots/Recordings
N/A
Breaking changes
Related issues
N/A
Security considerations
No security implications as this is purely for observability purposes.
Checklist