Skip to content

Commit 4fa7057

Browse files
committed
migrate plugin keys to explicit scopes
Migrates all EPP in-tree dynamic attribute keys (predicted-latency, prefix-cache, concurrency, multimodal, and tokenizer) to explicitly declare RequestScope and EndpointScope boundaries. Strictly demotes legacy UnspecifiedScope out-of-tree plugins to Post-Admission phase at startup, ogging actionable warning logs and utilizing DAG validations to fail-fast on pre-admission legacy dependencies. Signed-off-by: Luke Van Drie <lukevandrie@google.com>
1 parent 60f4c2b commit 4fa7057

8 files changed

Lines changed: 218 additions & 39 deletions

File tree

pkg/epp/datalayer/data_graph.go

Lines changed: 110 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ func ValidateAndOrderDataDependencies(plugins []plugin.Plugin) ([]string, error)
4545
// CompilePipeline compiles the active plugins into two topologically sorted slices:
4646
// Pre-Admission and Post-Admission, applying pruning and scope validations.
4747
func CompilePipeline(plugins []plugin.Plugin) ([]string, []string, error) {
48+
coercedScopes := auditAndCoerceScopes(plugins)
49+
4850
pluginMap := make(map[string]plugin.Plugin)
4951
for _, p := range plugins {
5052
pluginMap[p.TypedName().String()] = p
@@ -55,19 +57,19 @@ func CompilePipeline(plugins []plugin.Plugin) ([]string, []string, error) {
5557
return nil, nil, err
5658
}
5759

58-
dag, err := buildDAG(producers, consumers)
60+
dag, err := buildDAG(producers, consumers, coercedScopes)
5961
if err != nil {
6062
return nil, nil, err
6163
}
6264

6365
V_kept, filteredDag := pruneUnusedProducers(pluginMap, dag)
6466

65-
preAdmission, postAdmission, err := slicePipelineByAdmissionGating(pluginMap, V_kept, filteredDag)
67+
preAdmission, postAdmission, err := slicePipelineByAdmissionGating(pluginMap, V_kept, filteredDag, coercedScopes)
6668
if err != nil {
6769
return nil, nil, err
6870
}
6971

70-
if err := validatePreAdmissionScopes(pluginMap, preAdmission); err != nil {
72+
if err := validatePreAdmissionScopes(pluginMap, preAdmission, coercedScopes); err != nil {
7173
return nil, nil, err
7274
}
7375

@@ -206,6 +208,7 @@ func slicePipelineByAdmissionGating(
206208
pluginMap map[string]plugin.Plugin,
207209
V_kept map[string]bool,
208210
filteredDag map[string][]string,
211+
coercedScopes map[string]plugin.DataScope,
209212
) (map[string]bool, map[string]bool, error) {
210213
preAdmissionRoots := make(map[string]bool)
211214
for name := range V_kept {
@@ -218,10 +221,12 @@ func slicePipelineByAdmissionGating(
218221
} else if _, ok := p.(fwkfc.OrderingPolicy); ok {
219222
isRoot = true
220223
} else if eagerP, ok := p.(plugin.EagerProducerPlugin); ok && eagerP.Eager() {
221-
for key := range eagerP.Produces() {
222-
if key.Scope() == plugin.RequestScope {
223-
isRoot = true
224-
break
224+
if eagerP.Produces() != nil {
225+
for key := range eagerP.Produces() {
226+
if resolveScope(key, coercedScopes) == plugin.RequestScope {
227+
isRoot = true
228+
break
229+
}
225230
}
226231
}
227232
}
@@ -259,22 +264,22 @@ func slicePipelineByAdmissionGating(
259264

260265
// validatePreAdmissionScopes validates that Pre-Admission plugins do not consume or produce EndpointScope or
261266
// UnspecifiedScope keys.
262-
func validatePreAdmissionScopes(pluginMap map[string]plugin.Plugin, preAdmission map[string]bool) error {
267+
func validatePreAdmissionScopes(pluginMap map[string]plugin.Plugin, preAdmission map[string]bool, coercedScopes map[string]plugin.DataScope) error {
263268
for name := range preAdmission {
264269
p := pluginMap[name]
265270
if consumer, ok := p.(plugin.ConsumerPlugin); ok {
266271
for key := range consumer.Consumes() {
267-
if key.Scope() == plugin.EndpointScope || key.Scope() == plugin.UnspecifiedScope {
272+
if resolveScope(key, coercedScopes) == plugin.EndpointScope {
268273
return fmt.Errorf("scope mismatch: plugin %q in Pre-Admission consumes key %q with invalid scope %q",
269-
name, key.String(), key.Scope())
274+
name, key.String(), resolveScope(key, coercedScopes))
270275
}
271276
}
272277
}
273278
if producer, ok := p.(plugin.ProducerPlugin); ok {
274279
for key := range producer.Produces() {
275-
if key.Scope() == plugin.EndpointScope || key.Scope() == plugin.UnspecifiedScope {
280+
if resolveScope(key, coercedScopes) == plugin.EndpointScope {
276281
return fmt.Errorf("scope mismatch: plugin %q in Pre-Admission produces key %q with invalid scope %q",
277-
name, key.String(), key.Scope())
282+
name, key.String(), resolveScope(key, coercedScopes))
278283
}
279284
}
280285
}
@@ -355,7 +360,7 @@ const (
355360
DefaultLayer = -1
356361
)
357362

358-
func pluginToLayerExecutionOrder(p plugin.Plugin) int {
363+
func pluginToLayerExecutionOrder(p plugin.Plugin, coercedScopes map[string]plugin.DataScope) int {
359364
// PreAdmitter
360365
if _, ok := p.(fwkrc.PreAdmitter); ok {
361366
return PreAdmissionLayer
@@ -376,7 +381,7 @@ func pluginToLayerExecutionOrder(p plugin.Plugin) int {
376381
if producer.Produces() != nil && len(producer.Produces()) > 0 {
377382
hasProduces = true
378383
for key := range producer.Produces() {
379-
if key.Scope() != plugin.RequestScope {
384+
if resolveScope(key, coercedScopes) != plugin.RequestScope {
380385
isRequestScoped = false
381386
break
382387
}
@@ -418,7 +423,7 @@ func pluginToLayerExecutionOrder(p plugin.Plugin) int {
418423

419424
// buildDAG builds a dependency graph among data preparation plugins based on their
420425
// produced and consumed data keys.
421-
func buildDAG(producers map[string]plugin.ProducerPlugin, consumers map[string]plugin.ConsumerPlugin) (map[string][]string, error) {
426+
func buildDAG(producers map[string]plugin.ProducerPlugin, consumers map[string]plugin.ConsumerPlugin, coercedScopes map[string]plugin.DataScope) (map[string][]string, error) {
422427
dag := make(map[string][]string)
423428
// Create dependency graph as a DAG.
424429
for _, producer := range producers {
@@ -432,25 +437,39 @@ func buildDAG(producers map[string]plugin.ProducerPlugin, consumers map[string]p
432437
if pName == cName {
433438
continue
434439
}
435-
if producer.Produces() != nil && consumer.Consumes() != nil {
436-
hasDependency := false
437-
for producedKey, producedData := range producer.Produces() {
438-
if consumedData, ok := consumer.Consumes()[producedKey]; ok {
439-
// Check types are same.
440-
if reflect.TypeOf(producedData) != reflect.TypeOf(consumedData) {
441-
return nil, fmt.Errorf("data type mismatch between produced and consumed data for key: %s", producedKey.String())
442-
}
443-
if pluginToLayerExecutionOrder(producer) > pluginToLayerExecutionOrder(consumer) {
444-
return nil, fmt.Errorf("invalid plugin layer execution order: producer %s needs to be executed before consumer %s", pName, cName)
440+
hasDependency := false
441+
if producer.Produces() != nil {
442+
// Required consumes check
443+
if consumer.Consumes() != nil {
444+
for producedKey, producedData := range producer.Produces() {
445+
if consumedData, ok := consumer.Consumes()[producedKey]; ok {
446+
// Check types are same.
447+
if reflect.TypeOf(producedData) != reflect.TypeOf(consumedData) {
448+
return nil, fmt.Errorf("data type mismatch between produced and consumed data for key: %s", producedKey.String())
449+
}
450+
if pluginToLayerExecutionOrder(producer, coercedScopes) > pluginToLayerExecutionOrder(consumer, coercedScopes) {
451+
return nil, fmt.Errorf("invalid plugin layer execution order: producer %s needs to be executed before consumer %s", pName, cName)
452+
}
453+
hasDependency = true
445454
}
446-
hasDependency = true
447455
}
448456
}
449-
if hasDependency {
450-
// Consumer depends on producer, so add an edge from consumer to producer.
451-
dag[cName] = append(dag[cName], pName)
457+
// Optional consumes check
458+
if optConsumer, ok := consumer.(plugin.OptionalConsumerPlugin); ok && optConsumer.OptionalConsumes() != nil {
459+
for _, producedKey := range optConsumer.OptionalConsumes() {
460+
if _, ok := producer.Produces()[producedKey]; ok {
461+
if pluginToLayerExecutionOrder(producer, coercedScopes) > pluginToLayerExecutionOrder(consumer, coercedScopes) {
462+
return nil, fmt.Errorf("invalid plugin layer execution order: producer %s needs to be executed before consumer %s (via optional dependency)", pName, cName)
463+
}
464+
hasDependency = true
465+
}
466+
}
452467
}
453468
}
469+
if hasDependency {
470+
// Consumer depends on producer, so add an edge from consumer to producer.
471+
dag[cName] = append(dag[cName], pName)
472+
}
454473
}
455474
}
456475
return dag, nil
@@ -563,3 +582,65 @@ func findAndFormatCycle(graph map[string][]string, inDegree map[string]int) stri
563582
}
564583
return "unknown cycle"
565584
}
585+
586+
// resolveScope resolves a key's scope, falling back to its coerced scope if it is UnspecifiedScope.
587+
func resolveScope(key plugin.DataKey, coercedScopes map[string]plugin.DataScope) plugin.DataScope {
588+
if key.Scope() == plugin.UnspecifiedScope {
589+
if scope, ok := coercedScopes[key.String()]; ok {
590+
return scope
591+
}
592+
}
593+
return key.Scope()
594+
}
595+
596+
// auditAndCoerceScopes audits all active plugins and determines dynamic scopes for UnspecifiedScope keys.
597+
func auditAndCoerceScopes(plugins []plugin.Plugin) map[string]plugin.DataScope {
598+
logger := log.Log.WithName("epp-graph-compiler")
599+
coercedScopes := make(map[string]plugin.DataScope)
600+
unspecifiedKeysLogged := make(map[string]bool)
601+
602+
for _, p := range plugins {
603+
var unspecifiedKeys []plugin.DataKey
604+
605+
if consumer, ok := p.(plugin.ConsumerPlugin); ok && consumer.Consumes() != nil {
606+
for key := range consumer.Consumes() {
607+
if key.Scope() == plugin.UnspecifiedScope {
608+
unspecifiedKeys = append(unspecifiedKeys, key)
609+
}
610+
}
611+
}
612+
if optConsumer, ok := p.(plugin.OptionalConsumerPlugin); ok && optConsumer.OptionalConsumes() != nil {
613+
for _, key := range optConsumer.OptionalConsumes() {
614+
if key.Scope() == plugin.UnspecifiedScope {
615+
unspecifiedKeys = append(unspecifiedKeys, key)
616+
}
617+
}
618+
}
619+
if producer, ok := p.(plugin.ProducerPlugin); ok && producer.Produces() != nil {
620+
for key := range producer.Produces() {
621+
if key.Scope() == plugin.UnspecifiedScope {
622+
unspecifiedKeys = append(unspecifiedKeys, key)
623+
}
624+
}
625+
}
626+
627+
for _, key := range unspecifiedKeys {
628+
keyStr := key.String()
629+
if unspecifiedKeysLogged[keyStr] {
630+
continue
631+
}
632+
unspecifiedKeysLogged[keyStr] = true
633+
634+
coercedScopes[keyStr] = plugin.EndpointScope
635+
636+
logger.Info(
637+
"WARNING: Legacy/un-upgraded plugin registered using UnspecifiedScope data key. Coercing to EndpointScope. Please upgrade the plugin to declare an explicit scope.",
638+
"plugin", p.TypedName().String(),
639+
"dataKey", keyStr,
640+
"coercedScope", plugin.EndpointScope,
641+
)
642+
}
643+
}
644+
645+
return coercedScopes
646+
}

pkg/epp/datalayer/data_graph_test.go

Lines changed: 101 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ func TestDAGAndTopologicalOrder(t *testing.T) {
153153
consumers[p.TypedName().String()] = cp
154154
}
155155
}
156-
dag, err := buildDAG(producers, consumers)
156+
dag, err := buildDAG(producers, consumers, nil)
157157
if err != nil {
158158
if tc.expectedErr != "" {
159159
assert.Error(t, err)
@@ -552,13 +552,13 @@ func TestCompilePipeline_ScopeConstraints(t *testing.T) {
552552
})
553553

554554
t.Run("PreAdmissionConsumesUnspecifiedKey", func(t *testing.T) {
555+
// UnspecifiedScope is strictly coerced to EndpointScope, so Pre-Admission consuming it throws a scope mismatch.
555556
dkUnspec := fwkplugin.NewDataKey("unspecKey", "mock")
556557
preAdmitterUnspec := &MockPreAdmitter{name: "PreAdmitterUnspec", consumes: map[fwkplugin.DataKey]any{dkUnspec: nil}}
557558

558559
_, _, err := CompilePipeline([]fwkplugin.Plugin{preAdmitterUnspec})
559560
assert.Error(t, err)
560561
assert.Contains(t, err.Error(), "scope mismatch")
561-
assert.Contains(t, err.Error(), "consumes key")
562562
})
563563

564564
t.Run("PreAdmissionProducesEndpointKey", func(t *testing.T) {
@@ -579,6 +579,7 @@ func TestCompilePipeline_ScopeConstraints(t *testing.T) {
579579
})
580580

581581
t.Run("PreAdmissionProducesUnspecifiedKey", func(t *testing.T) {
582+
// UnspecifiedScope is strictly coerced to EndpointScope, so Pre-Admission producing it throws a scope mismatch.
582583
dkReq := fwkplugin.NewRequestDataKey("reqKey", "mock")
583584
dkUnspec := fwkplugin.NewDataKey("unspecKey", "mock")
584585
pEagerUnspec := &mockEagerProducer{
@@ -592,7 +593,6 @@ func TestCompilePipeline_ScopeConstraints(t *testing.T) {
592593
_, _, err := CompilePipeline([]fwkplugin.Plugin{pEagerUnspec})
593594
assert.Error(t, err)
594595
assert.Contains(t, err.Error(), "scope mismatch")
595-
assert.Contains(t, err.Error(), "produces key")
596596
})
597597
}
598598

@@ -666,6 +666,95 @@ func TestCompilePipeline_BoundaryEdgeCases(t *testing.T) {
666666
})
667667
}
668668

669+
func TestCompilePipeline_LegacyFallback(t *testing.T) {
670+
t.Run("LegacyUnspecifiedKey_PreAdmissionCoercion", func(t *testing.T) {
671+
// Key with UnspecifiedScope
672+
dkUnspec := fwkplugin.NewDataKey("legacyKey", "mock")
673+
674+
// Producer producing the unspecified key
675+
pLegacy := &mockDataProducerP{name: "PLegacy", produces: map[fwkplugin.DataKey]any{dkUnspec: nil}}
676+
677+
// Pre-admission plugin consuming the unspecified key
678+
admitterReq := &MockPreAdmitter{name: "PreAdmitter", consumes: map[fwkplugin.DataKey]any{dkUnspec: nil}}
679+
680+
_, _, err := CompilePipeline([]fwkplugin.Plugin{pLegacy, admitterReq})
681+
assert.Error(t, err)
682+
// Expected invalid layer execution order error because legacy key is coerced to EndpointScope.
683+
assert.Contains(t, err.Error(), "invalid plugin layer execution order")
684+
})
685+
686+
t.Run("LegacyUnspecifiedKey_PostAdmissionCoercion", func(t *testing.T) {
687+
// Key with UnspecifiedScope
688+
dkUnspec := fwkplugin.NewDataKey("legacyKeyPost", "mock")
689+
690+
// Producer producing the unspecified key
691+
pLegacy := &mockDataProducerP{name: "PLegacyPost", produces: map[fwkplugin.DataKey]any{dkUnspec: nil}}
692+
693+
// Post-admission (admitter) plugin consuming the unspecified key
694+
admitterEp := &MockAdmitter{name: "Admitter", consumes: map[fwkplugin.DataKey]any{dkUnspec: nil}}
695+
696+
preAdmission, postAdmission, err := CompilePipeline([]fwkplugin.Plugin{pLegacy, admitterEp})
697+
assert.NoError(t, err)
698+
699+
// Verify they compile and are correctly sliced into Post-Admission.
700+
assert.Empty(t, preAdmission)
701+
assert.Contains(t, postAdmission, "PLegacyPost/mock")
702+
assert.Contains(t, postAdmission, "Admitter/mock")
703+
})
704+
705+
t.Run("LegacyUnspecifiedKey_OptionalPreAdmissionCoercion", func(t *testing.T) {
706+
// Key with UnspecifiedScope
707+
dkUnspec := fwkplugin.NewDataKey("legacyOptionalKey", "mock")
708+
709+
// Producer producing the unspecified key
710+
pLegacy := &mockDataProducerP{name: "PLegacyOpt", produces: map[fwkplugin.DataKey]any{dkUnspec: nil}}
711+
712+
// Pre-admission plugin optionally consuming the unspecified key
713+
admitterReq := &MockOptionalPreAdmitter{
714+
MockPreAdmitter: MockPreAdmitter{name: "PreAdmitter", consumes: nil},
715+
optionalConsumes: []fwkplugin.DataKey{dkUnspec},
716+
}
717+
718+
_, _, err := CompilePipeline([]fwkplugin.Plugin{pLegacy, admitterReq})
719+
assert.Error(t, err)
720+
assert.Contains(t, err.Error(), "invalid plugin layer execution order")
721+
})
722+
t.Run("OptionalConsumer_PruningAndTopologicalOrdering", func(t *testing.T) {
723+
// Key with RequestScope
724+
dkOpt := fwkplugin.NewRequestDataKey("optionalKey", "mock")
725+
726+
// Producer producing the optional key (non-eager)
727+
pOpt := &mockDataProducerP{name: "POpt", produces: map[fwkplugin.DataKey]any{dkOpt: nil}}
728+
729+
// Consumer optionally consuming the key
730+
admitterReq := &MockOptionalPreAdmitter{
731+
MockPreAdmitter: MockPreAdmitter{name: "PreAdmitter", consumes: nil},
732+
optionalConsumes: []fwkplugin.DataKey{dkOpt},
733+
}
734+
735+
preAdmission, _, err := CompilePipeline([]fwkplugin.Plugin{pOpt, admitterReq})
736+
assert.NoError(t, err)
737+
738+
// Verify that the producer is NOT pruned (it is kept because it's optionally consumed by active PreAdmitter).
739+
assert.Contains(t, preAdmission, "POpt/mock")
740+
assert.Contains(t, preAdmission, "PreAdmitter/mock")
741+
742+
// Verify correct topological sorted order: producer must execute before consumer.
743+
indexOfProducer := -1
744+
indexOfConsumer := -1
745+
for i, name := range preAdmission {
746+
switch name {
747+
case "POpt/mock":
748+
indexOfProducer = i
749+
case "PreAdmitter/mock":
750+
indexOfConsumer = i
751+
}
752+
}
753+
assert.True(t, indexOfProducer != -1 && indexOfConsumer != -1)
754+
assert.Less(t, indexOfProducer, indexOfConsumer, "Producer POpt should execute before optional consumer PreAdmitter")
755+
})
756+
}
757+
669758
func TestCreateMissingDataProducers(t *testing.T) {
670759
producerTypeA := "producer-a"
671760
producerTypeB := "producer-b"
@@ -921,6 +1010,15 @@ func (m *MockPreAdmitter) PreAdmit(ctx context.Context, request *fwksched.Infere
9211010
return nil
9221011
}
9231012

1013+
type MockOptionalPreAdmitter struct {
1014+
MockPreAdmitter
1015+
optionalConsumes []fwkplugin.DataKey
1016+
}
1017+
1018+
func (m *MockOptionalPreAdmitter) OptionalConsumes() []fwkplugin.DataKey {
1019+
return m.optionalConsumes
1020+
}
1021+
9241022
func assertTopologicalOrder(t *testing.T, dag map[string][]string, ordered []string) {
9251023
t.Helper()
9261024
positions := make(map[string]int)

pkg/epp/framework/plugins/datalayer/attribute/concurrency/data_types.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import (
2222
inflightloadconstants "github.com/llm-d/llm-d-router/pkg/epp/framework/plugins/requestcontrol/dataproducer/inflightload/constants"
2323
)
2424

25-
var InFlightLoadDataKey = plugin.NewDataKey("InFlightLoadDataKey", inflightloadconstants.InFlightLoadProducerType)
25+
var InFlightLoadDataKey = plugin.NewEndpointDataKey("InFlightLoadDataKey", inflightloadconstants.InFlightLoadProducerType)
2626

2727
// InFlightLoad captures the current real-time load of an endpoint as tracked by the EPP.
2828
type InFlightLoad struct {

0 commit comments

Comments
 (0)