diff --git a/core/schemas/span_filter.go b/core/schemas/span_filter.go new file mode 100644 index 00000000000..cd484229422 --- /dev/null +++ b/core/schemas/span_filter.go @@ -0,0 +1,118 @@ +package schemas + +import ( + "fmt" + "slices" + "strings" +) + +// PluginSpanFilterMode controls whether the plugins list is an allowlist or denylist. +type PluginSpanFilterMode string + +const ( + // PluginSpanFilterModeInclude exports only the listed plugins' spans. + PluginSpanFilterModeInclude PluginSpanFilterMode = "include" + // PluginSpanFilterModeExclude exports everything except the listed plugins' spans. + PluginSpanFilterModeExclude PluginSpanFilterMode = "exclude" +) + +// PluginSpanFilter configures which plugin spans an observability connector exports. +// Mode "include" exports only the listed plugins; mode "exclude" exports everything +// except them. It is shared by every observability connector (OTEL, Datadog, BigQuery) +// so the span-name contract and reparenting behavior stay consistent across exporters. +type PluginSpanFilter struct { + Mode PluginSpanFilterMode `json:"mode"` + Plugins []string `json:"plugins"` +} + +// Validate reports whether the filter's mode is one of the two valid modes. +// A nil filter is valid (it filters nothing). +func (f *PluginSpanFilter) Validate() error { + if f == nil { + return nil + } + switch f.Mode { + case PluginSpanFilterModeInclude, PluginSpanFilterModeExclude: + return nil + default: + return fmt.Errorf("plugin_span_filter.mode %q is invalid: must be %q or %q", + f.Mode, PluginSpanFilterModeInclude, PluginSpanFilterModeExclude) + } +} + +// PluginNameFromSpan extracts "" from a plugin span whose name follows the +// core tracer contract "plugin..", where is one of prehook, +// posthook, mcp_prehook, mcp_posthook, mcp_connect_prehook, or mcp_connect_posthook +// (see core/bifrost.go). It returns "" for non-plugin spans or names that don't match +// the contract (wrong prefix, or fewer than three segments), so malformed names pass +// through ShouldExportSpan as exported rather than being silently filtered. +// +// The segment is intentionally not constrained to a fixed list: the tracer +// emits several hook stages (including the mcp_* variants above), so pinning it to +// just prehook/posthook would make every MCP-hook span unfilterable. +func PluginNameFromSpan(span *Span) string { + if span == nil || span.Kind != SpanKindPlugin { + return "" + } + parts := strings.SplitN(span.Name, ".", 3) + if len(parts) != 3 || parts[0] != "plugin" || parts[1] == "" { + return "" + } + return parts[1] +} + +// ShouldExportSpan reports whether a span survives the filter. Non-plugin spans and +// spans evaluated against a nil filter are always exported. Plugin spans are checked +// against the filter's plugin list and mode. +func (f *PluginSpanFilter) ShouldExportSpan(span *Span) bool { + if f == nil || span == nil || span.Kind != SpanKindPlugin { + return true + } + pluginName := PluginNameFromSpan(span) + if pluginName == "" { + // Malformed plugin span name: export rather than silently drop. + return true + } + inList := slices.Contains(f.Plugins, pluginName) + if f.Mode == PluginSpanFilterModeInclude { + return inList + } + return !inList // exclude mode +} + +// BuildReparentMap returns a map of filteredSpanID → effective ancestor spanID for all +// spans that the filter removes. When plugin spans are chained (each span's parent is the +// previous plugin's span), removing a span from the middle would leave its children with a +// dangling parent ID. The map lets callers rewrite those parent IDs to the nearest exported +// ancestor, handling consecutive filtered spans in a chain. Returns nil when the filter is +// nil or nothing is filtered. +func (f *PluginSpanFilter) BuildReparentMap(spans []*Span) map[string]string { + if f == nil { + return nil + } + // First pass: record direct parent ID for every filtered span. + filtered := make(map[string]string) // spanID -> parentID + for _, span := range spans { + if !f.ShouldExportSpan(span) { + filtered[span.SpanID] = span.ParentID + } + } + if len(filtered) == 0 { + return nil + } + // Second pass: resolve chains so each filtered span maps to its first exported ancestor. + // Cap the walk at len(filtered) to break out of any cycle caused by malformed span data. + maxHops := len(filtered) + for spanID := range filtered { + parentID := filtered[spanID] + for range maxHops { + grandParentID, isFiltered := filtered[parentID] + if !isFiltered { + break + } + parentID = grandParentID + } + filtered[spanID] = parentID + } + return filtered +} diff --git a/core/schemas/span_filter_test.go b/core/schemas/span_filter_test.go new file mode 100644 index 00000000000..093a90ee471 --- /dev/null +++ b/core/schemas/span_filter_test.go @@ -0,0 +1,143 @@ +package schemas + +import "testing" + +func pluginSpan(id, parent, name string) *Span { + return &Span{SpanID: id, ParentID: parent, Name: name, Kind: SpanKindPlugin} +} + +func TestPluginSpanFilter_Validate(t *testing.T) { + tests := []struct { + name string + filter *PluginSpanFilter + wantErr bool + }{ + {"nil filter", nil, false}, + {"include", &PluginSpanFilter{Mode: PluginSpanFilterModeInclude}, false}, + {"exclude", &PluginSpanFilter{Mode: PluginSpanFilterModeExclude}, false}, + {"invalid mode", &PluginSpanFilter{Mode: "nonsense"}, true}, + {"empty mode", &PluginSpanFilter{Mode: ""}, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := tt.filter.Validate(); (err != nil) != tt.wantErr { + t.Errorf("Validate() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +func TestPluginNameFromSpan(t *testing.T) { + tests := []struct { + name string + span *Span + want string + }{ + {"prehook", pluginSpan("1", "", "plugin.logging.prehook"), "logging"}, + {"posthook", pluginSpan("1", "", "plugin.compat.posthook"), "compat"}, + {"mcp hook stage still resolves", pluginSpan("1", "", "plugin.governance.mcp_connect_prehook"), "governance"}, + {"non-plugin kind", &Span{Name: "plugin.logging.prehook", Kind: SpanKindLLMCall}, ""}, + {"malformed name", pluginSpan("1", "", "plugin"), ""}, + {"missing stage", pluginSpan("1", "", "plugin.logging"), ""}, + {"wrong prefix", pluginSpan("1", "", "otel.logging.prehook"), ""}, + {"empty name segment", pluginSpan("1", "", "plugin..prehook"), ""}, + {"nil span", nil, ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := PluginNameFromSpan(tt.span); got != tt.want { + t.Errorf("PluginNameFromSpan() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestPluginSpanFilter_ShouldExportSpan(t *testing.T) { + llm := &Span{SpanID: "llm", Name: "llm.call", Kind: SpanKindLLMCall} + logging := pluginSpan("p1", "", "plugin.logging.prehook") + compat := pluginSpan("p2", "", "plugin.compat.prehook") + + tests := []struct { + name string + filter *PluginSpanFilter + span *Span + want bool + }{ + {"nil filter exports plugin", nil, logging, true}, + {"non-plugin always exported", &PluginSpanFilter{Mode: PluginSpanFilterModeInclude, Plugins: []string{"logging"}}, llm, true}, + {"include lists plugin", &PluginSpanFilter{Mode: PluginSpanFilterModeInclude, Plugins: []string{"logging"}}, logging, true}, + {"include omits plugin", &PluginSpanFilter{Mode: PluginSpanFilterModeInclude, Plugins: []string{"logging"}}, compat, false}, + {"exclude lists plugin", &PluginSpanFilter{Mode: PluginSpanFilterModeExclude, Plugins: []string{"logging"}}, logging, false}, + {"exclude omits plugin", &PluginSpanFilter{Mode: PluginSpanFilterModeExclude, Plugins: []string{"logging"}}, compat, true}, + {"malformed plugin span exported", &PluginSpanFilter{Mode: PluginSpanFilterModeInclude, Plugins: []string{"logging"}}, pluginSpan("p3", "", "plugin"), true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.filter.ShouldExportSpan(tt.span); got != tt.want { + t.Errorf("ShouldExportSpan() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestPluginSpanFilter_BuildReparentMap(t *testing.T) { + t.Run("nil filter returns nil", func(t *testing.T) { + f := (*PluginSpanFilter)(nil) + if got := f.BuildReparentMap([]*Span{pluginSpan("1", "", "plugin.logging.prehook")}); got != nil { + t.Errorf("expected nil, got %v", got) + } + }) + + t.Run("nothing filtered returns nil", func(t *testing.T) { + f := &PluginSpanFilter{Mode: PluginSpanFilterModeExclude, Plugins: []string{"absent"}} + spans := []*Span{pluginSpan("1", "", "plugin.logging.prehook")} + if got := f.BuildReparentMap(spans); got != nil { + t.Errorf("expected nil, got %v", got) + } + }) + + t.Run("single filtered span maps to its parent", func(t *testing.T) { + // root(llm) <- logging <- compat. Exclude logging only. + f := &PluginSpanFilter{Mode: PluginSpanFilterModeExclude, Plugins: []string{"logging"}} + spans := []*Span{ + {SpanID: "root", Name: "llm.call", Kind: SpanKindLLMCall}, + pluginSpan("logging", "root", "plugin.logging.prehook"), + pluginSpan("compat", "logging", "plugin.compat.prehook"), + } + got := f.BuildReparentMap(spans) + if got["logging"] != "root" { + t.Errorf("logging should reparent to root, got %q", got["logging"]) + } + }) + + t.Run("chain of filtered spans resolves to first exported ancestor", func(t *testing.T) { + // root(llm) <- a <- b <- c. Exclude a and b. c should reparent to root. + f := &PluginSpanFilter{Mode: PluginSpanFilterModeExclude, Plugins: []string{"a", "b"}} + spans := []*Span{ + {SpanID: "root", Name: "llm.call", Kind: SpanKindLLMCall}, + pluginSpan("a", "root", "plugin.a.prehook"), + pluginSpan("b", "a", "plugin.b.prehook"), + pluginSpan("c", "b", "plugin.c.prehook"), + } + got := f.BuildReparentMap(spans) + if got["a"] != "root" { + t.Errorf("a should resolve to root, got %q", got["a"]) + } + if got["b"] != "root" { + t.Errorf("b should resolve to root, got %q", got["b"]) + } + if _, ok := got["c"]; ok { + t.Errorf("c is exported and should not be in the map") + } + }) + + t.Run("cycle is bounded and does not hang", func(t *testing.T) { + // Malformed: a's parent is b, b's parent is a. Both filtered. + f := &PluginSpanFilter{Mode: PluginSpanFilterModeExclude, Plugins: []string{"a", "b"}} + spans := []*Span{ + pluginSpan("a", "b", "plugin.a.prehook"), + pluginSpan("b", "a", "plugin.b.prehook"), + } + _ = f.BuildReparentMap(spans) // must terminate + }) +} diff --git a/plugins/otel/converter.go b/plugins/otel/converter.go index d5a6fe281bd..924bbe679e1 100644 --- a/plugins/otel/converter.go +++ b/plugins/otel/converter.go @@ -3,7 +3,6 @@ package otel import ( "encoding/hex" "fmt" - "slices" "strings" "github.com/maximhq/bifrost/core/schemas" @@ -70,72 +69,15 @@ func hexToBytes(hexStr string, length int) []byte { return bytes } -// shouldExportSpan reports whether a span should be included in the export. -// Non-plugin spans are always exported. Plugin spans are checked against pluginSpanFilter. -func (p *OtelPlugin) shouldExportSpan(span *schemas.Span) bool { - if span.Kind != schemas.SpanKindPlugin || p.pluginSpanFilter == nil { - return true - } - // Span names follow the pattern "plugin..prehook" / "plugin..posthook". - parts := strings.SplitN(span.Name, ".", 3) - if len(parts) < 2 { - return true - } - pluginName := parts[1] - - inList := slices.Contains(p.pluginSpanFilter.Plugins, pluginName) - - if p.pluginSpanFilter.Mode == PluginSpanFilterModeInclude { - return inList - } - return !inList // exclude mode -} - -// buildReparentMap returns a map of filteredSpanID → effective ancestor spanID for all -// spans that will be skipped. When plugin spans are chained (each span's parent is the -// previous plugin's span), removing a span from the middle would leave its children with -// a dangling parent ID. The map lets us rewrite those parent IDs to the nearest exported -// ancestor, handling consecutive filtered spans in a chain. -func (p *OtelPlugin) buildReparentMap(spans []*schemas.Span) map[string]string { - if p.pluginSpanFilter == nil { - return nil - } - // First pass: record direct parent ID for every filtered span. - filtered := make(map[string]string) // spanID -> parentID - for _, span := range spans { - if !p.shouldExportSpan(span) { - filtered[span.SpanID] = span.ParentID - } - } - if len(filtered) == 0 { - return nil - } - // Second pass: resolve chains so each filtered span maps to its first exported ancestor. - // Cap the walk at len(filtered) to break out of any cycle caused by malformed span data. - maxHops := len(filtered) - for spanID := range filtered { - parentID := filtered[spanID] - for range maxHops { - grandParentID, isFiltered := filtered[parentID] - if !isFiltered { - break - } - parentID = grandParentID - } - filtered[spanID] = parentID - } - return filtered -} - // convertTraceToResourceSpan converts a Bifrost trace to OTEL ResourceSpan for the given // profile service name. Span filtering and instance attributes are shared across profiles; // only the resource service name differs per profile. func (p *OtelPlugin) convertTraceToResourceSpan(serviceName string, trace *schemas.Trace, requestHeaders []string, disableContentLogging bool) *ResourceSpan { - reparent := p.buildReparentMap(trace.Spans) + reparent := p.pluginSpanFilter.BuildReparentMap(trace.Spans) filteredHeaders := schemas.FilterHeaders(trace.RequestHeaders, requestHeaders) otelSpans := make([]*Span, 0, len(trace.Spans)) for _, span := range trace.Spans { - if !p.shouldExportSpan(span) { + if !p.pluginSpanFilter.ShouldExportSpan(span) { continue } otelSpan := convertSpanToOTELSpan(trace.TraceID, span, disableContentLogging) diff --git a/plugins/otel/converter_test.go b/plugins/otel/converter_test.go index f9e8833b936..afcabf5b3d9 100644 --- a/plugins/otel/converter_test.go +++ b/plugins/otel/converter_test.go @@ -1,6 +1,7 @@ package otel import ( + "bytes" "testing" "time" @@ -18,161 +19,50 @@ func makeSpan(id, parentID, name string, kind schemas.SpanKind) *schemas.Span { } } -func TestShouldExportSpan(t *testing.T) { - tests := []struct { - name string - filter *PluginSpanFilter - span *schemas.Span - want bool - }{ - { - name: "nil filter exports everything", - filter: nil, - span: makeSpan("1", "", "plugin.logging.prehook", schemas.SpanKindPlugin), - want: true, - }, - { - name: "non-plugin span always exported regardless of filter", - filter: &PluginSpanFilter{Mode: PluginSpanFilterModeExclude, Plugins: []string{"logging"}}, - span: makeSpan("1", "", "llm.call", schemas.SpanKindLLMCall), - want: true, - }, - { - name: "exclude mode: plugin in list is suppressed", - filter: &PluginSpanFilter{Mode: PluginSpanFilterModeExclude, Plugins: []string{"logging", "compat"}}, - span: makeSpan("1", "", "plugin.logging.prehook", schemas.SpanKindPlugin), - want: false, - }, - { - name: "exclude mode: plugin not in list is exported", - filter: &PluginSpanFilter{Mode: PluginSpanFilterModeExclude, Plugins: []string{"logging"}}, - span: makeSpan("1", "", "plugin.governance.posthook", schemas.SpanKindPlugin), - want: true, - }, - { - name: "exclude mode: posthook variant suppressed the same as prehook", - filter: &PluginSpanFilter{Mode: PluginSpanFilterModeExclude, Plugins: []string{"logging"}}, - span: makeSpan("1", "", "plugin.logging.posthook", schemas.SpanKindPlugin), - want: false, - }, - { - name: "include mode: plugin in list is exported", - filter: &PluginSpanFilter{Mode: PluginSpanFilterModeInclude, Plugins: []string{"guardrails"}}, - span: makeSpan("1", "", "plugin.guardrails.prehook", schemas.SpanKindPlugin), - want: true, - }, - { - name: "include mode: plugin not in list is suppressed", - filter: &PluginSpanFilter{Mode: PluginSpanFilterModeInclude, Plugins: []string{"guardrails"}}, - span: makeSpan("1", "", "plugin.logging.prehook", schemas.SpanKindPlugin), - want: false, - }, - { - name: "exclude mode: empty list suppresses nothing", - filter: &PluginSpanFilter{Mode: PluginSpanFilterModeExclude, Plugins: []string{}}, - span: makeSpan("1", "", "plugin.logging.prehook", schemas.SpanKindPlugin), - want: true, - }, - { - name: "include mode: empty list suppresses everything", - filter: &PluginSpanFilter{Mode: PluginSpanFilterModeInclude, Plugins: []string{}}, - span: makeSpan("1", "", "plugin.logging.prehook", schemas.SpanKindPlugin), - want: false, - }, - { - name: "span name without dots passes through", - filter: &PluginSpanFilter{Mode: PluginSpanFilterModeExclude, Plugins: []string{"logging"}}, - span: makeSpan("1", "", "nodots", schemas.SpanKindPlugin), - want: true, - }, - } +// TestConvertTraceToResourceSpan_PluginSpanFilter exercises the OTEL converter's end-to-end +// filtering behavior (the parts unique to this package; the filter/reparent logic itself is +// covered by core/schemas/span_filter_test.go). It asserts that filtered plugin spans are +// dropped from the exported ResourceSpan and that an exported child whose direct parent was +// filtered is re-parented to the nearest exported ancestor. +func TestConvertTraceToResourceSpan_PluginSpanFilter(t *testing.T) { + p := &OtelPlugin{pluginSpanFilter: &PluginSpanFilter{ + Mode: PluginSpanFilterModeExclude, + Plugins: []string{"logging"}, + }} - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - p := &OtelPlugin{pluginSpanFilter: tt.filter} - if got := p.shouldExportSpan(tt.span); got != tt.want { - t.Errorf("shouldExportSpan() = %v, want %v", got, tt.want) - } - }) + // Span tree: root (internal) -> logging.prehook (filtered) -> governance.prehook (kept). + root := makeSpan("aaaa", "", "request", schemas.SpanKindInternal) + trace := &schemas.Trace{ + TraceID: "00000000000000000000000000000001", + RootSpan: root, + Spans: []*schemas.Span{ + root, + makeSpan("bbbb", "aaaa", "plugin.logging.prehook", schemas.SpanKindPlugin), + makeSpan("cccc", "bbbb", "plugin.governance.prehook", schemas.SpanKindPlugin), + }, } -} - -func TestBuildReparentMap(t *testing.T) { - excludeLogging := &PluginSpanFilter{Mode: PluginSpanFilterModeExclude, Plugins: []string{"logging"}} - - t.Run("nil filter returns nil map", func(t *testing.T) { - p := &OtelPlugin{pluginSpanFilter: nil} - spans := []*schemas.Span{makeSpan("a", "root", "plugin.logging.prehook", schemas.SpanKindPlugin)} - if m := p.buildReparentMap(spans); m != nil { - t.Errorf("expected nil, got %v", m) - } - }) - - t.Run("no filtered spans returns nil map", func(t *testing.T) { - p := &OtelPlugin{pluginSpanFilter: excludeLogging} - spans := []*schemas.Span{ - makeSpan("a", "root", "plugin.governance.prehook", schemas.SpanKindPlugin), - } - if m := p.buildReparentMap(spans); m != nil { - t.Errorf("expected nil, got %v", m) - } - }) - - t.Run("single filtered span maps to its direct parent", func(t *testing.T) { - p := &OtelPlugin{pluginSpanFilter: excludeLogging} - // root -> logging (filtered) -> governance - spans := []*schemas.Span{ - makeSpan("root", "", "request", schemas.SpanKindInternal), - makeSpan("log-pre", "root", "plugin.logging.prehook", schemas.SpanKindPlugin), - makeSpan("gov-pre", "log-pre", "plugin.governance.prehook", schemas.SpanKindPlugin), - } - m := p.buildReparentMap(spans) - if m == nil { - t.Fatal("expected non-nil map") - } - if got := m["log-pre"]; got != "root" { - t.Errorf("filtered span should map to parent 'root', got %q", got) - } - }) - t.Run("chain of filtered spans resolves to nearest exported ancestor", func(t *testing.T) { - // root -> telemetry (filtered) -> logging (filtered) -> governance - p := &OtelPlugin{pluginSpanFilter: &PluginSpanFilter{ - Mode: PluginSpanFilterModeExclude, - Plugins: []string{"telemetry", "logging"}, - }} - spans := []*schemas.Span{ - makeSpan("root", "", "request", schemas.SpanKindInternal), - makeSpan("tel-pre", "root", "plugin.telemetry.prehook", schemas.SpanKindPlugin), - makeSpan("log-pre", "tel-pre", "plugin.logging.prehook", schemas.SpanKindPlugin), - makeSpan("gov-pre", "log-pre", "plugin.governance.prehook", schemas.SpanKindPlugin), - } - m := p.buildReparentMap(spans) - if m == nil { - t.Fatal("expected non-nil map") - } - // Both filtered spans must resolve to "root" so governance.prehook re-parents there. - if got := m["tel-pre"]; got != "root" { - t.Errorf("tel-pre should resolve to 'root', got %q", got) - } - if got := m["log-pre"]; got != "root" { - t.Errorf("log-pre should skip the chain and resolve to 'root', got %q", got) - } - }) + rs := p.convertTraceToResourceSpan("svc", trace, nil, false) + spans := rs.ScopeSpans[0].Spans - t.Run("filtered span with no parent resolves to empty string", func(t *testing.T) { - p := &OtelPlugin{pluginSpanFilter: excludeLogging} - spans := []*schemas.Span{ - // logging span has no parent (root of trace) - makeSpan("log-pre", "", "plugin.logging.prehook", schemas.SpanKindPlugin), - makeSpan("gov-pre", "log-pre", "plugin.governance.prehook", schemas.SpanKindPlugin), - } - m := p.buildReparentMap(spans) - if m == nil { - t.Fatal("expected non-nil map") - } - if got := m["log-pre"]; got != "" { - t.Errorf("root-level filtered span should resolve to empty string, got %q", got) - } - }) + // The filtered logging span is dropped; root + governance remain. + if len(spans) != 2 { + t.Fatalf("expected 2 exported spans (logging dropped), got %d", len(spans)) + } + byID := make(map[string]*Span, len(spans)) + for _, s := range spans { + byID[string(s.SpanId)] = s + } + if _, ok := byID[string(hexToBytes("bbbb", 8))]; ok { + t.Error("filtered logging span should not be exported") + } + gov, ok := byID[string(hexToBytes("cccc", 8))] + if !ok { + t.Fatal("governance span should be exported") + } + // governance's direct parent (logging) was filtered, so its parent must be rewritten to + // the nearest exported ancestor (root), not left dangling at the dropped logging span. + if !bytes.Equal(gov.ParentSpanId, hexToBytes("aaaa", 8)) { + t.Errorf("governance ParentSpanId = %x, want %x (reparented to root)", gov.ParentSpanId, hexToBytes("aaaa", 8)) + } } diff --git a/plugins/otel/main.go b/plugins/otel/main.go index 13d4722e483..d82a0576803 100644 --- a/plugins/otel/main.go +++ b/plugins/otel/main.go @@ -41,21 +41,19 @@ const ( ProtocolGRPC Protocol = "grpc" ) -// PluginSpanFilterMode controls whether the plugins list is an allowlist or denylist. -type PluginSpanFilterMode string +// PluginSpanFilter, its mode type, and the include/exclude constants are shared across +// all observability connectors and live in core/schemas. They are re-exported here as +// aliases so existing OTEL config parsing, tests, and the UI keep their import paths. +type ( + PluginSpanFilterMode = schemas.PluginSpanFilterMode + PluginSpanFilter = schemas.PluginSpanFilter +) const ( - PluginSpanFilterModeInclude PluginSpanFilterMode = "include" - PluginSpanFilterModeExclude PluginSpanFilterMode = "exclude" + PluginSpanFilterModeInclude = schemas.PluginSpanFilterModeInclude + PluginSpanFilterModeExclude = schemas.PluginSpanFilterModeExclude ) -// PluginSpanFilter configures which plugin spans are exported to the OTEL collector. -// Mode "include" exports only the listed plugins; mode "exclude" exports everything except them. -type PluginSpanFilter struct { - Mode PluginSpanFilterMode `json:"mode"` - Plugins []string `json:"plugins"` -} - // Profile is a single OTEL export target: a collector endpoint and an optional // metrics-push destination. A Config holds one or more profiles; each profile gets // its own trace client and (when enabled) metrics exporter at runtime. @@ -349,13 +347,8 @@ func Init(ctx context.Context, config *Config, _logger schemas.Logger, pricingMa if len(config.Profiles) == 0 { return nil, fmt.Errorf("at least one otel profile is required") } - if config.PluginSpanFilter != nil { - switch config.PluginSpanFilter.Mode { - case PluginSpanFilterModeInclude, PluginSpanFilterModeExclude: - default: - return nil, fmt.Errorf("plugin_span_filter.mode %q is invalid: must be %q or %q", - config.PluginSpanFilter.Mode, PluginSpanFilterModeInclude, PluginSpanFilterModeExclude) - } + if err := config.PluginSpanFilter.Validate(); err != nil { + return nil, err } // Loading attributes from environment attributesFromEnvironment := make([]*commonpb.KeyValue, 0) diff --git a/ui/app/workspace/plugins/sheets/pluginTracingSheet.tsx b/ui/app/workspace/observability/sheets/pluginTracingSheet.tsx similarity index 72% rename from ui/app/workspace/plugins/sheets/pluginTracingSheet.tsx rename to ui/app/workspace/observability/sheets/pluginTracingSheet.tsx index 44fd01d864b..46b3ac4a428 100644 --- a/ui/app/workspace/plugins/sheets/pluginTracingSheet.tsx +++ b/ui/app/workspace/observability/sheets/pluginTracingSheet.tsx @@ -11,6 +11,14 @@ import { toast } from "sonner"; interface PluginTracingSheetProps { open: boolean; onClose: () => void; + /** + * Backend plugin name of the observability connector whose span filter is being edited + * (e.g. "otel", "datadog", "bigquery"). The sheet reads/writes only this plugin's + * `plugin_span_filter`; the backend merges it over the rest of the connector config. + */ + pluginName: string; + /** Human-readable destination used in the copy, e.g. "the OTEL collector", "Datadog". */ + destination: string; } function resolveToggleState(filter: PluginSpanFilter | null | undefined, allPlugins: string[]): Record { @@ -43,7 +51,7 @@ function buildFilter(toggles: Record): PluginSpanFilter | null function PluginRow({ name, checked, onChange }: { name: string; checked: boolean; onChange: (v: boolean) => void }) { return (
- {name} + {name}
@@ -51,41 +59,49 @@ function PluginRow({ name, checked, onChange }: { name: string; checked: boolean ); } -export default function PluginTracingSheet({ open, onClose }: PluginTracingSheetProps) { +export default function PluginTracingSheet({ open, onClose, pluginName, destination }: PluginTracingSheetProps) { const { data: builtinPluginNames = [] } = useGetBuiltinPluginsQuery(); const { data: allPluginsData } = useGetPluginsQuery(); const customPluginNames = (allPluginsData ?? []).filter((p) => p.isCustom).map((p) => p.name); const allPlugins = [...builtinPluginNames, ...customPluginNames]; - const { data: otelPlugin } = useGetPluginQuery("otel"); + const { data: targetPlugin } = useGetPluginQuery(pluginName); const [updatePlugin, { isLoading }] = useUpdatePluginMutation(); const [toggles, setToggles] = useState>({}); const wasOpenRef = useRef(false); useEffect(() => { if (open && !wasOpenRef.current) { - if (!otelPlugin) return; // wait until persisted config is available - const filter = (otelPlugin.config?.plugin_span_filter as PluginSpanFilter | undefined) ?? null; + if (!targetPlugin) return; // wait until persisted config is available + const filter = (targetPlugin.config?.plugin_span_filter as PluginSpanFilter | undefined) ?? null; + if (filter?.mode === "include" && allPlugins.length === 0) return; setToggles(resolveToggleState(filter, allPlugins)); wasOpenRef.current = true; } if (!open) wasOpenRef.current = false; - }, [open, otelPlugin, allPlugins]); + }, [open, targetPlugin, allPlugins]); const setToggle = useCallback((name: string, value: boolean) => { setToggles((prev) => ({ ...prev, [name]: value })); }, []); const handleSave = useCallback(async () => { - if (!otelPlugin) { - toast.error("OTEL plugin not found"); + if (!wasOpenRef.current) { + // Toggles haven't been initialized from persisted config yet (e.g. the plugin list + // is still loading for an include-mode filter). Saving now would build an empty + // filter and wipe the stored plugin_span_filter, so block until init completes. + toast.error("Plugin list is still loading. Please wait before saving."); + return; + } + if (!targetPlugin) { + toast.error(`${destination} is not configured yet. Save its configuration before configuring plugin tracing.`); return; } const filter = buildFilter(toggles); try { await updatePlugin({ - name: "otel", + name: pluginName, data: { - enabled: otelPlugin.enabled, + enabled: targetPlugin.enabled, config: { plugin_span_filter: filter }, }, }).unwrap(); @@ -94,7 +110,7 @@ export default function PluginTracingSheet({ open, onClose }: PluginTracingSheet } catch (error) { toast.error(getErrorMessage(error)); } - }, [toggles, otelPlugin, updatePlugin, onClose]); + }, [toggles, targetPlugin, updatePlugin, onClose, pluginName, destination]); return ( @@ -102,8 +118,8 @@ export default function PluginTracingSheet({ open, onClose }: PluginTracingSheet Configure Plugin Tracing - Choose which plugin hook spans are exported to the OTEL collector. Disabling a plugin removes its spans from traces without - affecting execution. + Choose which plugin hook spans are exported to {destination}. Disabling a plugin removes its spans from traces without affecting + execution. @@ -111,7 +127,7 @@ export default function PluginTracingSheet({ open, onClose }: PluginTracingSheet
-

Built-in Plugins

+

Built-in Plugins

toggles[n] ?? true)} @@ -137,7 +153,7 @@ export default function PluginTracingSheet({ open, onClose }: PluginTracingSheet {customPluginNames.length > 0 && (
-

Custom Plugins

+

Custom Plugins

toggles[n] ?? true)} @@ -167,7 +183,8 @@ export default function PluginTracingSheet({ open, onClose }: PluginTracingSheet - If plugin_span_filter is set inside the OTEL plugin config in config.json, it takes precedence over these settings after restarting Bifrost. + If plugin_span_filter is set in the {pluginName}{" "} + plugin config in config.json, it takes precedence over these settings after restarting Bifrost. @@ -175,7 +192,13 @@ export default function PluginTracingSheet({ open, onClose }: PluginTracingSheet -
@@ -183,4 +206,4 @@ export default function PluginTracingSheet({ open, onClose }: PluginTracingSheet ); -} +} \ No newline at end of file diff --git a/ui/app/workspace/observability/views/plugins/otelView.tsx b/ui/app/workspace/observability/views/plugins/otelView.tsx index 0eb3b2293af..f34ba8cfe5c 100644 --- a/ui/app/workspace/observability/views/plugins/otelView.tsx +++ b/ui/app/workspace/observability/views/plugins/otelView.tsx @@ -1,9 +1,12 @@ +import { Button } from "@/components/ui/button"; import { getErrorMessage, useAppSelector, useUpdatePluginMutation } from "@/lib/store"; import { OtelFormSchema } from "@/lib/types/schemas"; import { toHeaderStringMap } from "@/lib/utils/envVarForm"; -import { useMemo } from "react"; +import { Activity } from "lucide-react"; +import { useMemo, useState } from "react"; import { toast } from "sonner"; import { OtelFormFragment } from "../../fragments/otelFormFragment"; +import PluginTracingSheet from "../../sheets/pluginTracingSheet"; interface OtelViewProps { onDelete?: () => void; @@ -14,6 +17,7 @@ export default function OtelView({ onDelete, isDeleting }: OtelViewProps) { const selectedPlugin = useAppSelector((state) => state.plugin.selectedPlugin); const currentConfig = useMemo(() => ({ config: selectedPlugin?.config, enabled: selectedPlugin?.enabled }), [selectedPlugin]); const [updatePlugin] = useUpdatePluginMutation(); + const [isTracingSheetOpen, setIsTracingSheetOpen] = useState(false); const handleOtelConfigSave = (config: OtelFormSchema): Promise => { // The backend stores headers as a plain "env.VAR"/literal string map, so flatten the @@ -48,8 +52,26 @@ export default function OtelView({ onDelete, isDeleting }: OtelViewProps) { return (
+
+ +
+ setIsTracingSheetOpen(false)} + pluginName="otel" + destination="the OTEL collector" + />
); } \ No newline at end of file diff --git a/ui/app/workspace/plugins/page.tsx b/ui/app/workspace/plugins/page.tsx index 8c41ba2b7a7..1bae4a33c07 100644 --- a/ui/app/workspace/plugins/page.tsx +++ b/ui/app/workspace/plugins/page.tsx @@ -2,12 +2,11 @@ import { Button } from "@/components/ui/button"; import { setSelectedPlugin, useAppDispatch, useAppSelector, useGetPluginsQuery } from "@/lib/store"; import { cn } from "@/lib/utils"; import { RbacOperation, RbacResource, useRbac } from "@enterprise/lib"; -import { Activity, ListOrdered, PlusIcon, Puzzle } from "lucide-react"; +import { ListOrdered, PlusIcon, Puzzle } from "lucide-react"; import { useQueryState } from "nuqs"; import { useEffect, useMemo, useState } from "react"; import AddNewPluginSheet from "./sheets/addNewPluginSheet"; import PluginSequenceSheet from "./sheets/pluginSequenceSheet"; -import PluginTracingSheet from "./sheets/pluginTracingSheet"; import { PluginsEmptyState } from "./views/pluginsEmptyState"; import PluginsView from "./views/pluginsView"; @@ -21,7 +20,6 @@ export default function PluginsPage() { const customPlugins = useMemo(() => plugins?.filter((plugin) => plugin.isCustom), [plugins]); const [isSheetOpen, setIsSheetOpen] = useState(false); const [isSequenceSheetOpen, setIsSequenceSheetOpen] = useState(false); - const [isTracingSheetOpen, setIsTracingSheetOpen] = useState(false); const handleAddNew = () => { setIsSheetOpen(true); @@ -51,12 +49,7 @@ export default function PluginsPage() { if (customPlugins?.length === 0 && !isLoading) { return (
- setIsTracingSheetOpen(true)} - canConfigureTracing={hasUpdatePluginAccess} - /> + - setIsTracingSheetOpen(false)} />
); } @@ -133,17 +125,6 @@ export default function PluginsPage() {
Edit Plugin Sequence
)} -
@@ -165,7 +146,6 @@ export default function PluginsPage() { }} /> setIsSequenceSheetOpen(false)} plugins={plugins ?? []} /> - setIsTracingSheetOpen(false)} />
); } \ No newline at end of file diff --git a/ui/app/workspace/plugins/views/pluginsEmptyState.tsx b/ui/app/workspace/plugins/views/pluginsEmptyState.tsx index b300d9a50be..fa2d1c15882 100644 --- a/ui/app/workspace/plugins/views/pluginsEmptyState.tsx +++ b/ui/app/workspace/plugins/views/pluginsEmptyState.tsx @@ -1,16 +1,14 @@ import { Button } from "@/components/ui/button"; -import { Activity, ArrowUpRight, Puzzle } from "lucide-react"; +import { ArrowUpRight, Puzzle } from "lucide-react"; const CUSTOM_PLUGINS_DOCS_URL = "https://docs.getbifrost.ai/plugins"; interface PluginsEmptyStateProps { onCreateClick: () => void; canCreate?: boolean; - onConfigureTracingClick?: () => void; - canConfigureTracing?: boolean; } -export function PluginsEmptyState({ onCreateClick, canCreate = true, onConfigureTracingClick, canConfigureTracing = true }: PluginsEmptyStateProps) { +export function PluginsEmptyState({ onCreateClick, canCreate = true }: PluginsEmptyStateProps) { return (
Read more - {onConfigureTracingClick && ( - - )}