structured logging - #5497
structured logging#5497kohlivrinda wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (19)
🚧 Files skipped from review as they are similar to previous changes (13)
📝 WalkthroughSummary by CodeRabbit
WalkthroughGovernance complexity classification records tier, mechanism, and score in request context. The metadata flows into traces, metrics, persisted logs, and searchable APIs. Log APIs support complexity filters and response fields. Migrations, parity tests, OpenAPI schemas, and observability documentation cover the metadata. ChangesGovernance complexity observability
Estimated code review effort: 4 (Complex) | ~60 minutes Mergeability Score: 🔵 Low · up to The PR adds filterable complexity metadata, but the API schema still permits arbitrary non-null mechanism strings instead of enforcing the documented values. This is a bounded contract risk requiring owner awareness or a small follow-up; the change is otherwise mergeable. Sequence Diagram(s)sequenceDiagram
participant GovernancePlugin
participant BifrostContext
participant PostLLMHook
participant Logstore
participant HTTPHandler
GovernancePlugin->>BifrostContext: record complexity tier, mechanism, and score
PostLLMHook->>BifrostContext: read complexity metadata
PostLLMHook->>Logstore: persist enriched log entry
HTTPHandler->>Logstore: query with complexity filters
Logstore-->>HTTPHandler: return logs with complexity fields
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
framework/logstore/matviews.go (1)
788-789: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd regression coverage for the new matview exclusions.
Line 788-789 correctly routes non-empty complexity and routing-mechanism filters to raw logs. Add deterministic cases for each filter, plus empty slices, so future changes cannot send unsupported filters to
mv_logs_hourlyand return incomplete results.As per path instructions, framework changes should include tests that cover edge cases and failure paths.
🤖 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 `@framework/logstore/matviews.go` around lines 788 - 789, Add regression tests around the matview-routing logic containing ComplexityTiers and RoutingMechanisms, covering non-empty and empty slices for each filter. Assert that populated unsupported filters route to raw logs rather than mv_logs_hourly, while empty slices preserve the existing matview path; keep the cases deterministic and include the relevant failure-path assertions.Source: Path instructions
🤖 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 `@framework/logstore/logstoreparity_test.go`:
- Line 414: Update the parity projection in the logstore parity test to include
l.ComplexityScore alongside ComplexityTier and RoutingMechanism, ensuring lookup
comparisons verify the persisted complexity score.
In `@plugins/logging/main.go`:
- Around line 929-931: Update the fallback error-entry path used when
pendingLogsEntries is missing so it also persists complexityTier,
routingMechanism, and complexityScore metadata before writing and returning.
Reuse the values retrieved near the complexity metadata setup, including the
hasComplexityScore state, and preserve the existing metadata behavior for normal
entries.
---
Nitpick comments:
In `@framework/logstore/matviews.go`:
- Around line 788-789: Add regression tests around the matview-routing logic
containing ComplexityTiers and RoutingMechanisms, covering non-empty and empty
slices for each filter. Assert that populated unsupported filters route to raw
logs rather than mv_logs_hourly, while empty slices preserve the existing
matview path; keep the cases deterministic and include the relevant failure-path
assertions.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1b9787ea-62e2-4cbb-91ac-bb0959066ae7
📒 Files selected for processing (14)
core/schemas/bifrost.godocs/openapi/openapi.jsonframework/logstore/logstoreparity_test.goframework/logstore/matviews.goframework/logstore/migrations.goframework/logstore/rdb.goframework/logstore/tables.goplugins/governance/complexity/config.goplugins/governance/main.goplugins/governance/prerequesthookcomplexity_test.goplugins/governance/routing.goplugins/logging/main.goplugins/logging/operations_test.gotransports/bifrost-http/handlers/logging.go
1b8e8ee to
4edc8b3
Compare
b4a90f3 to
a7be829
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@docs/openapi/openapi.json`:
- Around line 82627-82645: Update the OpenAPI 3.1 schema definitions for
complexity_tier, routing_mechanism, and complexity_score to remove nullable and
represent null using JSON Schema null branches. Keep complexity_tier’s existing
enum values while adding null to its allowed values, and use anyOf with a null
type for the other nullable fields.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4f4a3b8f-7a0f-4c6e-884f-0737c549fd32
📒 Files selected for processing (14)
core/schemas/bifrost.godocs/openapi/openapi.jsonframework/logstore/logstoreparity_test.goframework/logstore/matviews.goframework/logstore/migrations.goframework/logstore/rdb.goframework/logstore/tables.goplugins/governance/complexity/config.goplugins/governance/main.goplugins/governance/prerequesthookcomplexity_test.goplugins/governance/routing.goplugins/logging/main.goplugins/logging/operations_test.gotransports/bifrost-http/handlers/logging.go
🚧 Files skipped from review as they are similar to previous changes (12)
- plugins/governance/complexity/config.go
- framework/logstore/matviews.go
- framework/logstore/rdb.go
- plugins/governance/prerequesthookcomplexity_test.go
- transports/bifrost-http/handlers/logging.go
- plugins/logging/main.go
- plugins/logging/operations_test.go
- framework/logstore/tables.go
- plugins/governance/main.go
- plugins/governance/routing.go
- framework/logstore/migrations.go
- framework/logstore/logstoreparity_test.go
4edc8b3 to
d700aae
Compare
a7be829 to
ac858a4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@framework/logstore/migrations.go`:
- Line 283: Update the migration registry around
migrationAddComplexityRoutingColumns so idx_logs_complexity_tier and
idx_logs_complexity_mechanism are removed from logs_add_performance_indexes and
created by a new dedicated concurrent-index migration registered after the
column migration. Ensure fresh installs create the indexes only after the
columns exist, while existing deployments run the new migration.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a5d75543-8f1e-40f1-a973-24addb0f0ea6
📒 Files selected for processing (18)
core/schemas/bifrost.gocore/schemas/enrichment.gocore/schemas/trace.godocs/openapi/openapi.jsonframework/logstore/logstoreparity_test.goframework/logstore/matviews.goframework/logstore/migrations.goframework/logstore/rdb.goframework/logstore/tables.goframework/tracing/tracer.goplugins/governance/complexity/config.goplugins/governance/main.goplugins/governance/prerequesthookcomplexity_test.goplugins/governance/routing.goplugins/logging/main.goplugins/logging/operations_test.goplugins/telemetry/main.gotransports/bifrost-http/handlers/logging.go
🚧 Files skipped from review as they are similar to previous changes (9)
- framework/logstore/matviews.go
- plugins/governance/routing.go
- framework/logstore/tables.go
- framework/logstore/rdb.go
- plugins/logging/main.go
- plugins/governance/complexity/config.go
- plugins/governance/prerequesthookcomplexity_test.go
- plugins/governance/main.go
- framework/logstore/logstoreparity_test.go
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@core/schemas/enrichment.go`:
- Around line 69-74: Update the Datadog connector documentation, especially the
Bifrost Context/tag section and related query guidance, to describe the new
complexity_tier and complexity_mechanism connector dimensions, including their
supported values and attribution usage. Do not document the raw complexity score
as a dimension.
In `@plugins/telemetry/main.go`:
- Line 275: The telemetry migration must remain additive: in
plugins/telemetry/main.go lines 275, 921, and 954-955, retain and populate the
legacy routing_mechanism label alongside complexity_mechanism; in
core/schemas/trace.go lines 799-799, preserve the deprecated routing-mechanism
attribute constant; in core/schemas/enrichment.go lines 75-76, retain the legacy
enrichment dimension; and in framework/tracing/tracer.go lines 358-359, emit the
legacy trace attribute alongside bifrost.complexity_mechanism.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a0b59628-4586-499c-8cff-a474369d12c2
📒 Files selected for processing (18)
core/schemas/bifrost.gocore/schemas/enrichment.gocore/schemas/trace.godocs/openapi/openapi.jsonframework/logstore/logstoreparity_test.goframework/logstore/matviews.goframework/logstore/migrations.goframework/logstore/rdb.goframework/logstore/tables.goframework/tracing/tracer.goplugins/governance/complexity/config.goplugins/governance/main.goplugins/governance/prerequesthookcomplexity_test.goplugins/governance/routing.goplugins/logging/main.goplugins/logging/operations_test.goplugins/telemetry/main.gotransports/bifrost-http/handlers/logging.go
ac858a4 to
d72ba2c
Compare
d700aae to
00f6b8d
Compare
987e97c to
b9bdc35
Compare
722c30f to
c18a0dd
Compare
b9bdc35 to
09e21c8
Compare
09e21c8 to
f55678d
Compare
c18a0dd to
81bd30f
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@framework/logstore/migrations.go`:
- Around line 2708-2719: Remove idx_logs_complexity_tier and
idx_logs_complexity_mechanism from performanceIndexes, then add both partial
concurrent index definitions in a new migration step positioned after
logs_add_complexity_routing_columns. Ensure fresh installations create the
columns before these indexes, while existing installations execute the new
migration and receive both indexes.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9b03aacf-799e-4b61-93b8-db86e293d5b7
📒 Files selected for processing (19)
core/schemas/bifrost.gocore/schemas/enrichment.gocore/schemas/trace.godocs/features/observability/datadog.mdxdocs/openapi/openapi.jsonframework/logstore/logstoreparity_test.goframework/logstore/matviews.goframework/logstore/migrations.goframework/logstore/rdb.goframework/logstore/tables.goframework/tracing/tracer.goplugins/governance/complexity/config.goplugins/governance/main.goplugins/governance/prerequesthookcomplexity_test.goplugins/governance/routing.goplugins/logging/main.goplugins/logging/operations_test.goplugins/telemetry/main.gotransports/bifrost-http/handlers/logging.go
🚧 Files skipped from review as they are similar to previous changes (15)
- plugins/governance/routing.go
- plugins/governance/prerequesthookcomplexity_test.go
- framework/logstore/matviews.go
- plugins/governance/complexity/config.go
- docs/features/observability/datadog.mdx
- framework/logstore/rdb.go
- framework/logstore/tables.go
- plugins/logging/operations_test.go
- framework/tracing/tracer.go
- core/schemas/trace.go
- plugins/governance/main.go
- framework/logstore/logstoreparity_test.go
- core/schemas/enrichment.go
- plugins/logging/main.go
- transports/bifrost-http/handlers/logging.go
f55678d to
9983c0b
Compare
81bd30f to
83fed9d
Compare
9983c0b to
fb7445b
Compare
83fed9d to
d0a7d6e
Compare
d0a7d6e to
e7b2905
Compare
fb7445b to
299e88f
Compare
e7b2905 to
976a2fc
Compare
299e88f to
5639cd8
Compare
976a2fc to
5f21723
Compare
17f69b6 to
ec603a5
Compare
5f21723 to
47b458e
Compare
ec603a5 to
61b5b6a
Compare
47b458e to
6f1e6a7
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |

Summary
Adds first-class support for routing complexity classification data in request logs. When a governance routing rule references
complexity_tier, the classifier now records the computed tier (SIMPLE,MEDIUM,COMPLEX), the raw score, and the mechanism used (lexicalorskipped) into theBifrostContext. The logging plugin persists these values to three new database columns, and the HTTP API exposes them as filterable query parameters.Changes
BifrostContextKeyGovernanceRoutingMechanismtoBifrostContextKeyGovernanceComplexityMechanismandAttrBifrostRoutingMechanismtoAttrBifrostComplexityMechanismfor consistency; addedBifrostContextKeyGovernanceComplexityTierandBifrostContextKeyGovernanceComplexityScorecontext keys written by the governance plugin during routing rule evaluation.MechanismLexicalandMechanismSkippedconstants in the complexity package to standardise the mechanism values recorded in logs.logs_add_complexity_routing_columns) that addscomplexity_tier,complexity_mechanism, andcomplexity_scorecolumns to thelogstable, with partial indexes on the first two (skippingNULLrows since most requests will not use complexity routing).PostLLMHookand writes them to the log entry.SearchFiltersgainedComplexityTiersandComplexityMechanismsfields;applyFiltersin the RDB store appliesINclauses for both; the materialized-view fast-path is bypassed when either filter is active.listSelectColumnsnow includescomplexity_tierandcomplexity_mechanismso list responses carry these fields.getLogs,getLogsStats, andparseHistogramFiltersparsecomplexity_tiersandcomplexity_mechanismsquery parameters.complexity_tiersandcomplexity_mechanismsquery parameters on all three log endpoints, andcomplexity_tier,complexity_mechanism, andcomplexity_scoreresponse fields on the log object.complexity_tierandcomplexity_mechanismtags, including example metric queries and a note that the raw score is not exported as a tag due to unbounded cardinality.routing_mechanismrenamed tocomplexity_mechanismto match the updated attribute name.complexity_tiers,complexity_mechanisms, andmechanism_skippedsearch cases.skippedmechanism recorded when an unsupported input type is encountered. A logging plugin test verifies all three fields are persisted to the store.Type of change
Affected areas
How to test
go test ./framework/logstore/... ./plugins/governance/... ./plugins/logging/...complexity_tier.GET /logs?complexity_tiers=COMPLEXand confirmcomplexity_tier,complexity_mechanism, andcomplexity_scoreare populated.complexity_mechanism: skippedwith null tier and score.complexity_mechanisms=lexicalandcomplexity_mechanisms=skippedto verify both filter paths return the expected subsets.Breaking changes
The new columns are nullable and default to
NULLfor all existing rows. The migration is additive only. The rename fromrouting_mechanismtocomplexity_mechanismaffects context keys, span attributes, Prometheus label names, and enrichment dimensions — consumers of those internal identifiers will need to update their references.Security considerations
The complexity score and tier are derived from request content already visible to admins in the log detail view. No new PII is introduced. The raw score is stored as an unindexed float and is only returned in detail views.
Checklist
docs/contributing/README.mdand followed the guidelines