Skip to content

Commit 2556d72

Browse files
Add QueryWithFeatureChecker to minimize feature flag service calls
Adds two new methods to ToolIndex: - UniqueFeatureFlags(): Returns all unique feature flags used by tools - QueryWithFeatureChecker(): Queries with automatic flag checking Key optimization: Instead of checking the same feature flag N times for N tools that use it, each unique flag is checked exactly once. For example, if 50 tools use 3 unique flags, this reduces feature flag service calls from O(50) to O(3). Test demonstrates: 50 tools using 3 flags results in exactly 3 checks.
1 parent fb79b44 commit 2556d72

File tree

2 files changed

+197
-0
lines changed

2 files changed

+197
-0
lines changed

pkg/inventory/tool_index.go

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,3 +280,84 @@ func (idx *ToolIndex) ToolsetIDs() []ToolsetID {
280280
sort.Slice(ids, func(i, j int) bool { return ids[i] < ids[j] })
281281
return ids
282282
}
283+
284+
// UniqueFeatureFlags returns all unique feature flags referenced by tools in the index.
285+
// This allows callers to check only the flags that matter, once per request.
286+
func (idx *ToolIndex) UniqueFeatureFlags() []string {
287+
seen := make(map[string]bool)
288+
for flag := range idx.requiresFeature {
289+
seen[flag] = true
290+
}
291+
for flag := range idx.disabledByFeature {
292+
seen[flag] = true
293+
}
294+
295+
flags := make([]string, 0, len(seen))
296+
for flag := range seen {
297+
flags = append(flags, flag)
298+
}
299+
sort.Strings(flags)
300+
return flags
301+
}
302+
303+
// QueryWithFeatureChecker performs a query, automatically checking feature flags.
304+
// Each unique flag is checked exactly once via the checker function.
305+
// This minimizes feature flag service calls from O(tools) to O(unique flags).
306+
func (idx *ToolIndex) QueryWithFeatureChecker(ctx context.Context, cfg QueryConfigWithChecker) QueryResult {
307+
// Check each unique feature flag exactly once
308+
var enabledFeatures, disabledFeatures []string
309+
310+
for flag := range idx.requiresFeature {
311+
enabled, err := cfg.FeatureChecker(ctx, flag)
312+
if err != nil || !enabled {
313+
disabledFeatures = append(disabledFeatures, flag)
314+
} else {
315+
enabledFeatures = append(enabledFeatures, flag)
316+
}
317+
}
318+
319+
for flag := range idx.disabledByFeature {
320+
// Only check if we haven't already
321+
alreadyChecked := false
322+
for _, f := range enabledFeatures {
323+
if f == flag {
324+
alreadyChecked = true
325+
break
326+
}
327+
}
328+
for _, f := range disabledFeatures {
329+
if f == flag {
330+
alreadyChecked = true
331+
break
332+
}
333+
}
334+
if !alreadyChecked {
335+
enabled, err := cfg.FeatureChecker(ctx, flag)
336+
if err != nil || !enabled {
337+
disabledFeatures = append(disabledFeatures, flag)
338+
} else {
339+
enabledFeatures = append(enabledFeatures, flag)
340+
}
341+
}
342+
}
343+
344+
// Delegate to the standard Query with resolved flags
345+
return idx.Query(QueryConfig{
346+
AllToolsets: cfg.AllToolsets,
347+
EnabledToolsets: cfg.EnabledToolsets,
348+
AdditionalTools: cfg.AdditionalTools,
349+
ReadOnly: cfg.ReadOnly,
350+
EnabledFeatures: enabledFeatures,
351+
DisabledFeatures: disabledFeatures,
352+
})
353+
}
354+
355+
// QueryConfigWithChecker is like QueryConfig but uses a checker function
356+
// instead of pre-resolved feature flag lists.
357+
type QueryConfigWithChecker struct {
358+
AllToolsets bool
359+
EnabledToolsets []ToolsetID
360+
AdditionalTools []string
361+
ReadOnly bool
362+
FeatureChecker FeatureFlagChecker // Reuses the existing FeatureFlagChecker type from filters.go
363+
}

pkg/inventory/tool_index_test.go

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,122 @@ func TestToolIndex_ToolsetBitmap(t *testing.T) {
491491
assert.True(t, bmX.IsEmpty())
492492
}
493493

494+
func TestToolIndex_UniqueFeatureFlags(t *testing.T) {
495+
t.Parallel()
496+
497+
testTools := []ServerTool{
498+
mockServerToolInToolset("basic_tool", "tools", true),
499+
mockServerToolWithFeatureFlag("advanced_tool", "tools", "feature_a", ""),
500+
mockServerToolWithFeatureFlag("experimental_tool", "tools", "feature_b", ""),
501+
mockServerToolWithFeatureFlag("legacy_tool", "tools", "", "feature_c"),
502+
mockServerToolWithFeatureFlag("complex_tool", "tools", "feature_a", "feature_d"), // reuses feature_a
503+
}
504+
505+
index := BuildToolIndex(testTools)
506+
507+
flags := index.UniqueFeatureFlags()
508+
509+
// Should have 4 unique flags: feature_a, feature_b, feature_c, feature_d
510+
assert.Len(t, flags, 4)
511+
assert.Contains(t, flags, "feature_a")
512+
assert.Contains(t, flags, "feature_b")
513+
assert.Contains(t, flags, "feature_c")
514+
assert.Contains(t, flags, "feature_d")
515+
}
516+
517+
func TestToolIndex_QueryWithFeatureChecker(t *testing.T) {
518+
t.Parallel()
519+
520+
testTools := []ServerTool{
521+
mockServerToolInToolset("basic_tool", "tools", true),
522+
mockServerToolWithFeatureFlag("needs_feature_a", "tools", "feature_a", ""),
523+
mockServerToolWithFeatureFlag("needs_feature_b", "tools", "feature_b", ""),
524+
mockServerToolWithFeatureFlag("disabled_by_feature_c", "tools", "", "feature_c"),
525+
}
526+
527+
index := BuildToolIndex(testTools)
528+
529+
// Track which flags were checked
530+
checkedFlags := make(map[string]int)
531+
532+
checker := func(_ context.Context, flag string) (bool, error) {
533+
checkedFlags[flag]++
534+
// Enable feature_a, disable feature_b, enable feature_c
535+
switch flag {
536+
case "feature_a":
537+
return true, nil
538+
case "feature_b":
539+
return false, nil
540+
case "feature_c":
541+
return true, nil // This will disable "disabled_by_feature_c"
542+
default:
543+
return false, nil
544+
}
545+
}
546+
547+
ctx := context.Background()
548+
result := index.QueryWithFeatureChecker(ctx, QueryConfigWithChecker{
549+
EnabledToolsets: []ToolsetID{"tools"},
550+
ReadOnly: false,
551+
FeatureChecker: checker,
552+
})
553+
554+
// Each flag should be checked exactly once
555+
assert.Equal(t, 1, checkedFlags["feature_a"], "feature_a should be checked once")
556+
assert.Equal(t, 1, checkedFlags["feature_b"], "feature_b should be checked once")
557+
assert.Equal(t, 1, checkedFlags["feature_c"], "feature_c should be checked once")
558+
559+
// Materialize and verify results
560+
tools := index.Materialize(ctx, result)
561+
names := make([]string, len(tools))
562+
for i, tool := range tools {
563+
names[i] = tool.Tool.Name
564+
}
565+
566+
// basic_tool: no flags, included
567+
// needs_feature_a: feature_a enabled, included
568+
// needs_feature_b: feature_b disabled, excluded
569+
// disabled_by_feature_c: feature_c enabled, excluded
570+
assert.Len(t, tools, 2)
571+
assert.Contains(t, names, "basic_tool")
572+
assert.Contains(t, names, "needs_feature_a")
573+
assert.NotContains(t, names, "needs_feature_b")
574+
assert.NotContains(t, names, "disabled_by_feature_c")
575+
}
576+
577+
func TestToolIndex_QueryWithFeatureChecker_MinimizesChecks(t *testing.T) {
578+
t.Parallel()
579+
580+
// Create 50 tools that all use the same 3 feature flags
581+
testTools := make([]ServerTool, 50)
582+
for i := 0; i < 50; i++ {
583+
flag := []string{"flag_a", "flag_b", "flag_c"}[i%3]
584+
testTools[i] = mockServerToolWithFeatureFlag(
585+
"tool_"+string(rune('a'+i%26)),
586+
"tools",
587+
flag, // All tools require one of 3 flags
588+
"",
589+
)
590+
}
591+
592+
index := BuildToolIndex(testTools)
593+
594+
checkCount := 0
595+
checker := func(_ context.Context, _ string) (bool, error) {
596+
checkCount++
597+
return true, nil
598+
}
599+
600+
ctx := context.Background()
601+
_ = index.QueryWithFeatureChecker(ctx, QueryConfigWithChecker{
602+
EnabledToolsets: []ToolsetID{"tools"},
603+
FeatureChecker: checker,
604+
})
605+
606+
// Should only check 3 unique flags, not 50 tools
607+
assert.Equal(t, 3, checkCount, "Should check each unique flag exactly once")
608+
}
609+
494610
func BenchmarkBuildToolIndex_130Tools(b *testing.B) {
495611
// Create realistic toolset distribution
496612
toolsets := []ToolsetID{"repos", "issues", "pull_requests", "users", "actions", "code_security", "projects", "notifications", "discussions", "experiments"}

0 commit comments

Comments
 (0)