From e8a8e48f213ff107e81fbb6005235a93239b8fd2 Mon Sep 17 00:00:00 2001 From: roroghost17 Date: Wed, 10 Jun 2026 02:54:29 +0530 Subject: [PATCH] fix: fixes plugin names sanitization for live loaded plugins for connectors --- core/schemas/span_filter.go | 8 ++- core/schemas/span_filter_test.go | 36 ++++++++++++ core/utils.go | 4 +- docs/enterprise/datadog-connector.mdx | 2 +- docs/features/observability/otel.mdx | 2 +- transports/bifrost-http/handlers/plugins.go | 12 +++- .../bifrost-http/handlers/plugins_test.go | 43 +++++++++++++++ transports/bifrost-http/lib/config.go | 24 ++++++++ transports/bifrost-http/server/server.go | 10 ++++ .../sheets/pluginTracingSheet.tsx | 55 +++++-------------- ui/lib/store/apis/pluginsApi.ts | 10 ++++ 11 files changed, 159 insertions(+), 47 deletions(-) diff --git a/core/schemas/span_filter.go b/core/schemas/span_filter.go index cd484229422..d3f92f9e3c4 100644 --- a/core/schemas/span_filter.go +++ b/core/schemas/span_filter.go @@ -40,9 +40,15 @@ func (f *PluginSpanFilter) Validate() error { } } +// SanitizePluginSpanName normalizes a plugin's name into the form embedded in its +// span names. +func SanitizePluginSpanName(name string) string { + return strings.ToLower(strings.ReplaceAll(name, " ", "-")) +} + // 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 +// posthook, prerequesthook, 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. diff --git a/core/schemas/span_filter_test.go b/core/schemas/span_filter_test.go index 093a90ee471..19e262b7270 100644 --- a/core/schemas/span_filter_test.go +++ b/core/schemas/span_filter_test.go @@ -52,6 +52,42 @@ func TestPluginNameFromSpan(t *testing.T) { } } +func TestSanitizePluginSpanName(t *testing.T) { + tests := []struct { + in string + want string + }{ + {"logging", "logging"}, + {"enterprise-prompts", "enterprise-prompts"}, + {"Model Catalog Resolver", "model-catalog-resolver"}, + {"UPPER", "upper"}, + {"", ""}, + } + for _, tt := range tests { + if got := SanitizePluginSpanName(tt.in); got != tt.want { + t.Errorf("SanitizePluginSpanName(%q) = %q, want %q", tt.in, got, tt.want) + } + } +} + +// TestSanitizedNameMatchesSpanExtraction locks the invariant that the name used to build a +// plugin span (SanitizePluginSpanName(GetName())) is exactly what PluginNameFromSpan extracts +// back out. If these ever diverge, the UI's filterable-plugin list stops matching real spans +// and span filtering silently no-ops — the bug this contract exists to prevent. +func TestSanitizedNameMatchesSpanExtraction(t *testing.T) { + pluginNames := []string{"logging", "enterprise-prompts", "adaptive-loadbalancer", "Has Spaces", "MixedCase"} + stages := []string{"prehook", "posthook", "prerequesthook", "mcp_prehook", "mcp_connect_posthook"} + for _, raw := range pluginNames { + sanitized := SanitizePluginSpanName(raw) + for _, stage := range stages { + spanName := "plugin." + sanitized + "." + stage + if got := PluginNameFromSpan(pluginSpan("1", "", spanName)); got != sanitized { + t.Errorf("PluginNameFromSpan(%q) = %q, want %q", spanName, got, sanitized) + } + } + } +} + func TestPluginSpanFilter_ShouldExportSpan(t *testing.T) { llm := &Span{SpanID: "llm", Name: "llm.call", Kind: SpanKindLLMCall} logging := pluginSpan("p1", "", "plugin.logging.prehook") diff --git a/core/utils.go b/core/utils.go index 34b8739689e..b978b223c4f 100644 --- a/core/utils.go +++ b/core/utils.go @@ -557,9 +557,9 @@ func ValidateExternalURL(urlStr string, allowPrivateNetwork bool) error { return nil } -// sanitizeSpanName sanitizes a span name to remove capital letters and spaces to make it a valid span name +// sanitizeSpanName sanitizes a span name to remove capital letters and spaces to make it a valid span name. func sanitizeSpanName(name string) string { - return strings.ToLower(strings.ReplaceAll(name, " ", "-")) + return schemas.SanitizePluginSpanName(name) } // IsCodemodeTool returns true if the given tool name is a codemode tool. diff --git a/docs/enterprise/datadog-connector.mdx b/docs/enterprise/datadog-connector.mdx index fd51aa02d92..f5ee90c4d93 100644 --- a/docs/enterprise/datadog-connector.mdx +++ b/docs/enterprise/datadog-connector.mdx @@ -447,7 +447,7 @@ By default every plugin's pre- and post-hook execution generates a span, which c | `exclude` | Export spans for all plugins **except** those listed | | `include` | Export spans **only** for the listed plugins | -**Built-in plugin names** (for reference): `telemetry`, `prompts`, `logging`, `governance`, `otel`, `semantic_cache`, `compat`, `maxim`. +**Plugin names:** list each plugin using the exact name shown for it in the **Configure Plugin Tracing** sheet — this is the same name that appears in the span (`plugin..`), and it is what the filter matches against. Note that some plugins are registered under a different name than their config key: the enterprise prompts and governance plugins appear as `enterprise-prompts` and `enterprise-governance` (not `prompts`/`governance`). Common names include `telemetry`, `logging`, `otel`, `semantic_cache`, `compat`, `maxim`, `enterprise-prompts`, `enterprise-governance`, `datadog`, `bigquery`, `guardrails`, `adaptive-loadbalancer`, and `model-catalog-resolver`. The exact set depends on which plugins are loaded in your deployment. When a plugin span is filtered out, its children are automatically re-parented to the nearest exported ancestor so the trace hierarchy stays connected. The filter applies to APM trace spans only; it does not change DogStatsD metrics, which are never derived from plugin spans. diff --git a/docs/features/observability/otel.mdx b/docs/features/observability/otel.mdx index 87cabaf9e11..a5992962d88 100644 --- a/docs/features/observability/otel.mdx +++ b/docs/features/observability/otel.mdx @@ -1065,7 +1065,7 @@ By default every plugin's pre- and post-hook execution generates a span, which c | `exclude` | Export spans for all plugins **except** those listed | | `include` | Export spans **only** for the listed plugins | -**Built-in plugin names** (for reference): `telemetry`, `prompts`, `logging`, `governance`, `otel`, `semantic_cache`, `compat`, `maxim`. +**Plugin names:** list each plugin using the exact name shown for it in the **Configure Plugin Tracing** sheet — this is the same name that appears in the span (`plugin..`), and it is what the filter matches against. The built-in OSS plugins are `telemetry`, `prompts`, `logging`, `governance`, `otel`, `semantic_cache`, `compat`, and `maxim`. In enterprise deployments some plugins are registered under a different name than their config key — for example the prompts and governance plugins appear as `enterprise-prompts` and `enterprise-governance` — so always copy the name from the tracing sheet rather than assuming the config key. When a plugin span is filtered out, its children are automatically re-parented to the nearest exported ancestor so the trace hierarchy stays connected. diff --git a/transports/bifrost-http/handlers/plugins.go b/transports/bifrost-http/handlers/plugins.go index 8018947db3d..b74399600e4 100644 --- a/transports/bifrost-http/handlers/plugins.go +++ b/transports/bifrost-http/handlers/plugins.go @@ -18,6 +18,7 @@ import ( type PluginsLoader interface { GetPluginStatus(ctx context.Context) map[string]schemas.PluginStatus + GetLoadedPluginNames() []string ReloadPlugin(ctx context.Context, name string, path *string, pluginConfig any, placement *schemas.PluginPlacement, order *int) error RemovePlugin(ctx context.Context, name string) error // NormalizePluginConfig converts a raw config map to DB-storage format using @@ -95,6 +96,7 @@ func (h *PluginsHandler) expandPluginConfigForAPI(name string, config map[string func (h *PluginsHandler) RegisterRoutes(r *router.Router, middlewares ...schemas.BifrostHTTPMiddleware) { r.GET("/api/plugins", lib.ChainMiddlewares(h.getPlugins, middlewares...)) r.GET("/api/plugins/builtins", lib.ChainMiddlewares(h.getBuiltinPlugins, middlewares...)) + r.GET("/api/plugins/loaded", lib.ChainMiddlewares(h.getLoadedPlugins, middlewares...)) r.GET("/api/plugins/{name}", lib.ChainMiddlewares(h.getPlugin, middlewares...)) r.POST("/api/plugins", lib.ChainMiddlewares(h.createPlugin, middlewares...)) r.PUT("/api/plugins/{name}", lib.ChainMiddlewares(h.updatePlugin, middlewares...)) @@ -159,13 +161,21 @@ func (h *PluginsHandler) buildPluginResponseWithStatuses(plugin *configstoreTabl } } -// getBuiltinPlugins returns the canonical list of built-in plugin names +// getBuiltinPlugins returns the canonical list of built-in plugin names. func (h *PluginsHandler) getBuiltinPlugins(ctx *fasthttp.RequestCtx) { SendJSON(ctx, map[string]any{ "plugins": lib.GetBuiltinPluginNames(), }) } +// getLoadedPlugins returns the names of all plugins currently loaded at runtime, whose +// spans an observability connector can filter. +func (h *PluginsHandler) getLoadedPlugins(ctx *fasthttp.RequestCtx) { + SendJSON(ctx, map[string]any{ + "plugins": h.pluginsLoader.GetLoadedPluginNames(), + }) +} + // getPlugins gets all plugins func (h *PluginsHandler) getPlugins(ctx *fasthttp.RequestCtx) { if h.configStore == nil { diff --git a/transports/bifrost-http/handlers/plugins_test.go b/transports/bifrost-http/handlers/plugins_test.go index d268fa74af9..e9d65808445 100644 --- a/transports/bifrost-http/handlers/plugins_test.go +++ b/transports/bifrost-http/handlers/plugins_test.go @@ -51,6 +51,7 @@ func (noopPluginsLoader) RemovePlugin(_ context.Context, _ string) error { retur func (noopPluginsLoader) GetPluginStatus(_ context.Context) map[string]schemas.PluginStatus { return nil } +func (noopPluginsLoader) GetLoadedPluginNames() []string { return nil } func (noopPluginsLoader) NormalizePluginConfig(_ string, _ map[string]any) (map[string]any, error) { return nil, nil } @@ -163,3 +164,45 @@ func TestUpdatePlugin_ConfigMerge_NewPlugin(t *testing.T) { t.Fatalf("expected 200, got %d: %s", ctx.Response.StatusCode(), ctx.Response.Body()) } } + +// namedPluginsLoader is a noopPluginsLoader that returns a fixed set of loaded +// plugin names, used to assert the getLoadedPlugins response contract. +type namedPluginsLoader struct { + noopPluginsLoader + names []string +} + +func (l namedPluginsLoader) GetLoadedPluginNames() []string { return l.names } + +// TestGetLoadedPlugins verifies that getLoadedPlugins returns the loader's plugin +// names under the "plugins" JSON key, locking the response shape the UI depends on. +func TestGetLoadedPlugins(t *testing.T) { + want := []string{"logging", "telemetry", "enterprise-governance"} + h := &PluginsHandler{ + pluginsLoader: namedPluginsLoader{names: want}, + configStore: nil, + } + + ctx := &fasthttp.RequestCtx{} + ctx.Request.Header.SetMethod("GET") + h.getLoadedPlugins(ctx) + + if ctx.Response.StatusCode() != 200 { + t.Fatalf("expected 200, got %d: %s", ctx.Response.StatusCode(), ctx.Response.Body()) + } + + var response struct { + Plugins []string `json:"plugins"` + } + if err := json.Unmarshal(ctx.Response.Body(), &response); err != nil { + t.Fatalf("unmarshal response: %v", err) + } + if len(response.Plugins) != len(want) { + t.Fatalf("expected %d plugins, got %d: %v", len(want), len(response.Plugins), response.Plugins) + } + for i, name := range want { + if response.Plugins[i] != name { + t.Errorf("plugins[%d] = %q, want %q", i, response.Plugins[i], name) + } + } +} diff --git a/transports/bifrost-http/lib/config.go b/transports/bifrost-http/lib/config.go index 1459dcb2b54..ba141725a1e 100644 --- a/transports/bifrost-http/lib/config.go +++ b/transports/bifrost-http/lib/config.go @@ -4415,6 +4415,30 @@ func (c *Config) GetLoadedLLMPlugins() []schemas.LLMPlugin { return nil } +// GetLoadedPluginNames returns the sanitized names of every currently loaded plugin, +// matching the names embedded in their trace span names. +func (c *Config) GetLoadedPluginNames() []string { + plugins := c.BasePlugins.Load() + if plugins == nil { + return nil + } + seen := make(map[string]struct{}, len(*plugins)) + names := make([]string, 0, len(*plugins)) + for _, p := range *plugins { + name := schemas.SanitizePluginSpanName(p.GetName()) + if name == "" { + continue + } + if _, ok := seen[name]; ok { + continue + } + seen[name] = struct{}{} + names = append(names, name) + } + slices.Sort(names) + return names +} + // pluginChunkInterceptor implements StreamChunkInterceptor by calling plugin hooks type pluginChunkInterceptor struct { plugins []schemas.HTTPTransportPlugin diff --git a/transports/bifrost-http/server/server.go b/transports/bifrost-http/server/server.go index d78836649c3..bfca02695a1 100644 --- a/transports/bifrost-http/server/server.go +++ b/transports/bifrost-http/server/server.go @@ -64,6 +64,7 @@ type ServerCallbacks interface { ReloadPlugin(ctx context.Context, name string, path *string, pluginConfig any, placement *schemas.PluginPlacement, order *int) error RemovePlugin(ctx context.Context, name string) error GetPluginStatus(ctx context.Context) map[string]schemas.PluginStatus + GetLoadedPluginNames() []string NormalizePluginConfig(name string, config map[string]any) (map[string]any, error) ExpandPluginConfigForAPI(name string, config map[string]any) (map[string]any, error) // Auth related callbacks @@ -1158,6 +1159,15 @@ func (s *BifrostHTTPServer) GetPluginStatus(ctx context.Context) map[string]sche return s.Config.GetPluginStatus() } +// GetLoadedPluginNames returns the sanitized names of all currently loaded plugins, +// matching the names embedded in their trace span names. +func (s *BifrostHTTPServer) GetLoadedPluginNames() []string { + if s.Config == nil { + return []string{} + } + return s.Config.GetLoadedPluginNames() +} + // NormalizePluginConfig implements handlers.PluginsLoader. It looks up the plugin // by name in the ConfigMarshallers cache and calls MarshalConfigForStorage if found. // Returns nil, nil when the plugin is not loaded or does not implement ConfigMarshallerPlugin. diff --git a/ui/app/workspace/observability/sheets/pluginTracingSheet.tsx b/ui/app/workspace/observability/sheets/pluginTracingSheet.tsx index 46b3ac4a428..cae6d9b4837 100644 --- a/ui/app/workspace/observability/sheets/pluginTracingSheet.tsx +++ b/ui/app/workspace/observability/sheets/pluginTracingSheet.tsx @@ -3,7 +3,7 @@ import { Button } from "@/components/ui/button"; import { Sheet, SheetContent, SheetDescription, SheetHeader, SheetTitle } from "@/components/ui/sheet"; import { Switch } from "@/components/ui/switch"; import { TriStateCheckbox } from "@/components/ui/tristateCheckbox"; -import { getErrorMessage, useGetBuiltinPluginsQuery, useGetPluginQuery, useGetPluginsQuery, useUpdatePluginMutation } from "@/lib/store"; +import { getErrorMessage, useGetLoadedPluginsQuery, useGetPluginQuery, useUpdatePluginMutation } from "@/lib/store"; import { PluginSpanFilter } from "@/lib/types/config"; import { useCallback, useEffect, useRef, useState } from "react"; import { toast } from "sonner"; @@ -60,10 +60,10 @@ function PluginRow({ name, checked, onChange }: { name: string; checked: boolean } 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]; + // All currently loaded plugins (built-in, enterprise, custom, and auto-loaded) that can + // emit spans, named to match the connector's span filter. One flat list — the backend + // already returns the complete set, so there's no built-in/custom split to maintain. + const { data: allPlugins = [], isLoading: isLoadingLoadedPlugins } = useGetLoadedPluginsQuery(); const { data: targetPlugin } = useGetPluginQuery(pluginName); const [updatePlugin, { isLoading }] = useUpdatePluginMutation(); const [toggles, setToggles] = useState>({}); @@ -73,12 +73,12 @@ export default function PluginTracingSheet({ open, onClose, pluginName, destinat if (open && !wasOpenRef.current) { 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; + if (isLoadingLoadedPlugins || allPlugins.length === 0) return; setToggles(resolveToggleState(filter, allPlugins)); wasOpenRef.current = true; } if (!open) wasOpenRef.current = false; - }, [open, targetPlugin, allPlugins]); + }, [open, targetPlugin, allPlugins, isLoadingLoadedPlugins]); const setToggle = useCallback((name: string, value: boolean) => { setToggles((prev) => ({ ...prev, [name]: value })); @@ -127,55 +127,28 @@ export default function PluginTracingSheet({ open, onClose, pluginName, destinat
-

Built-in Plugins

+

Plugins

toggles[n] ?? true)} + allIds={allPlugins} + selectedIds={allPlugins.filter((n) => toggles[n] ?? true)} onChange={(next) => { const nextSet = new Set(next); setToggles((prev) => { const updated = { ...prev }; - for (const n of builtinPluginNames) updated[n] = nextSet.has(n); + for (const n of allPlugins) updated[n] = nextSet.has(n); return updated; }); }} - ariaLabel="Toggle all built-in plugin tracing" - data-testid="plugin-tracing-select-all-builtins" + ariaLabel="Toggle all plugin tracing" + data-testid="plugin-tracing-select-all" />
- {builtinPluginNames.map((name) => ( + {allPlugins.map((name) => ( setToggle(name, v)} /> ))}
- - {customPluginNames.length > 0 && ( -
-
-

Custom Plugins

- toggles[n] ?? true)} - onChange={(next) => { - const nextSet = new Set(next); - setToggles((prev) => { - const updated = { ...prev }; - for (const n of customPluginNames) updated[n] = nextSet.has(n); - return updated; - }); - }} - ariaLabel="Toggle all custom plugin tracing" - data-testid="plugin-tracing-select-all-custom" - /> -
-
- {customPluginNames.map((name) => ( - setToggle(name, v)} /> - ))} -
-
- )}
diff --git a/ui/lib/store/apis/pluginsApi.ts b/ui/lib/store/apis/pluginsApi.ts index 055d29d5fad..46c0f629017 100644 --- a/ui/lib/store/apis/pluginsApi.ts +++ b/ui/lib/store/apis/pluginsApi.ts @@ -10,6 +10,15 @@ export const pluginsApi = baseApi.injectEndpoints({ transformResponse: (response: { plugins: string[] }) => response.plugins || [], }), + // Get the names of all currently loaded plugins (sanitized to match the names + // embedded in their trace span names). Used by the plugin tracing sheet so it + // lists every plugin that actually emits spans, including enterprise plugins. + getLoadedPlugins: builder.query({ + query: () => "/plugins/loaded", + providesTags: ["Plugins"], + transformResponse: (response: { plugins: string[] }) => response.plugins || [], + }), + // Get all plugins getPlugins: builder.query({ query: () => "/plugins", @@ -97,6 +106,7 @@ export const pluginsApi = baseApi.injectEndpoints({ export const { useGetBuiltinPluginsQuery, + useGetLoadedPluginsQuery, useGetPluginsQuery, useGetPluginQuery, useCreatePluginMutation,