Skip to content

Commit 2b0e757

Browse files
committed
refactor(datalayer): key AttributeMap by DataKey instead of string
Signed-off-by: satyamg1620 <Satyam.Gupta.3@ibm.com>
1 parent ae478aa commit 2b0e757

63 files changed

Lines changed: 275 additions & 213 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

pkg/epp/datalayer/data_graph_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ import (
3535
"github.com/llm-d/llm-d-router/pkg/epp/util"
3636
)
3737

38-
const mockProducedDataKey = "mockProducedData"
38+
var mockProducedDataKey = fwkplugin.NewDataKey("mockProducedData", "mock")
3939

4040
type mockDataProducerP struct {
4141
name string

pkg/epp/framework/interface/datalayer/attributemap.go

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ package datalayer
1818

1919
import (
2020
"sync"
21+
22+
fwkplugin "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/plugin"
2123
)
2224

2325
// Cloneable types support cloning of the value.
@@ -41,16 +43,21 @@ func (d *DynamicAttribute) Clone() Cloneable {
4143
// AttributeMap is used to store flexible metadata or traits
4244
// across different aspects of an inference server.
4345
// Stored values must be Cloneable.
46+
//
47+
// Keys are DataKey values rather than strings so that a plugin can only reach
48+
// an attribute through a key it holds -- the same value it names in Produces()
49+
// or Consumes(). This removes the raw-string escape hatch by which a plugin
50+
// could read or write an attribute unrelated to its declaration.
4451
type AttributeMap interface {
45-
Put(string, Cloneable)
46-
Get(string) (Cloneable, bool)
47-
Keys() []string
52+
Put(fwkplugin.DataKey, Cloneable)
53+
Get(fwkplugin.DataKey) (Cloneable, bool)
54+
Keys() []fwkplugin.DataKey
4855
Clone() AttributeMap
4956
}
5057

5158
// Attributes provides a goroutine-safe implementation of AttributeMap.
5259
type Attributes struct {
53-
data sync.Map // key: attribute name (string), value: attribute value (opaque, Cloneable)
60+
data sync.Map // key: DataKey, value: attribute value (opaque, Cloneable)
5461
}
5562

5663
// NewAttributes returns a new instance of Attributes.
@@ -59,14 +66,14 @@ func NewAttributes() AttributeMap {
5966
}
6067

6168
// Put adds or updates an attribute in the map.
62-
func (a *Attributes) Put(key string, value Cloneable) {
69+
func (a *Attributes) Put(key fwkplugin.DataKey, value Cloneable) {
6370
if value != nil {
6471
a.data.Store(key, value) // TODO: Clone into map to ensure isolation
6572
}
6673
}
6774

6875
// Get retrieves an attribute by key, returning a cloned copy (or resolving it dynamically).
69-
func (a *Attributes) Get(key string) (Cloneable, bool) {
76+
func (a *Attributes) Get(key fwkplugin.DataKey) (Cloneable, bool) {
7077
val, ok := a.data.Load(key)
7178
if !ok {
7279
return nil, false
@@ -87,11 +94,11 @@ func (a *Attributes) Get(key string) (Cloneable, bool) {
8794
}
8895

8996
// Keys returns all keys in the attribute map.
90-
func (a *Attributes) Keys() []string {
91-
var keys []string
97+
func (a *Attributes) Keys() []fwkplugin.DataKey {
98+
var keys []fwkplugin.DataKey
9299
a.data.Range(func(key, _ any) bool {
93-
if sk, ok := key.(string); ok {
94-
keys = append(keys, sk)
100+
if dk, ok := key.(fwkplugin.DataKey); ok {
101+
keys = append(keys, dk)
95102
}
96103
return true
97104
})
@@ -102,9 +109,9 @@ func (a *Attributes) Keys() []string {
102109
func (a *Attributes) Clone() AttributeMap {
103110
clone := NewAttributes()
104111
a.data.Range(func(key, value any) bool {
105-
if sk, ok := key.(string); ok {
112+
if dk, ok := key.(fwkplugin.DataKey); ok {
106113
if v, ok := value.(Cloneable); ok {
107-
clone.Put(sk, v)
114+
clone.Put(dk, v)
108115
}
109116
}
110117
return true
@@ -114,7 +121,7 @@ func (a *Attributes) Clone() AttributeMap {
114121

115122
// ReadAttribute retrieves attribute with the given key from AttributeMap and asserts it to type T.
116123
// Second return value is 'false' if the key is not found or the type assertion fails.
117-
func ReadAttribute[T Cloneable](attributeMap AttributeMap, key string) (T, bool) {
124+
func ReadAttribute[T Cloneable](attributeMap AttributeMap, key fwkplugin.DataKey) (T, bool) {
118125
var zero T
119126

120127
raw, ok := attributeMap.Get(key)

pkg/epp/framework/interface/datalayer/attributemap_test.go

Lines changed: 41 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,13 @@ import (
2121

2222
"github.com/google/go-cmp/cmp"
2323
"github.com/stretchr/testify/assert"
24+
25+
fwkplugin "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/plugin"
26+
)
27+
28+
var (
29+
keyA = fwkplugin.NewDataKey("a", "test-producer")
30+
keyB = fwkplugin.NewDataKey("b", "test-producer")
2431
)
2532

2633
type dummy struct {
@@ -42,38 +49,41 @@ func (d *anotherDummy) Clone() Cloneable {
4249
func TestExpectPutThenGetToMatch(t *testing.T) {
4350
attrs := NewAttributes()
4451
original := &dummy{"foo"}
45-
attrs.Put("a", original)
52+
attrs.Put(keyA, original)
4653

47-
got, ok := attrs.Get("a")
54+
got, ok := attrs.Get(keyA)
4855
assert.True(t, ok, "expected key to exist")
4956
assert.NotSame(t, original, got, "expected Get to return a clone, not original")
5057

5158
dv, ok := got.(*dummy)
5259
assert.True(t, ok, "expected value to be of type *dummy")
5360
assert.Equal(t, "foo", dv.Text)
5461

55-
_, ok = attrs.Get("b")
62+
_, ok = attrs.Get(keyB)
5663
assert.False(t, ok, "expected key not to exist")
5764
}
5865

5966
func TestExpectKeysToMatchAdded(t *testing.T) {
6067
attrs := NewAttributes()
61-
attrs.Put("x", &dummy{"1"})
62-
attrs.Put("y", &dummy{"2"})
68+
keyX := fwkplugin.NewDataKey("x", "test-producer")
69+
keyY := fwkplugin.NewDataKey("y", "test-producer")
70+
attrs.Put(keyX, &dummy{"1"})
71+
attrs.Put(keyY, &dummy{"2"})
6372

6473
keys := attrs.Keys()
6574
assert.Len(t, keys, 2)
66-
assert.ElementsMatch(t, keys, []string{"x", "y"})
75+
assert.ElementsMatch(t, keys, []fwkplugin.DataKey{keyX, keyY})
6776
}
6877

6978
func TestCloneReturnsCopy(t *testing.T) {
7079
original := NewAttributes()
71-
original.Put("k", &dummy{"value"})
80+
keyK := fwkplugin.NewDataKey("k", "test-producer")
81+
original.Put(keyK, &dummy{"value"})
7282

7383
cloned := original.Clone()
7484

75-
kOrig, _ := original.Get("k")
76-
kClone, _ := cloned.Get("k")
85+
kOrig, _ := original.Get(keyK)
86+
kClone, _ := cloned.Get(keyK)
7787

7888
assert.NotSame(t, kOrig, kClone, "expected cloned value to be a different instance")
7989
if diff := cmp.Diff(kOrig, kClone); diff != "" {
@@ -85,24 +95,38 @@ func TestReadAttribute(t *testing.T) {
8595
// successful retrieval
8696
attrs := NewAttributes()
8797
original := &dummy{"foo"}
88-
attrs.Put("a", original)
98+
attrs.Put(keyA, original)
8999

90-
got, ok := ReadAttribute[*dummy](attrs, "a")
100+
got, ok := ReadAttribute[*dummy](attrs, keyA)
91101
assert.True(t, ok, "expected key to exist and value to be of type *dummy")
92102
assert.NotSame(t, original, got, "expected Get to return a clone, not original")
93103
assert.Equal(t, "foo", got.Text)
94104

95105
// missing key
96-
_, ok = ReadAttribute[*dummy](attrs, "b")
106+
_, ok = ReadAttribute[*dummy](attrs, keyB)
97107
assert.False(t, ok, "expected key not to exist")
98108

99109
// type mismatch
100-
other, ok := ReadAttribute[*anotherDummy](attrs, "a")
110+
other, ok := ReadAttribute[*anotherDummy](attrs, keyA)
101111
assert.False(t, ok, "expected type mismatch")
102112
assert.Nil(t, other) // zero value of pointer is nil
103113

104114
}
105115

116+
func TestKeysWithDifferentProducersAreDistinct(t *testing.T) {
117+
attrs := NewAttributes()
118+
attrs.Put(keyA, &dummy{"foo"})
119+
120+
other := keyA.WithNonEmptyProducerName("other-producer")
121+
_, ok := attrs.Get(other)
122+
assert.False(t, ok, "expected key with a different producer name to be a distinct entry")
123+
124+
attrs.Put(other, &dummy{"bar"})
125+
got, ok := attrs.Get(keyA)
126+
assert.True(t, ok)
127+
assert.Equal(t, "foo", got.(*dummy).Text, "expected the original entry to be untouched")
128+
}
129+
106130
func TestDynamicAttribute(t *testing.T) {
107131
attrs := NewAttributes()
108132

@@ -113,18 +137,19 @@ func TestDynamicAttribute(t *testing.T) {
113137
},
114138
}
115139

116-
attrs.Put("dynamic_key", dynamic)
140+
dynamicKey := fwkplugin.NewDataKey("dynamic", "test-producer")
141+
attrs.Put(dynamicKey, dynamic)
117142

118143
// First read
119-
got, ok := attrs.Get("dynamic_key")
144+
got, ok := attrs.Get(dynamicKey)
120145
assert.True(t, ok)
121146
assert.Equal(t, "live", got.(*dummy).Text)
122147

123148
// Mutate the source value
124149
val.Text = "changed"
125150

126151
// Second read should reflect change
127-
got2, ok := attrs.Get("dynamic_key")
152+
got2, ok := attrs.Get(dynamicKey)
128153
assert.True(t, ok)
129154
assert.Equal(t, "changed", got2.(*dummy).Text)
130155
}

pkg/epp/framework/interface/scheduling/types.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"sync"
2424

2525
fwkdl "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/datalayer"
26+
fwkplugin "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/plugin"
2627
fwkrh "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/requesthandling"
2728
)
2829

@@ -79,9 +80,9 @@ type Endpoint interface {
7980
GetMetadata() *fwkdl.EndpointMetadata
8081
GetMetrics() *fwkdl.Metrics
8182
String() string
82-
Get(string) (fwkdl.Cloneable, bool)
83-
Put(string, fwkdl.Cloneable)
84-
Keys() []string
83+
Get(fwkplugin.DataKey) (fwkdl.Cloneable, bool)
84+
Put(fwkplugin.DataKey, fwkdl.Cloneable)
85+
Keys() []fwkplugin.DataKey
8586
Clone() fwkdl.AttributeMap
8687
}
8788

pkg/epp/framework/interface/scheduling/types_test.go

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,16 @@ import (
2323
"k8s.io/apimachinery/pkg/types"
2424

2525
fwkdl "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/datalayer"
26+
fwkplugin "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/plugin"
2627
fwkrh "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/requesthandling"
2728
)
2829

30+
var (
31+
testKeyK = fwkplugin.NewDataKey("k", "test-producer")
32+
testKeyK1 = fwkplugin.NewDataKey("k1", "test-producer")
33+
testKeyExtra = fwkplugin.NewDataKey("extra", "test-producer")
34+
)
35+
2936
type cloneableString string
3037

3138
func (s cloneableString) Clone() fwkdl.Cloneable { return s }
@@ -71,7 +78,7 @@ func TestNewEndpoint_CopiesInputs(t *testing.T) {
7178
meta := newTestMetadata("pod-a")
7279
metrics := newTestMetrics()
7380
attr := fwkdl.NewAttributes()
74-
attr.Put("key", cloneableString("value"))
81+
attr.Put(testKeyK, cloneableString("value"))
7582

7683
ep := NewEndpoint(meta, metrics, attr)
7784
assert.NotNil(t, ep)
@@ -85,7 +92,7 @@ func TestNewEndpoint_CopiesInputs(t *testing.T) {
8592
assert.Equal(t, 3, ep.GetMetrics().RunningRequestsSize)
8693

8794
// values from attribute map should be retrievable
88-
v, ok := ep.Get("key")
95+
v, ok := ep.Get(testKeyK)
8996
assert.True(t, ok)
9097
assert.Equal(t, cloneableString("value"), v)
9198
}
@@ -96,8 +103,8 @@ func TestNewEndpoint_NilAttributeUsesDefault(t *testing.T) {
96103
assert.Empty(t, ep.Keys())
97104

98105
// Should still be safe to write into the default-allocated attribute map
99-
ep.Put("k", cloneableString("v"))
100-
v, ok := ep.Get("k")
106+
ep.Put(testKeyK, cloneableString("v"))
107+
v, ok := ep.Get(testKeyK)
101108
assert.True(t, ok)
102109
assert.Equal(t, cloneableString("v"), v)
103110
}
@@ -115,20 +122,20 @@ func TestEndpoint_StringContainsFields(t *testing.T) {
115122

116123
func TestEndpoint_Clone(t *testing.T) {
117124
attr := fwkdl.NewAttributes()
118-
attr.Put("k", cloneableString("v"))
125+
attr.Put(testKeyK, cloneableString("v"))
119126
ep := NewEndpoint(newTestMetadata("pod-d"), newTestMetrics(), attr)
120127

121128
cloned := ep.Clone()
122-
v, ok := cloned.Get("k")
129+
v, ok := cloned.Get(testKeyK)
123130
assert.True(t, ok)
124131
assert.Equal(t, cloneableString("v"), v)
125132
}
126133

127134
func TestEndpointComparer_Equal(t *testing.T) {
128135
attrA := fwkdl.NewAttributes()
129-
attrA.Put("k", cloneableString("v"))
136+
attrA.Put(testKeyK, cloneableString("v"))
130137
attrB := fwkdl.NewAttributes()
131-
attrB.Put("k", cloneableString("v"))
138+
attrB.Put(testKeyK, cloneableString("v"))
132139

133140
a := NewEndpoint(newTestMetadata("pod"), newTestMetrics(), attrA)
134141
b := NewEndpoint(newTestMetadata("pod"), newTestMetrics(), attrB)
@@ -153,10 +160,10 @@ func TestEndpointComparer_DifferByMetrics(t *testing.T) {
153160

154161
func TestEndpointComparer_DifferByAttributeKeys(t *testing.T) {
155162
attrA := fwkdl.NewAttributes()
156-
attrA.Put("k1", cloneableString("v"))
163+
attrA.Put(testKeyK1, cloneableString("v"))
157164
attrB := fwkdl.NewAttributes()
158-
attrB.Put("k1", cloneableString("v"))
159-
attrB.Put("extra", cloneableString("x"))
165+
attrB.Put(testKeyK1, cloneableString("v"))
166+
attrB.Put(testKeyExtra, cloneableString("x"))
160167

161168
a := NewEndpoint(newTestMetadata("pod"), newTestMetrics(), attrA)
162169
b := NewEndpoint(newTestMetadata("pod"), newTestMetrics(), attrB)
@@ -166,9 +173,9 @@ func TestEndpointComparer_DifferByAttributeKeys(t *testing.T) {
166173

167174
func TestEndpointComparer_DifferByAttributeValues(t *testing.T) {
168175
attrA := fwkdl.NewAttributes()
169-
attrA.Put("k", cloneableString("v1"))
176+
attrA.Put(testKeyK, cloneableString("v1"))
170177
attrB := fwkdl.NewAttributes()
171-
attrB.Put("k", cloneableString("v2"))
178+
attrB.Put(testKeyK, cloneableString("v2"))
172179

173180
a := NewEndpoint(newTestMetadata("pod"), newTestMetrics(), attrA)
174181
b := NewEndpoint(newTestMetadata("pod"), newTestMetrics(), attrB)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,6 @@ func (v GPUUtilization) Clone() fwkdl.Cloneable {
4040
}
4141

4242
// ReadGPUUtilization retrieves GPU utilization from an endpoint's AttributeMap.
43-
func ReadGPUUtilization(attrs fwkdl.AttributeMap, key string) (GPUUtilization, bool) {
43+
func ReadGPUUtilization(attrs fwkdl.AttributeMap, key plugin.DataKey) (GPUUtilization, bool) {
4444
return fwkdl.ReadAttribute[GPUUtilization](attrs, key)
4545
}

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

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,16 @@ limitations under the License.
1616

1717
package metrics
1818

19-
import fwkdl "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/datalayer"
19+
import (
20+
fwkdl "github.com/llm-d/llm-d-router/pkg/epp/framework/interface/datalayer"
21+
"github.com/llm-d/llm-d-router/pkg/epp/framework/interface/plugin"
22+
)
23+
24+
const (
25+
// MetricsExtractorType is the plugin type for the core metrics extractor,
26+
// which publishes the custom scalar metric attributes.
27+
MetricsExtractorType = "core-metrics-extractor"
28+
)
2029

2130
// ScalarMetricValue is a numeric endpoint attribute extracted from a configured scalar metric.
2231
type ScalarMetricValue float64
@@ -25,6 +34,14 @@ func (v ScalarMetricValue) Clone() fwkdl.Cloneable {
2534
return v
2635
}
2736

28-
func ReadScalarMetricValue(attrs fwkdl.AttributeMap, key string) (ScalarMetricValue, bool) {
37+
// ScalarMetricDataKey returns the key under which the core metrics extractor
38+
// publishes the custom scalar metric configured as attributeKey. The data type
39+
// of a custom metric is chosen in configuration rather than in code, so the key
40+
// is derived at construction time instead of being declared as a package var.
41+
func ScalarMetricDataKey(attributeKey string) plugin.DataKey {
42+
return plugin.NewDataKey(attributeKey, MetricsExtractorType)
43+
}
44+
45+
func ReadScalarMetricValue(attrs fwkdl.AttributeMap, key plugin.DataKey) (ScalarMetricValue, bool) {
2946
return fwkdl.ReadAttribute[ScalarMetricValue](attrs, key)
3047
}

pkg/epp/framework/plugins/datalayer/extractor/dcgm/extractor.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ func (e *Extractor) Extract(_ context.Context, in fwkdl.PollInput[sourcemetrics.
102102
}
103103

104104
normalized := attrgpu.GPUUtilization(maxUtil / 100.0)
105-
in.Endpoint.GetAttributes().Put(e.dk.String(), normalized)
105+
in.Endpoint.GetAttributes().Put(e.dk, normalized)
106106
return nil
107107
}
108108

0 commit comments

Comments
 (0)