feat: adds support for multiple team and bu units in logstore - #4041
Conversation
📝 WalkthroughWalkthroughThis PR enables logs to be stored and queried by multiple teams and business units in addition to existing scalar values. It adds JSON-array columns to the Log schema, implements PostgreSQL fan-out SQL logic to unnest those arrays during filtering and aggregation, updates materialized views for team/business-unit dropdowns, introduces database migrations and GIN indexes, and enhances the UI to support attributed dimension rankings and multi-value team/business-unit rendering in log details. ChangesMulti-team/multi-business-unit filtering feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
93659ab to
bea42b4
Compare
dbaa616 to
d171170
Compare
bea42b4 to
55544a3
Compare
d171170 to
dbaa616
Compare
55544a3 to
bea42b4
Compare
bea42b4 to
801690f
Compare
dbaa616 to
d171170
Compare
801690f to
de257d5
Compare
d171170 to
9529196
Compare
Confidence Score: 5/5Safe to merge; all changes are additive and backward-compatible with the existing scalar columns. The new columns are nullable, the fan-out query uses UNION ALL with mutually-exclusive branch guards so no row is double-counted, DAC scope is preserved through l.*, and the migration correctly forces a matview DROP so ensureMatViews recreates with the new body. Test coverage is comprehensive — SQL structure tests, integration tests against a live DB, and explicit DAC scope leak tests. The only nit is a silently ignored sonic.Marshal error that cannot fail in practice for a []string argument. No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "feat: adds support for multiple team and..." | Re-trigger Greptile |
|
@coderabbitai full-review |
|
✅ Action performedFull review finished. |
9529196 to
b7d6813
Compare
de257d5 to
75288aa
Compare
Merge activity
|
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 3475-3508: The migration currently drops mv_filter_teams and
mv_filter_business_units and relies on ensureMatViews() (async startup) to
recreate them, causing a gap where readers see "relation does not exist";
instead modify migrationRecreateFilterTeamBUMatViews so it atomically replaces
the views inside the same migration transaction: after dropping each view (or
better: create a new temp materialized view with the multi-value SELECT body,
then DROP the old view and RENAME the temp to the canonical name) run the CREATE
MATERIALIZED VIEW ... AS <multi-value SELECT> (or the create-temp + rename
sequence) using tx.Exec within the Migrate function so the new definitions exist
before any name is removed; remove the reliance on ensureMatViews for this shape
change so rolling deploy readers never see the gap.
In `@framework/logstore/rdb.go`:
- Around line 95-105: The CROSS JOIN LATERAL in teamOrBUFanoutFrom currently
casts TEXT to jsonb and calls jsonb_array_elements_text before the IS JSON ARRAY
guard is evaluated, which can error on malformed/non-array TEXT; update
teamOrBUFanoutFrom to pre-check the column is a JSON array before calling
jsonb_array_elements_text (e.g., use jsonb_typeof(<col>::jsonb) = 'array' or
wrap the lateral source in a CASE/SELECT that only calls
jsonb_array_elements_text when the jsonb_typeof check passes), ensuring the
scalar fallback remains reachable; keep the multiValueDimensionFilterSQL /
fanoutFrom behavior unchanged for non-Postgres dialector checks.
In `@framework/logstore/tables.go`:
- Around line 588-619: The serialized JSON strings produced from sonic.Marshal
for l.TeamIDsParsed, l.TeamNamesParsed, l.BusinessUnitIDsParsed and
l.BusinessUnitNamesParsed must be passed through sanitizeJSONForJSONB before
being assigned to the pointer fields (l.TeamIDs, l.TeamNames, l.BusinessUnitIDs,
l.BusinessUnitNames); update the blocks that currently set s := string(data);
l.TeamIDs = &s (and the other three) to instead sanitize the string (e.g. s :=
sanitizeJSONForJSONB(string(data))) and assign that sanitized value so stored
TEXT will be safe to cast to jsonb.
In `@ui/app/workspace/logs/sheets/logDetailView.tsx`:
- Around line 971-995: The business unit links render duplicate data-testid
values and include the comma inside the clickable Link; update the Link
rendering inside the map (where LogEntryDetailsView renders business units from
log.business_unit_ids/log.business_unit_id) to give each Link a unique
data-testid (e.g., append the business unit id or the map index to
"logdetails-business-unit-link") and move the comma separator outside the Link
(render the comma as a sibling element after the Link, not inside it) so
separators are not part of the clickable area.
- Around line 930-954: The team links inside LogEntryDetailsView reuse the same
data-testid and include the separator comma inside the interactive Link, which
breaks E2E selectors and expands the clickable area; update the Link rendering
in the mapped array (the <Link> elements) to assign a unique test id (e.g.,
derive from t.id or the map index such as
data-testid={`logdetails-team-link-${t.id}` or `- ${i}`) and move the trailing
comma out of the Link so the separator is rendered after the Link (e.g., render
Link for t.name only, then conditionally render "," after the Link when i <
arr.length - 1). Ensure you update the code locations referenced by
LogEntryDetailsView and the Link mapping to apply both changes.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 4bed945f-8360-42b7-8892-2e6a2119a530
📒 Files selected for processing (14)
core/schemas/bifrost.goframework/logstore/matviews.goframework/logstore/migrations.goframework/logstore/multi_team_filter_test.goframework/logstore/multi_team_matview_test.goframework/logstore/postgres.goframework/logstore/rdb.goframework/logstore/rdb_postgres_perf_test.goframework/logstore/tables.goplugins/logging/main.goui/app/workspace/dashboard/components/dimensionRankingsTab.tsxui/app/workspace/dashboard/components/tabViews/dimensionRankingsTabView.tsxui/app/workspace/logs/sheets/logDetailView.tsxui/lib/types/logs.ts
| // migrationRecreateFilterTeamBUMatViews drops mv_filter_teams and | ||
| // mv_filter_business_units so ensureMatViews recreates them with the multi-value | ||
| // body (scalar column UNION the JSON-array column). Required because | ||
| // repairMatViewShapes only detects drift by column presence, and the column | ||
| // shape (id, name, user_id, team_id, virtual_key_id) is unchanged — only the | ||
| // SELECT body changed — so the views would otherwise keep their old scalar-only | ||
| // definition. Recreated views keep identical columns, so old replicas reading | ||
| // them during a rolling deploy are unaffected (no legacyMatViewNames dance). | ||
| func migrationRecreateFilterTeamBUMatViews(ctx context.Context, db *gorm.DB) error { | ||
| if db.Dialector.Name() != "postgres" { | ||
| return nil | ||
| } | ||
| opts := *migrator.DefaultOptions | ||
| opts.UseTransaction = true | ||
| m := migrator.New(db, &opts, []*migrator.Migration{{ | ||
| ID: "logs_recreate_filter_team_bu_matviews_multivalue", | ||
| Migrate: func(tx *gorm.DB) error { | ||
| tx = tx.WithContext(ctx) | ||
| for _, view := range []string{"mv_filter_teams", "mv_filter_business_units"} { | ||
| if err := tx.Exec("DROP MATERIALIZED VIEW IF EXISTS " + view + " CASCADE").Error; err != nil { | ||
| return fmt.Errorf("failed to drop %s: %w", view, err) | ||
| } | ||
| } | ||
| return nil | ||
| }, | ||
| Rollback: func(tx *gorm.DB) error { | ||
| return nil | ||
| }, | ||
| }}) | ||
| if err := m.Migrate(); err != nil { | ||
| return fmt.Errorf("error while recreating filter team/business-unit matviews: %s", err.Error()) | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Don't drop shared filter matviews before their replacements exist.
This migration removes mv_filter_teams and mv_filter_business_units, but the new definitions are only recreated later by ensureMatViews() in the async startup path. During a rolling deploy, older replicas still querying those names will hit relation does not exist in that gap. Rebuild these views atomically in the same migration/transaction, or defer this shape change until all readers have been updated.
🧰 Tools
🪛 OpenGrep (1.22.0)
[ERROR] 3494-3494: SQL query built via fmt.Sprintf or string concatenation passed to a database method. Use parameterized queries with placeholder arguments.
(coderabbit.sql-injection.go-query-format)
🤖 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/migrations.go` around lines 3475 - 3508, The migration
currently drops mv_filter_teams and mv_filter_business_units and relies on
ensureMatViews() (async startup) to recreate them, causing a gap where readers
see "relation does not exist"; instead modify
migrationRecreateFilterTeamBUMatViews so it atomically replaces the views inside
the same migration transaction: after dropping each view (or better: create a
new temp materialized view with the multi-value SELECT body, then DROP the old
view and RENAME the temp to the canonical name) run the CREATE MATERIALIZED VIEW
... AS <multi-value SELECT> (or the create-temp + rename sequence) using tx.Exec
within the Migrate function so the new definitions exist before any name is
removed; remove the reliance on ensureMatViews for this shape change so rolling
deploy readers never see the gap.
| func multiValueDimensionFilterSQL(scalarCol, arrayCol string, ids []string) (string, []interface{}) { | ||
| arrConds := make([]string, len(ids)) | ||
| args := []interface{}{ids} | ||
| for i, id := range ids { | ||
| arrConds[i] = arrayCol + "::jsonb @> ?::jsonb" | ||
| frag, _ := sonic.Marshal([]string{id}) | ||
| args = append(args, string(frag)) | ||
| } | ||
| sql := fmt.Sprintf("(%s IN ? OR (%s IS NOT NULL AND %s IS JSON ARRAY AND (%s)))", | ||
| scalarCol, arrayCol, arrayCol, strings.Join(arrConds, " OR ")) | ||
| return sql, args |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "func teamOrBUFanoutFrom|multiValueDimensionFilterSQL|applyFilters|GetDimensionRankings|GetDimensionCostHistogram|GetDimensionTokenHistogram" framework/logstore/rdb.go
# Print the teamOrBUFanoutFrom function body
python3 - <<'PY'
import re, itertools, sys, pathlib
p = pathlib.Path("framework/logstore/rdb.go")
text = p.read_text()
m = re.search(r"func\s+teamOrBUFanoutFrom\s*\([^)]*\)\s*string\s*{", text)
if not m:
print("teamOrBUFanoutFrom not found")
sys.exit(0)
start = m.start()
# naive brace matching to extract function
i = m.end()
depth = 1
while i < len(text) and depth>0:
if text[i] == '{':
depth += 1
elif text[i] == '}':
depth -= 1
i += 1
print(text[m.start():i])
PY
# Print surrounding area for applyFilters and JSON array usage
python3 - <<'PY'
import pathlib, re
p = pathlib.Path("framework/logstore/rdb.go")
lines = p.read_text().splitlines()
def show_around(pattern, ctx=25):
for idx,l in enumerate(lines):
if re.search(pattern,l):
a=max(0,idx-ctx); b=min(len(lines),idx+ctx+1)
print("\n--- around:", pattern, "line", idx+1, "---")
for j in range(a,b):
print(f"{j+1:6d}: {lines[j]}")
return
show_around(r"func\s+applyFilters")
show_around(r"func\s+GetDimensionRankings")
show_around(r"func\s+GetDimensionCostHistogram")
show_around(r"func\s+GetDimensionTokenHistogram")
show_around(r"func\s+multiValueDimensionFilterSQL")
PYRepository: maximhq/bifrost
Length of output: 5772
🏁 Script executed:
#!/bin/bash
set -euo pipefail
f="framework/logstore/rdb.go"
# Show teamOrBUFanoutFrom body (lines 119-ish); print a safe window
sed -n '105,210p' "$f"
# Show applyFilters section where multiValueDimensionFilterSQL is used
sed -n '140,230p' "$f"
# Show GetDimensionRankings, CostHistogram, TokenHistogram sections where fanout / postgres-only logic likely lives
sed -n '2040,2220p' "$f"
sed -n '2720,2925p' "$f"Repository: maximhq/bifrost
Length of output: 21401
Guard JSONB casts in teamOrBUFanoutFrom to prevent malformed/non-array TEXT from erroring in the FROM clause.
- In
framework/logstore/rdb.go,teamOrBUFanoutFromperformsjsonb_array_elements_text(l.%[1]s::jsonb)/jsonb_array_elements_text(l.%[2]s::jsonb)inside theCROSS JOIN LATERALFROM clause, while theIS JSON ARRAYguard appears later in the subquery; invalid/non-array values can still fail the cast and abort the query instead of reaching the scalar fallback branch. - Fanout behavior is effectively Postgres-only (
fanoutFrom+multiValueDimensionFilterSQLare used only whens.db.Dialector.Name() == "postgres"), so SQLite keeps scalarteam_id/business_unit_idsemantics.
🛠️ Suggested hardening
- FROM jsonb_array_elements_text(l.%[1]s::jsonb) WITH ORDINALITY AS t(value, ord)
- LEFT JOIN jsonb_array_elements_text(l.%[2]s::jsonb) WITH ORDINALITY AS n(value, ord) ON n.ord = t.ord
- WHERE l.%[1]s IS NOT NULL AND l.%[1]s IS JSON ARRAY
+ FROM jsonb_array_elements_text(
+ CASE WHEN l.%[1]s IS NOT NULL AND l.%[1]s IS JSON ARRAY THEN l.%[1]s::jsonb ELSE '[]'::jsonb END
+ ) WITH ORDINALITY AS t(value, ord)
+ LEFT JOIN jsonb_array_elements_text(
+ CASE WHEN l.%[2]s IS NOT NULL AND l.%[2]s IS JSON ARRAY THEN l.%[2]s::jsonb ELSE '[]'::jsonb END
+ ) WITH ORDINALITY AS n(value, ord) ON n.ord = t.ord🤖 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/rdb.go` around lines 95 - 105, The CROSS JOIN LATERAL in
teamOrBUFanoutFrom currently casts TEXT to jsonb and calls
jsonb_array_elements_text before the IS JSON ARRAY guard is evaluated, which can
error on malformed/non-array TEXT; update teamOrBUFanoutFrom to pre-check the
column is a JSON array before calling jsonb_array_elements_text (e.g., use
jsonb_typeof(<col>::jsonb) = 'array' or wrap the lateral source in a CASE/SELECT
that only calls jsonb_array_elements_text when the jsonb_typeof check passes),
ensuring the scalar fallback remains reachable; keep the
multiValueDimensionFilterSQL / fanoutFrom behavior unchanged for non-Postgres
dialector checks.
| if s.db.Dialector.Name() == "postgres" { | ||
| sql, args := multiValueDimensionFilterSQL("team_id", "team_ids", filters.TeamIDs) | ||
| baseQuery = baseQuery.Where(sql, args...) | ||
| } else { | ||
| baseQuery = baseQuery.Where("team_id IN ?", filters.TeamIDs) | ||
| } |
There was a problem hiding this comment.
SQLite still misses secondary teams and business units.
These branches only honor team_ids / business_unit_ids on PostgreSQL. On SQLite, filtering and dimension rankings/histograms still read only team_id / business_unit_id, so rows attributed to a secondary team/BU stay invisible there. Either add a json_each-based SQLite path or gate the feature above the store layer.
Based on learnings: only PostgreSQL and SQLite database dialects are supported in framework/logstore.
Also applies to: 194-199, 2077-2088, 2758-2772, 2867-2881
| if len(l.TeamIDsParsed) > 0 { | ||
| if data, err := sonic.Marshal(l.TeamIDsParsed); err != nil { | ||
| return err | ||
| } else { | ||
| s := string(data) | ||
| l.TeamIDs = &s | ||
| } | ||
| } | ||
| if len(l.TeamNamesParsed) > 0 { | ||
| if data, err := sonic.Marshal(l.TeamNamesParsed); err != nil { | ||
| return err | ||
| } else { | ||
| s := string(data) | ||
| l.TeamNames = &s | ||
| } | ||
| } | ||
| if len(l.BusinessUnitIDsParsed) > 0 { | ||
| if data, err := sonic.Marshal(l.BusinessUnitIDsParsed); err != nil { | ||
| return err | ||
| } else { | ||
| s := string(data) | ||
| l.BusinessUnitIDs = &s | ||
| } | ||
| } | ||
| if len(l.BusinessUnitNamesParsed) > 0 { | ||
| if data, err := sonic.Marshal(l.BusinessUnitNamesParsed); err != nil { | ||
| return err | ||
| } else { | ||
| s := string(data) | ||
| l.BusinessUnitNames = &s | ||
| } | ||
| } |
There was a problem hiding this comment.
Sanitize serialized team/business-unit arrays before storing JSON text.
At Line 588 and Line 596, the new JSON strings are persisted without sanitizeJSONForJSONB(...). These columns are later cast from TEXT to jsonb; if a value contains \u0000, those queries can fail at runtime.
Suggested fix
if len(l.TeamIDsParsed) > 0 {
if data, err := sonic.Marshal(l.TeamIDsParsed); err != nil {
return err
} else {
- s := string(data)
+ s := sanitizeJSONForJSONB(string(data))
l.TeamIDs = &s
}
}
if len(l.TeamNamesParsed) > 0 {
if data, err := sonic.Marshal(l.TeamNamesParsed); err != nil {
return err
} else {
- s := string(data)
+ s := sanitizeJSONForJSONB(string(data))
l.TeamNames = &s
}
}
if len(l.BusinessUnitIDsParsed) > 0 {
if data, err := sonic.Marshal(l.BusinessUnitIDsParsed); err != nil {
return err
} else {
- s := string(data)
+ s := sanitizeJSONForJSONB(string(data))
l.BusinessUnitIDs = &s
}
}
if len(l.BusinessUnitNamesParsed) > 0 {
if data, err := sonic.Marshal(l.BusinessUnitNamesParsed); err != nil {
return err
} else {
- s := string(data)
+ s := sanitizeJSONForJSONB(string(data))
l.BusinessUnitNames = &s
}
}🤖 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/tables.go` around lines 588 - 619, The serialized JSON
strings produced from sonic.Marshal for l.TeamIDsParsed, l.TeamNamesParsed,
l.BusinessUnitIDsParsed and l.BusinessUnitNamesParsed must be passed through
sanitizeJSONForJSONB before being assigned to the pointer fields (l.TeamIDs,
l.TeamNames, l.BusinessUnitIDs, l.BusinessUnitNames); update the blocks that
currently set s := string(data); l.TeamIDs = &s (and the other three) to instead
sanitize the string (e.g. s := sanitizeJSONForJSONB(string(data))) and assign
that sanitized value so stored TEXT will be safe to cast to jsonb.
| {(log.team_ids?.length || log.team_id) && ( | ||
| <LogEntryDetailsView | ||
| className="w-full" | ||
| label="Team" | ||
| label={(log.team_ids?.length ?? 0) > 1 ? "Teams" : "Team"} | ||
| value={ | ||
| <Link | ||
| to="/workspace/logs" | ||
| search={{ team_ids: [log.team_id] }} | ||
| className="text-blue-600 hover:underline dark:text-blue-400" | ||
| data-testid="logdetails-team-link" | ||
| > | ||
| {log.team_name || log.team_id} | ||
| </Link> | ||
| <span className="inline-flex flex-wrap gap-x-1"> | ||
| {(log.team_ids?.length | ||
| ? log.team_ids.map((id, i) => ({ id, name: log.team_names?.[i] || id })) | ||
| : [{ id: log.team_id!, name: log.team_name || log.team_id! }] | ||
| ).map((t, i, arr) => ( | ||
| <Link | ||
| key={t.id} | ||
| to="/workspace/logs" | ||
| search={{ team_ids: [t.id] }} | ||
| className="text-blue-600 hover:underline dark:text-blue-400" | ||
| data-testid="logdetails-team-link" | ||
| > | ||
| {t.name} | ||
| {i < arr.length - 1 ? "," : ""} | ||
| </Link> | ||
| ))} | ||
| </span> | ||
| } | ||
| /> | ||
| )} |
There was a problem hiding this comment.
Ensure unique data-testid values for each team link.
The data-testid="logdetails-team-link" is repeated for every link in the map, which violates E2E testing requirements. When multiple elements share the same testid, selectors like getByTestId("logdetails-team-link") will either throw or return an ambiguous result, causing test failures.
Additionally, placing the comma inside the Link element makes it part of the clickable area. Typically, separators like commas are placed outside interactive elements to reduce unintended click targets.
Proposed fix: unique testids and comma placement
<span className="inline-flex flex-wrap gap-x-1">
{(log.team_ids?.length
? log.team_ids.map((id, i) => ({ id, name: log.team_names?.[i] || id }))
: [{ id: log.team_id!, name: log.team_name || log.team_id! }]
- ).map((t, i, arr) => (
+ ).map((t, i, arr) => (
+ <>
<Link
key={t.id}
to="/workspace/logs"
search={{ team_ids: [t.id] }}
className="text-blue-600 hover:underline dark:text-blue-400"
- data-testid="logdetails-team-link"
+ data-testid={`logdetails-team-link-${t.id}`}
>
{t.name}
- {i < arr.length - 1 ? "," : ""}
</Link>
+ {i < arr.length - 1 ? "," : ""}
+ </>
))}
</span>🤖 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 `@ui/app/workspace/logs/sheets/logDetailView.tsx` around lines 930 - 954, The
team links inside LogEntryDetailsView reuse the same data-testid and include the
separator comma inside the interactive Link, which breaks E2E selectors and
expands the clickable area; update the Link rendering in the mapped array (the
<Link> elements) to assign a unique test id (e.g., derive from t.id or the map
index such as data-testid={`logdetails-team-link-${t.id}` or `- ${i}`) and move
the trailing comma out of the Link so the separator is rendered after the Link
(e.g., render Link for t.name only, then conditionally render "," after the Link
when i < arr.length - 1). Ensure you update the code locations referenced by
LogEntryDetailsView and the Link mapping to apply both changes.
| {(log.business_unit_ids?.length || log.business_unit_id) && ( | ||
| <LogEntryDetailsView | ||
| className="w-full" | ||
| label="Business Unit" | ||
| label={(log.business_unit_ids?.length ?? 0) > 1 ? "Business Units" : "Business Unit"} | ||
| value={ | ||
| <Link | ||
| to="/workspace/logs" | ||
| search={{ business_unit_ids: [log.business_unit_id] }} | ||
| className="text-blue-600 hover:underline dark:text-blue-400" | ||
| data-testid="logdetails-business-unit-link" | ||
| > | ||
| {log.business_unit_name || log.business_unit_id} | ||
| </Link> | ||
| <span className="inline-flex flex-wrap gap-x-1"> | ||
| {(log.business_unit_ids?.length | ||
| ? log.business_unit_ids.map((id, i) => ({ id, name: log.business_unit_names?.[i] || id })) | ||
| : [{ id: log.business_unit_id!, name: log.business_unit_name || log.business_unit_id! }] | ||
| ).map((b, i, arr) => ( | ||
| <Link | ||
| key={b.id} | ||
| to="/workspace/logs" | ||
| search={{ business_unit_ids: [b.id] }} | ||
| className="text-blue-600 hover:underline dark:text-blue-400" | ||
| data-testid="logdetails-business-unit-link" | ||
| > | ||
| {b.name} | ||
| {i < arr.length - 1 ? "," : ""} | ||
| </Link> | ||
| ))} | ||
| </span> | ||
| } | ||
| /> | ||
| )} |
There was a problem hiding this comment.
Ensure unique data-testid values for each business unit link.
The data-testid="logdetails-business-unit-link" is repeated for every link in the map, which violates E2E testing requirements. Each business unit link needs a unique testid to enable reliable test selectors.
The comma placement inside the Link element also makes the separator part of the clickable area, which is not standard UX practice.
Proposed fix: unique testids and comma placement
<span className="inline-flex flex-wrap gap-x-1">
{(log.business_unit_ids?.length
? log.business_unit_ids.map((id, i) => ({ id, name: log.business_unit_names?.[i] || id }))
: [{ id: log.business_unit_id!, name: log.business_unit_name || log.business_unit_id! }]
- ).map((b, i, arr) => (
+ ).map((b, i, arr) => (
+ <>
<Link
key={b.id}
to="/workspace/logs"
search={{ business_unit_ids: [b.id] }}
className="text-blue-600 hover:underline dark:text-blue-400"
- data-testid="logdetails-business-unit-link"
+ data-testid={`logdetails-business-unit-link-${b.id}`}
>
{b.name}
- {i < arr.length - 1 ? "," : ""}
</Link>
+ {i < arr.length - 1 ? "," : ""}
+ </>
))}
</span>🤖 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 `@ui/app/workspace/logs/sheets/logDetailView.tsx` around lines 971 - 995, The
business unit links render duplicate data-testid values and include the comma
inside the clickable Link; update the Link rendering inside the map (where
LogEntryDetailsView renders business units from
log.business_unit_ids/log.business_unit_id) to give each Link a unique
data-testid (e.g., append the business unit id or the map index to
"logdetails-business-unit-link") and move the comma separator outside the Link
(render the comma as a sibling element after the Link, not inside it) so
separators are not part of the clickable area.
## Summary This PR bumps the Go toolchain version from `1.26.3` to `1.26.4` across all modules and CI workflows, and cuts a new release (`core` v1.5.17, `framework` v1.3.17, `transports` v1.5.9, `plugins/compat` v0.1.16, `plugins/governance` v1.5.17, and associated plugin versions) incorporating a large batch of features and fixes accumulated since the previous release. ## Changes - **Go 1.26.4** — Updated `go-version` in all GitHub Actions workflows (`e2e-tests`, `helm-release`, `pr-tests`, `release-cli`, `release-pipeline`, `snyk`) and all `go.mod` files (core, framework, transports, cli, all plugins, examples, and test modules). - **Core (v1.5.17)** — OpenAI compaction support, multi-customer logs and usage tracking, multiple team/business unit support, `request_headers` wildcard pattern capture for OTel and Maxim plugins, xAI `x_search` tool, fetch URL validation with SSRF hardening, `file://` pricing URL scheme, virtual key provider fan-out filtering, and a broad set of fixes including Anthropic prompt cache key, empty thinking block stripping, OpenAI stream usage event cleanup, Gemini numeric schema constraints, stale connection retries, Azure Claude diagnostic strip, and passthrough budget handling. - **Framework (v1.3.17)** — Scope-aware budgets and limits wired from model configs, provider-level governance, multiple customer budget support with `calendar_aligned` windows, paginated virtual key fetch, `config.json` source-of-truth flow, FTS index cap reduction, sync worker drift fix, cascade deletes for model configs, and high-scale virtual key flow improvements. - **Transports (v1.5.9)** — Full changelog covering all of the above plus UI improvements (log navigation, customer detail sheet, `BudgetDisplay` component, inline loading shell, materialized view alias), SCIM provisioning fields, Helm/config schema additions (`roles`, `per_user_oauth`), client IP resolution from forwarded headers, and dependency upgrades (`recharts` to 3.8.1, `golang.org/x` CVE remediation). - **Plugins** — `governance` v1.5.17 adds team budget/rate-limit exporters, ghost node reconciliation fix, and VK double usage counting fix; `logging` v1.5.17 adds wildcard header capture and file attachment rendering; `otel` v1.2.17 adds `disable_content_logging` and multiple collectors support; `maxim` v1.6.17 adds `request_headers` wildcard capture; `compat` v0.1.16 fixes `max_tokens` preservation during param filtering. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [x] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Verify Go version go version # should report go1.26.4 # Run core tests cd core && go test ./... # Run framework tests cd framework && go test ./... # Run transports tests cd transports && go test ./... # Run plugin tests cd plugins/governance && go test ./... cd plugins/logging && go test ./... cd plugins/otel && go test ./... # UI cd ui pnpm i pnpm build pnpm test ``` ## Breaking changes - [ ] Yes - [x] No ## Related issues #4053, #4066, #4041, #4012, #3976, #3947, #3991, #4045, #3957, #3938, #3937, #3939, #3981, #3998, #3997, #4092, #4091, #4079, #4080, #4086, #3929, #3994, #4028, #3970, #3919, #3861, #3664, #3999, #4088, #4070, #4051, #4043, #4057, #4023, #3941, #3955, #4024, #3956, #3967, #3925, #3992, #3900 ## Security considerations - Fetch URL validation hardened against SSRF by tightening IP checks for private networks and link-local addresses (#4092, #3947, #3991). - Transitive `golang.org/x` dependencies (crypto, net, sys, text) bumped to address Docker Scout CVEs (#3900). ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * OpenAI compaction, multi-customer/team logstore support, request-header wildcard capture, enhanced governance (provider-level & scope-aware limits), disable-content-logging option, support for multiple OpenTelemetry collectors, SSRF hardening and URL validation. * **Chores** * Bumped Go toolchain across modules and updated component/plugin version releases. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Enterprise requests can belong to multiple teams and business units simultaneously (the user/AP path), but the log store previously only recorded a single scalar `team_id`/`business_unit_id`. This meant teams and BUs that existed only in the multi-value path were invisible in filter dropdowns, rankings, and histograms. This PR adds JSON-array columns (`team_ids`, `team_names`, `business_unit_ids`, `business_unit_names`) to the `logs` table and wires them through the entire stack — storage, filtering, aggregation, materialized views, and UI. ## Changes - **New context keys** `BifrostContextKeyGovernanceTeamIDs`, `BifrostContextKeyGovernanceTeamNames`, `BifrostContextKeyGovernanceBusinessUnitIDs`, `BifrostContextKeyGovernanceBusinessUnitNames` added to `bifrost.go` for the enterprise governance plugin to populate. - **New `Log` table columns** `team_ids`, `team_names`, `business_unit_ids`, `business_unit_names` (stored as JSON text, serialized/deserialized via `SerializeFields`/`DeserializeFields`). Parsed slices are exposed as `TeamIDsParsed` etc. for API responses. - **Logging plugin** reads the new context keys and populates the parsed slice fields on the log entry. - **`multiValueDimensionFilterSQL`** builds a Postgres predicate that ORs a scalar `IN` (btree) with per-id `jsonb @> ?` containment (partial GIN index), so filtering by team or BU matches both old scalar rows and new array rows. - **`teamOrBUFanoutFrom`** returns a lateral-join subquery that unnests the JSON-array columns (id+name aligned by ordinality) and falls back to the scalar columns for pre-migration rows, ensuring each team/BU is credited exactly once per request in rankings and histograms. - **`GetDimensionRankings`**, **`GetDimensionCostHistogram`**, and **`GetDimensionTokenHistogram`** use the fan-out subquery for team/BU dimensions on Postgres, bypassing the matview path (which only has the scalar primary). - **`canUseMatViewFilters`** now excludes queries with `TeamIDs` or `BusinessUnitIDs` filters, forcing them to the raw path where the array containment predicate is applied. - **`mv_filter_teams` and `mv_filter_business_units`** are rebuilt with `multiValueFilterMatViewBody`, which unions the scalar column with the JSON-array fan-out so both paths appear in filter dropdowns. - **GIN indexes** `idx_logs_team_ids_gin` and `idx_logs_business_unit_ids_gin` are built concurrently post-startup (with rollback support) to accelerate `@>` containment queries. - **Migrations** added for the new columns, GIN indexes, and matview recreation. The matview recreation migration is required because the column shape is unchanged (only the SELECT body changed), so `repairMatViewShapes` would not detect the drift. - **UI log detail view** renders multiple teams/BUs as comma-separated links when the array fields are present, with a pluralized label. - **Dashboard rankings tab** shows an "attributed" label and tooltip for team/BU dimensions, explaining that a request counts toward each team/BU it belongs to. - **`LogEntry` TypeScript type** extended with `team_ids`, `team_names`, `business_unit_ids`, `business_unit_names`. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [x] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Core/Transports go version go test ./framework/logstore/... # Specific new tests go test ./framework/logstore/ -run TestMultiValueDimensionFilterSQL go test ./framework/logstore/ -run TestTeamOrBUFanoutFrom go test ./framework/logstore/ -run TestCanUseMatViewFilters_ExcludesTeamBU go test ./framework/logstore/ -run TestFilterTeamMatView_CollectsScalarAndArray go test ./framework/logstore/ -run TestFilterBusinessUnitMatView_CollectsScalarAndArray go test ./framework/logstore/ -run TestFilterTeamMatView_DACScopeAppliesAfterFanout # UI cd ui pnpm i || npm i pnpm build || npm run build ``` To validate end-to-end: send requests via the enterprise user/AP path with multiple teams/BUs set in context, then verify: 1. All teams/BUs appear in the filter dropdown (not just the scalar primary). 2. Filtering logs by a team that only exists in the array column returns the correct rows. 3. The rankings and histogram charts credit each team/BU correctly. 4. The log detail view shows all teams/BUs as separate links. ## Breaking changes - [ ] Yes - [x] No The new columns are additive and nullable. Existing scalar `team_id`/`business_unit_id` columns are unchanged. Old replicas reading the recreated matviews during a rolling deploy are unaffected because the column shape is identical. ## Security considerations - DAC scope (row-level visibility via `QueryScope`) is preserved through the fan-out subquery: the original log row's `user_id`/`team_id`/`virtual_key_id` are carried through `l.*`, so scope predicates still apply correctly. A test (`TestFilterTeamMatView_DACScopeAppliesAfterFanout`) explicitly verifies that array-only teams owned by another user do not leak across scope boundaries. - Column and index names used in `fmt.Sprintf` SQL construction are internal constants, not user input. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Support filtering and analyzing logs across multiple teams and business units (multi-value IDs/names). * Log detail view displays and links multiple teams and business units (pluralized labels). * Added an "attributed" mode in dimension rankings for team and business-unit analysis with updated total tooltip/label. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## ✨ Features - **OpenAI Compaction** — Added OpenAI conversation compaction support across core, framework, logging, and the API surface (#4053) - **Multi-Customer & Org Hierarchy** — Logs and usage tracking now support multiple customers, teams, and business units, including business unit CRUD, team assignment, and governance endpoints in the OpenAPI spec (#4066, #4041, #4082) - **Provider-Level Governance** — Budgets & limits are now scope-aware and can be applied at the virtual-key top level and per provider, wired from the model configs table, with UI filters for scope and providers (#3938, #3937, #3939, #3981, #3962) - **Customer Budgets** — Customers support multiple budgets and `calendar_aligned` budget windows (#3998, #3997) - **Virtual Key Attribution & Controls** — Added a `created_by` user attribution column and a `blacklisted_models` column for virtual key provider configs (#3672, #3653) - **Request Header Capture** — OTel and Maxim observability plugins capture `request_headers` by pattern, with wildcard support (e.g. `x-custom-*`); logging gained the same wildcard header capture (#4012, #3958) - **OTel Content Controls & Collectors** — New `disable_content_logging` option drops message/tool content from exported spans, plus support for multiple OTel collectors (#4064, #3894) - **xAI x_search** — Added xAI `x_search` tool support (#3976) - **URL Validation** — Added fetch URL validation with private-network configuration and link-local blocking (#3947, #3991) - **File Scheme Pricing URLs** — Pricing source URLs now accept the `file://` scheme for air-gapped and self-hosted deployments (#4045) - **Paginated Virtual Keys** — Virtual key fetching is paginated to handle deployments with very large numbers of keys (#3957) - **Client IP Resolution** — Resolve client IP from `X-Forwarded-For`/`X-Real-IP` headers - **SCIM Provisioning** — Added `attributeType`/`attributeValue` SCIM provisioning fields - **Helm/Config Schema** — Added `roles` RBAC governance config and `per_user_oauth` MCP auth to the Helm chart and config schema (#4004, #4009) - **Log Navigation UI** — Added a "View logs" menu item to customer, team, and virtual key tables, clickable links in log detail views, a customer detail sheet, and a reusable `BudgetDisplay` component (#4073, #4054, #4026, #4055) - **Faster First Paint** — Added an inline loading shell to `#root` before React mounts (#4063) - **Materialized View Alias** — Added an `alias` column to the materialized view with filter support (#4078) ## 🐞 Fixed - **Fetch URL IP Checks** — Hardened fetch URL IP checks against SSRF (#4092) - **Mantle Model Matching** — Broadened Mantle model matching to all `gpt` variants (#4091) - **Empty Thinking Blocks** — Strip thinking blocks when the signature is empty (#4079) - **OpenAI Stream Usage** — Removed usage from the `responses.created` event in the OpenAI stream (#4080) - **Prompt Cache Key** — Set the prompt cache key from the Anthropic integration (#4086) - **Upstream Failure Status** — Map upstream connection failures to 502 instead of 400 (#3929) (thanks [@chris-colinsky](https://github.com/chris-colinsky)!) - **Gemini Schema Constraints** — Accept numeric schema integer constraints for Gemini (#3994) (thanks [@yanhao98](https://github.com/yanhao98)!) - **Files Provider Param** — Accept the `?provider=` query param on `GET /v1/files` (#3971) (thanks [@alexef](https://github.com/alexef)!) - **Optional Batch Model** — Made the `model` field optional on `POST /v1/batches` (#3973) (thanks [@alexef](https://github.com/alexef)!) - **Helm Azure Config** — Added missing `azure_key_config` fields to the Helm schema (#3996) (thanks [@axelray-dev](https://github.com/axelray-dev)!) - **Text Completion Chunk Model** — Added the missing `Model` field to `TextCompletionChunkResponse` (#3970) (thanks [@kuishou68](https://github.com/kuishou68)!) - **MCP Inline stdio Env** — MCP stdio server configs accept inline environment variable assignments (#3861) (thanks [@Shushmitaaaa](https://github.com/Shushmitaaaa)!) - **Orphaned Tool Results** — Orphaned tool results in the OpenAI to Anthropic conversion flow are no longer rejected by the Anthropic API (#3919) - **Node Usage Reconciliation** — Added a monotonic `inc_number` log cursor so node usage reconciliation does not skip late async log writes (#3664) - **Bedrock Output Assessments** — Corrected the type of `outputAssessments` in Bedrock responses (#4028) - **Model Pool Pricing Reloads** — Preserve non-pricing model pool entries across pricing reloads (#3999) - **Ghost Node Reconciliation** — Replicate the VK hierarchy flow for ghost node reconciliation (#4088) - **VK Double Usage Counting** — Fixed double usage counting when creating a virtual key (#4070) - **Model Config Lifecycle** — Cascade deletes for model configs and removal of stale in-memory model configs (#4051, #4043) - **FTS Index Cap** — Reduced the FTS index `left()` cap from 800k to 250k chars to stay within the tsvector limit (#4057) - **Sync Worker Drift** — Reduced the sync worker ticker period to 5m to prevent threshold drift (#4023) - **Passthrough** — Fixed passthrough budgets, gated passthrough models per VK, model extraction for Azure passthrough, and restricted fallbacks/provider selection to the VK boundary (#3941, #3988, #3983, #3924) - **Provider Response Headers** — Strip provider response headers and add a content-type filter (#3955, #4024) - **Stream Handling** — Drain non-SSE stream readers and retry stale connections (#3956, #3967) - **Azure Claude** — Strip Azure diagnostic property for Claude models (#3925) - **Compat max_tokens** — Preserve chat `max_tokens` during param filtering (#3992) - **Raw Request Flag** — Removed the raw request flag from providers that don't support it (#4058) - **UI Fixes** — Standardized page container layout, virtual key model configs UI, and dashboard chart tooltips (#4046, #4052, #4044) ## 🔧 Maintenance - **Dependency Upgrades** — Bumped transitive `golang.org/x` dependencies (crypto, net, sys, text) for Docker Scout CVE remediation and `recharts` to 3.8.1; cascaded version bumps across all modules (#3900, #4003)

Summary
Enterprise requests can belong to multiple teams and business units simultaneously (the user/AP path), but the log store previously only recorded a single scalar
team_id/business_unit_id. This meant teams and BUs that existed only in the multi-value path were invisible in filter dropdowns, rankings, and histograms. This PR adds JSON-array columns (team_ids,team_names,business_unit_ids,business_unit_names) to thelogstable and wires them through the entire stack — storage, filtering, aggregation, materialized views, and UI.Changes
BifrostContextKeyGovernanceTeamIDs,BifrostContextKeyGovernanceTeamNames,BifrostContextKeyGovernanceBusinessUnitIDs,BifrostContextKeyGovernanceBusinessUnitNamesadded tobifrost.gofor the enterprise governance plugin to populate.Logtable columnsteam_ids,team_names,business_unit_ids,business_unit_names(stored as JSON text, serialized/deserialized viaSerializeFields/DeserializeFields). Parsed slices are exposed asTeamIDsParsedetc. for API responses.multiValueDimensionFilterSQLbuilds a Postgres predicate that ORs a scalarIN(btree) with per-idjsonb @> ?containment (partial GIN index), so filtering by team or BU matches both old scalar rows and new array rows.teamOrBUFanoutFromreturns a lateral-join subquery that unnests the JSON-array columns (id+name aligned by ordinality) and falls back to the scalar columns for pre-migration rows, ensuring each team/BU is credited exactly once per request in rankings and histograms.GetDimensionRankings,GetDimensionCostHistogram, andGetDimensionTokenHistogramuse the fan-out subquery for team/BU dimensions on Postgres, bypassing the matview path (which only has the scalar primary).canUseMatViewFiltersnow excludes queries withTeamIDsorBusinessUnitIDsfilters, forcing them to the raw path where the array containment predicate is applied.mv_filter_teamsandmv_filter_business_unitsare rebuilt withmultiValueFilterMatViewBody, which unions the scalar column with the JSON-array fan-out so both paths appear in filter dropdowns.idx_logs_team_ids_ginandidx_logs_business_unit_ids_ginare built concurrently post-startup (with rollback support) to accelerate@>containment queries.repairMatViewShapeswould not detect the drift.LogEntryTypeScript type extended withteam_ids,team_names,business_unit_ids,business_unit_names.Type of change
Affected areas
How to test
To validate end-to-end: send requests via the enterprise user/AP path with multiple teams/BUs set in context, then verify:
Breaking changes
The new columns are additive and nullable. Existing scalar
team_id/business_unit_idcolumns are unchanged. Old replicas reading the recreated matviews during a rolling deploy are unaffected because the column shape is identical.Security considerations
QueryScope) is preserved through the fan-out subquery: the original log row'suser_id/team_id/virtual_key_idare carried throughl.*, so scope predicates still apply correctly. A test (TestFilterTeamMatView_DACScopeAppliesAfterFanout) explicitly verifies that array-only teams owned by another user do not leak across scope boundaries.fmt.SprintfSQL construction are internal constants, not user input.Checklist
docs/contributing/README.mdand followed the guidelinesSummary by CodeRabbit