feat: adds plugin span filtering for multiple connectors - #4199
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughExtracts plugin-span filtering and reparenting into core/schemas, updates OTEL plugin to use the shared filter and aliases its types/constants, and generalizes the tracing sheet UI to edit per-plugin span filters while moving tracing controls into the OTEL view. ChangesSpan Filter Core & Plugin Tracing UI Generalization
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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 unit tests (beta)
Comment |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
cae0d6c to
4cfa28e
Compare
Confidence Score: 5/5Safe to merge — the change is a well-scoped refactor that moves stable logic into a shared package without altering behavior. All call sites in plugins/otel delegate to nil-safe receivers with identical logic; the type aliases preserve JSON parsing and external import paths; the UI guard correctly blocks premature saves by gating on a state update that forces a re-render; no new cross-module dependencies are introduced because plugins/otel already depended on core/schemas. No files require special attention. The pluginTracingSheet.tsx initialization guard is slightly non-obvious but correct. Important Files Changed
Reviews (7): Last reviewed commit: "feat: adds plugin span filtering for mul..." | Re-trigger Greptile |
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 `@ui/app/workspace/observability/sheets/pluginTracingSheet.tsx`:
- Line 178: Replace the grammatically awkward and inaccurate phrase that uses
"inside the {destination}" with a reference to the actual plugin key by using
pluginName; update the text so it reads something like: "If plugin_span_filter
is set in the {pluginName} plugin config in config.json, it takes precedence
over these settings after restarting Bifrost." Locate the string in
pluginTracingSheet.tsx (the line currently referencing destination) and swap
destination for pluginName and "inside the" for "in the" to avoid double
articles and to point users to the real config key.
🪄 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: 57f65d45-1d23-4d1a-9abb-4d8cf85efb17
📒 Files selected for processing (9)
core/schemas/span_filter.gocore/schemas/span_filter_test.goplugins/otel/converter.goplugins/otel/converter_test.goplugins/otel/main.goui/app/workspace/observability/sheets/pluginTracingSheet.tsxui/app/workspace/observability/views/plugins/otelView.tsxui/app/workspace/plugins/page.tsxui/app/workspace/plugins/views/pluginsEmptyState.tsx
4cfa28e to
e6bc951
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/span_filter_test.go`:
- Around line 30-49: TestPluginNameFromSpan is missing tests for two
malformed-name scenarios; add entries to the tests slice to cover a two-segment
malformed name and a wrong-prefix case so PluginNameFromSpan's parser contract
is locked. Specifically, in the tests array inside TestPluginNameFromSpan add a
case with span produced by pluginSpan(..., "plugin.logging") expecting "" and a
case with pluginSpan(..., "otel.logging.prehook") expecting "" (also keep
existing "plugin" and nil cases) to ensure PluginNameFromSpan and
ShouldExportSpan continue to treat these as non-plugin names.
In `@core/schemas/span_filter.go`:
- Around line 96-109: The second-pass loop over the filtered map (using maxHops,
spanID, parentID, grandParentID) can still produce cycles that resolve to
another filtered span; after the bounded walk, check if the final parentID is
still a key in filtered and if so set filtered[spanID] = "" (collapse to root)
instead of leaving it pointing to another filtered span; update the logic in the
loop that currently uses maxHops to perform this final membership test and
assignment so any remaining filtered parent becomes "".
- Around line 43-54: The PluginNameFromSpan function currently treats any
dot-containing SpanKindPlugin name as a plugin; update it so it only returns the
middle name when the span name strictly matches the contract
"plugin.<name>.prehook" or "plugin.<name>.posthook": in PluginNameFromSpan
verify parts := strings.SplitN(span.Name, ".", 3) yields exactly 3 parts,
parts[0] == "plugin", and parts[2] is either "prehook" or "posthook"; otherwise
return "".
In `@ui/app/workspace/observability/sheets/pluginTracingSheet.tsx`:
- Around line 72-80: The sheet initialization can run before plugin names load
and overwrite persisted include-mode filters; modify the useEffect
initialization block (the effect that checks open && !wasOpenRef.current and
calls resolveToggleState/setToggles) to bail out when the saved filter is
include-mode but allPlugins is empty — specifically, after computing filter =
targetPlugin.config?.plugin_span_filter, if filter?.mode === "include" &&
allPlugins.length === 0 return early so you don't call
setToggles(resolveToggleState(...)) and flip include filters; keep
wasOpenRef.current unchanged until real initialization runs.
🪄 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: 2afc113a-64cc-41d9-b479-e8d8ce45b42a
📒 Files selected for processing (9)
core/schemas/span_filter.gocore/schemas/span_filter_test.goplugins/otel/converter.goplugins/otel/converter_test.goplugins/otel/main.goui/app/workspace/observability/sheets/pluginTracingSheet.tsxui/app/workspace/observability/views/plugins/otelView.tsxui/app/workspace/plugins/page.tsxui/app/workspace/plugins/views/pluginsEmptyState.tsx
e6bc951 to
7a6c43b
Compare
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 `@plugins/otel/converter_test.go`:
- Line 93: The current tests in pluginSpanFilter.ShouldExportSpan duplicate
core/schemas/span_filter_test.go; instead replace or refactor them to test
convertTraceToResourceSpan's end-to-end behavior: construct a trace containing
spans that the plugin filter should drop and spans that should be kept, run
convertTraceToResourceSpan (or the higher-level function that calls it), and
assert the exported OTEL spans' SpanId and ParentSpanId rewrites and that
filtered plugin spans are not present; update assertions that currently call
pluginSpanFilter.ShouldExportSpan to validate the converted OTEL output (span
IDs, parent relationships, and absence of dropped plugin spans) so regressions
in convertTraceToResourceSpan are caught.
In `@ui/app/workspace/observability/sheets/pluginTracingSheet.tsx`:
- Around line 87-90: The save handler (handleSave) can run before tracing
toggles are initialized (e.g., include-mode deferral) causing buildFilter to
return null and clearing plugin_span_filter; update handleSave to guard against
uninitialized toggles by either (A) early-returning with a user-visible toast
when toggles are not ready (so Save cannot proceed while targetPlugin exists) or
(B) preserving the existing plugin_span_filter when buildFilter returns null
(skip updating plugin_span_filter instead of writing null). Refer to handleSave,
buildFilter, toggles, targetPlugin and plugin_span_filter when making the
change.
🪄 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: 4655c3a5-4960-4afa-be6e-6db9b78f51ad
📒 Files selected for processing (9)
core/schemas/span_filter.gocore/schemas/span_filter_test.goplugins/otel/converter.goplugins/otel/converter_test.goplugins/otel/main.goui/app/workspace/observability/sheets/pluginTracingSheet.tsxui/app/workspace/observability/views/plugins/otelView.tsxui/app/workspace/plugins/page.tsxui/app/workspace/plugins/views/pluginsEmptyState.tsx
7a6c43b to
7e12811
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
core/schemas/span_filter.go (1)
57-58:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReject empty or over-segmented plugin stages.
Line 57 still treats malformed names like
plugin.logging.andplugin.logging.prehook.extraas valid plugin spans, becauseSplitN(..., 3)collapses extra.into the stage segment and Line 58 never checks that the stage is non-empty. That breaks the documented “malformed spans pass through” behavior by letting them be filtered/reparented.Suggested fix
- parts := strings.SplitN(span.Name, ".", 3) - if len(parts) != 3 || parts[0] != "plugin" || parts[1] == "" { + parts := strings.Split(span.Name, ".") + if len(parts) != 3 || parts[0] != "plugin" || parts[1] == "" || parts[2] == "" { return "" }🤖 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 `@core/schemas/span_filter.go` around lines 57 - 58, The current check uses strings.SplitN(span.Name, ".", 3) which allows malformed names like "plugin.logging." or "plugin.logging.prehook.extra" to pass; change to use strings.Split(span.Name, ".") (or otherwise ensure there are exactly 3 segments) and add explicit emptiness checks for both the stage and the final segment: require len(parts) == 3, parts[0] == "plugin", parts[1] != "" and parts[2] != "" so names with an empty stage or extra dot-segments are treated as malformed and pass through rather than being handled as valid plugin spans.
🤖 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 `@ui/app/workspace/plugins/page.tsx`:
- Line 52: The docs still point users to the Plugins page for "Configure Plugin
Tracing" even though the Plugins page no longer exposes tracing controls (see
PluginsEmptyState usage and surrounding removal of tracing UI in page.tsx);
update docs/features/observability/otel.mdx to point the OTEL configuration flow
to the new OTEL view entrypoint instead of the Plugins page—replace any links,
button targets, and instructional text that reference the Plugins page with the
new OTEL view entrypoint name, adjust any navigation examples, and update
screenshots or step labels as needed so the doc flow correctly opens the OTEL
view for Configure Plugin Tracing.
- Line 52: Update tests and docs to reference the new tracing UI: replace legacy
Playwright/UI selectors like "plugins-tracing-button" with the new data-testids
"otel-configure-tracing-button" and the "plugin-tracing-*" controls, update any
E2E specs that previously targeted the Plugins page flow to instead interact
with the tracing button rendered by the otel view (component otelView /
data-testid="otel-configure-tracing-button"), and refresh docs
(docs/features/observability/otel.mdx) to instruct users to open the
Observability → Configure Plugin Tracing UI from the OTEL view rather than the
old Plugins page; ensure E2E tests assert the presence and behavior of the new
elements and that selectors in tests/e2e and ui references are fully replaced.
---
Duplicate comments:
In `@core/schemas/span_filter.go`:
- Around line 57-58: The current check uses strings.SplitN(span.Name, ".", 3)
which allows malformed names like "plugin.logging." or
"plugin.logging.prehook.extra" to pass; change to use strings.Split(span.Name,
".") (or otherwise ensure there are exactly 3 segments) and add explicit
emptiness checks for both the stage and the final segment: require len(parts) ==
3, parts[0] == "plugin", parts[1] != "" and parts[2] != "" so names with an
empty stage or extra dot-segments are treated as malformed and pass through
rather than being handled as valid plugin spans.
🪄 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: 40fbc50b-feb9-4994-8cc1-e9a31e9d72e8
📒 Files selected for processing (9)
core/schemas/span_filter.gocore/schemas/span_filter_test.goplugins/otel/converter.goplugins/otel/converter_test.goplugins/otel/main.goui/app/workspace/observability/sheets/pluginTracingSheet.tsxui/app/workspace/observability/views/plugins/otelView.tsxui/app/workspace/plugins/page.tsxui/app/workspace/plugins/views/pluginsEmptyState.tsx
7e12811 to
7473d46
Compare
7473d46 to
a02e997
Compare
Merge activity
|
a02e997 to
af382a8
Compare
## Summary `PluginSpanFilter` and its associated logic (`ShouldExportSpan`, `BuildReparentMap`, `PluginNameFromSpan`) were previously defined and implemented inside the OTEL plugin package. This PR promotes them to `core/schemas` so they can be shared across all observability connectors (OTEL, Datadog, BigQuery) without duplicating the span-name contract or reparenting behavior. The OTEL package re-exports the types and constants as aliases to preserve existing import paths. The `PluginTracingSheet` UI component is also generalized to accept a `pluginName` and `destination` prop, and is relocated from the plugins page to the OTEL observability view where it belongs. ## Changes - Introduced `core/schemas/span_filter.go` with `PluginSpanFilter`, `PluginSpanFilterMode`, `PluginNameFromSpan`, `ShouldExportSpan`, and `BuildReparentMap`, along with full unit test coverage in `span_filter_test.go`. - Removed the duplicate `shouldExportSpan` and `buildReparentMap` methods from `plugins/otel/converter.go`; call sites now delegate to the shared schema methods. - `PluginSpanFilter`, `PluginSpanFilterMode`, and the include/exclude constants in `plugins/otel/main.go` are replaced with type aliases and const aliases pointing to `core/schemas`, keeping the OTEL package's public API unchanged. - Validation in `otel.Init` is replaced with a call to `config.PluginSpanFilter.Validate()`. - `PluginTracingSheet` is moved from `ui/app/workspace/plugins/sheets/` to `ui/app/workspace/observability/sheets/` and now accepts `pluginName` and `destination` props, making it connector-agnostic. - The "Configure Plugin Tracing" button and `PluginTracingSheet` are removed from the plugins page and plugins empty state, and are instead surfaced directly in `OtelView`. - Added a warning notice to the `ent-v1.4.7` changelog about a known `/virtual-key/quota` issue fixed in v1.4.8. - Improved the `v1.5.11` changelog rollback section with a warning callout and collapsible `AccordionGroup` sections for single-node and multi-node rollback SQL. ## Type of change - [ ] Bug fix - [ ] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [x] Plugins - [x] UI (React) - [x] Docs ## How to test ```sh # Core/Transports go test ./core/schemas/... ./plugins/otel/... # UI cd ui pnpm i pnpm build ``` - Open the Observability → OTEL view and confirm the "Configure Plugin Tracing" button appears and opens the sheet correctly. - Verify the sheet reads and writes `plugin_span_filter` only for the `otel` plugin. - Confirm the Plugins page no longer shows a "Configure Plugin Tracing" button or sheet. - Confirm the plugins empty state no longer renders the tracing button. ## Screenshots/Recordings N/A — functional behavior is unchanged; only the location of the tracing button has moved. ## Breaking changes - [ ] Yes - [x] No The OTEL package re-exports all renamed types and constants as aliases, so existing config parsing and external consumers are unaffected. ## Related issues N/A ## Security considerations No new auth, secrets, PII handling, or sandboxing changes introduced. ## 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) - [ ] I verified the CI pipeline passes locally if applicable <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Added configurable span filtering for plugin observability exports with include/exclude modes to control which plugins' spans are exported to observability connectors. * Extended plugin tracing configuration to support any backend plugin destination, not limited to a single connector. * **Refactor** * Consolidated span filtering logic for improved reusability and consistency across observability integrations. <!-- end of auto-generated comment: release notes by coderabbit.ai -->

Summary
PluginSpanFilterand its associated logic (ShouldExportSpan,BuildReparentMap,PluginNameFromSpan) were previously defined and implemented inside the OTEL plugin package. This PR promotes them tocore/schemasso they can be shared across all observability connectors (OTEL, Datadog, BigQuery) without duplicating the span-name contract or reparenting behavior. The OTEL package re-exports the types and constants as aliases to preserve existing import paths. ThePluginTracingSheetUI component is also generalized to accept apluginNameanddestinationprop, and is relocated from the plugins page to the OTEL observability view where it belongs.Changes
core/schemas/span_filter.gowithPluginSpanFilter,PluginSpanFilterMode,PluginNameFromSpan,ShouldExportSpan, andBuildReparentMap, along with full unit test coverage inspan_filter_test.go.shouldExportSpanandbuildReparentMapmethods fromplugins/otel/converter.go; call sites now delegate to the shared schema methods.PluginSpanFilter,PluginSpanFilterMode, and the include/exclude constants inplugins/otel/main.goare replaced with type aliases and const aliases pointing tocore/schemas, keeping the OTEL package's public API unchanged.otel.Initis replaced with a call toconfig.PluginSpanFilter.Validate().PluginTracingSheetis moved fromui/app/workspace/plugins/sheets/toui/app/workspace/observability/sheets/and now acceptspluginNameanddestinationprops, making it connector-agnostic.PluginTracingSheetare removed from the plugins page and plugins empty state, and are instead surfaced directly inOtelView.ent-v1.4.7changelog about a known/virtual-key/quotaissue fixed in v1.4.8.v1.5.11changelog rollback section with a warning callout and collapsibleAccordionGroupsections for single-node and multi-node rollback SQL.Type of change
Affected areas
How to test
plugin_span_filteronly for theotelplugin.Screenshots/Recordings
N/A — functional behavior is unchanged; only the location of the tracing button has moved.
Breaking changes
The OTEL package re-exports all renamed types and constants as aliases, so existing config parsing and external consumers are unaffected.
Related issues
N/A
Security considerations
No new auth, secrets, PII handling, or sandboxing changes introduced.
Checklist
docs/contributing/README.mdand followed the guidelinesSummary by CodeRabbit
Release Notes
New Features
Refactor