Skip to content

Commit 249c208

Browse files
nirrozenbaumnirrozenbaum
authored
align attritbute reporter plugin structure (#2290)
Signed-off-by: Nir Rozenbaum <nrozenba@redhat.com>
1 parent a4c9899 commit 249c208

File tree

2 files changed

+38
-47
lines changed

2 files changed

+38
-47
lines changed

‎pkg/epp/framework/plugins/requestcontrol/requestattributereporter/plugin.go‎

Lines changed: 31 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,11 @@ import (
2222
"errors"
2323
"fmt"
2424

25-
"github.com/go-logr/logr"
2625
"github.com/google/cel-go/cel"
2726
"google.golang.org/protobuf/types/known/structpb"
2827
"sigs.k8s.io/controller-runtime/pkg/log"
2928

29+
logutil "sigs.k8s.io/gateway-api-inference-extension/pkg/common/util/logging"
3030
"sigs.k8s.io/gateway-api-inference-extension/pkg/epp/framework/interface/datalayer"
3131
"sigs.k8s.io/gateway-api-inference-extension/pkg/epp/framework/interface/plugin"
3232
"sigs.k8s.io/gateway-api-inference-extension/pkg/epp/framework/interface/requestcontrol"
@@ -36,14 +36,14 @@ import (
3636
const (
3737
// RequestAttributeReporterType is the type of this plugin.
3838
RequestAttributeReporterType = "request-attribute-reporter"
39-
// DefaultNamespace is the default namespace for the dynamic metadata.
40-
DefaultNamespace = "envoy.lb"
39+
// defaultNamespace is the default namespace for the dynamic metadata.
40+
defaultNamespace = "envoy.lb"
4141
)
4242

4343
// Plugin state
4444
type Plugin struct {
45+
typedName plugin.TypedName
4546
config Config
46-
logger logr.Logger
4747
env *cel.Env
4848
expressionProg cel.Program
4949
conditionProg cel.Program // Can be nil if no condition
@@ -72,20 +72,19 @@ type AttributeKey struct {
7272
}
7373

7474
func RequestAttributeReporterPluginFactory(name string, rawParameters json.RawMessage, handle plugin.Handle) (plugin.Plugin, error) {
75-
logger := log.FromContext(handle.Context()).WithName(name)
7675
pluginConfig := Config{}
7776
if err := json.Unmarshal(rawParameters, &pluginConfig); err != nil {
7877
return nil, fmt.Errorf("failed to unmarshal config: %w", err)
7978
}
8079

81-
plugin, err := New(pluginConfig, logger)
80+
plugin, err := New(handle.Context(), pluginConfig)
8281
if err != nil {
8382
return nil, err
8483
}
85-
return plugin, nil
84+
return plugin.WithName(name), nil
8685
}
8786

88-
func New(config Config, logger logr.Logger) (*Plugin, error) {
87+
func New(ctx context.Context, config Config) (*Plugin, error) {
8988
if len(config.Attributes) != 1 {
9089
return nil, errors.New("attributes must contain exactly one entry")
9190
}
@@ -97,12 +96,10 @@ func New(config Config, logger logr.Logger) (*Plugin, error) {
9796
return nil, errors.New("attributes[0].expression cannot be empty")
9897
}
9998
if attributeKey.Namespace == "" {
100-
attributeKey.Namespace = DefaultNamespace
99+
attributeKey.Namespace = defaultNamespace
101100
}
102101

103-
env, err := cel.NewEnv(
104-
cel.Variable("usage", cel.ObjectType("google.protobuf.Struct")),
105-
)
102+
env, err := cel.NewEnv(cel.Variable("usage", cel.ObjectType("google.protobuf.Struct")))
106103
if err != nil {
107104
return nil, err
108105
}
@@ -131,49 +128,46 @@ func New(config Config, logger logr.Logger) (*Plugin, error) {
131128
}
132129

133130
return &Plugin{
131+
typedName: plugin.TypedName{Type: RequestAttributeReporterType, Name: RequestAttributeReporterType},
134132
config: config,
135-
logger: logger,
136133
env: env,
137134
expressionProg: expressionProg,
138135
conditionProg: conditionProg,
139136
}, nil
140137
}
141138

142-
// Type returns the type of the plugin.
143-
func (c *Plugin) Type() string {
144-
return RequestAttributeReporterType
139+
// WithName sets the name of the plugin.
140+
func (p *Plugin) WithName(name string) *Plugin {
141+
p.typedName.Name = name
142+
return p
145143
}
146144

147145
// TypedName returns the typed name of the plugin.
148146
func (c *Plugin) TypedName() plugin.TypedName {
149-
return plugin.TypedName{
150-
Type: c.Type(),
151-
}
147+
return c.typedName
152148
}
153149

154150
// ResponseComplete implements the requestcontrol.ResponseComplete interface.
155-
func (c *Plugin) ResponseComplete(
156-
ctx context.Context, request *scheduling.LLMRequest, response *requestcontrol.Response, _ *datalayer.EndpointMetadata) {
157-
logger := c.logger.WithValues("plugin", RequestAttributeReporterType)
158-
151+
func (c *Plugin) ResponseComplete(ctx context.Context, request *scheduling.LLMRequest, response *requestcontrol.Response,
152+
_ *datalayer.EndpointMetadata) {
159153
// Convert the request usage Go struct into a protobuf struct so that it can be used as a CEL variable.
160154
celData, err := c.getCelData(response)
161155
if err != nil {
162-
logger.V(1).Error(err, "Failed to convert usage into CEL data")
156+
log.FromContext(ctx).Error(err, "Failed to convert usage into CEL data")
163157
return
164158
}
165159

166-
shouldCalculateValue, err := c.shouldCalculateValue(celData)
160+
shouldCalculateValue, err := c.shouldCalculateValue(ctx, celData)
167161
if err != nil {
168-
logger.V(1).Error(err, "Error in shouldCalculateValue")
162+
log.FromContext(ctx).Error(err, "Error in shouldCalculateValue")
169163
return
170164
}
171165
if !shouldCalculateValue {
172-
logger.V(1).Info("shouldCalculateValue is false, returning")
166+
log.FromContext(ctx).Info("shouldCalculateValue is false, returning")
173167
return
174168
}
175169

176-
intVal, err := c.calculateValue(celData)
170+
intVal, err := c.calculateValue(ctx, celData)
177171
if err != nil {
178172
return // Error already logged
179173
}
@@ -201,7 +195,7 @@ func (c *Plugin) ResponseComplete(
201195

202196
namespaceMap.GetStructValue().Fields[attributeKey.Name] = attributeValue
203197

204-
logger.V(1).Info("Wrote dynamic metadata value to dynamic metadata", "value", intVal)
198+
log.FromContext(ctx).V(logutil.VERBOSE).Info("Wrote dynamic metadata value to dynamic metadata", "value", intVal)
205199
}
206200

207201
func (c *Plugin) getCelData(response *requestcontrol.Response) (any, error) {
@@ -216,22 +210,22 @@ func (c *Plugin) getCelData(response *requestcontrol.Response) (any, error) {
216210
return usageStruct, nil
217211
}
218212

219-
func (c *Plugin) shouldCalculateValue(celData any) (bool, error) {
213+
func (c *Plugin) shouldCalculateValue(ctx context.Context, celData any) (bool, error) {
220214
if c.conditionProg != nil {
221-
val, err := c.maybeExecuteProg(c.conditionProg, celData, "condition", c.config.Attributes[0].Condition)
215+
val, err := c.maybeExecuteProg(ctx, c.conditionProg, celData, "condition", c.config.Attributes[0].Condition)
222216
if err != nil {
223217
return false, err // Error already logged in maybeExecuteProg
224218
}
225219
if bVal, ok := val.(bool); !ok || !bVal {
226-
c.logger.V(1).Info("Condition not met", "condition", c.config.Attributes[0].Condition)
220+
log.FromContext(ctx).V(logutil.VERBOSE).Info("Condition not met", "condition", c.config.Attributes[0].Condition)
227221
return false, nil
228222
}
229223
}
230224
return true, nil
231225
}
232226

233-
func (c *Plugin) calculateValue(celData any) (int64, error) {
234-
val, err := c.maybeExecuteProg(c.expressionProg, celData, "expression", c.config.Attributes[0].Expression)
227+
func (c *Plugin) calculateValue(ctx context.Context, celData any) (int64, error) {
228+
val, err := c.maybeExecuteProg(ctx, c.expressionProg, celData, "expression", c.config.Attributes[0].Expression)
235229
if err != nil {
236230
return -1, err // Error already logged
237231
}
@@ -241,18 +235,18 @@ func (c *Plugin) calculateValue(celData any) (int64, error) {
241235
// Try int64 as well
242236
int64Val, ok := val.(int64)
243237
if !ok {
244-
c.logger.Error(errors.New("type conversion error"), "Expression result could not be converted to float64 or int64", "expression", c.config.Attributes[0].Expression, "result", val)
238+
log.FromContext(ctx).Error(errors.New("type conversion error"), "Expression result could not be converted to float64 or int64", "expression", c.config.Attributes[0].Expression, "result", val)
245239
return -1, nil
246240
}
247241
doubleVal = float64(int64Val)
248242
}
249243
return int64(doubleVal), nil
250244
}
251245

252-
func (c *Plugin) maybeExecuteProg(prog cel.Program, celData any, exprType string, expression string) (any, error) {
246+
func (c *Plugin) maybeExecuteProg(ctx context.Context, prog cel.Program, celData any, exprType string, expression string) (any, error) {
253247
val, _, err := prog.Eval(map[string]any{"usage": celData})
254248
if err != nil {
255-
c.logger.Error(err, "Failed to evaluate "+exprType, exprType, expression)
249+
log.FromContext(ctx).Error(err, "Failed to evaluate "+exprType, exprType, expression)
256250
return nil, err
257251
}
258252
return val.Value(), nil

‎pkg/epp/framework/plugins/requestcontrol/requestattributereporter/plugin_test.go‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ import (
2424
"google.golang.org/protobuf/proto"
2525
"google.golang.org/protobuf/testing/protocmp"
2626
"google.golang.org/protobuf/types/known/structpb"
27-
"sigs.k8s.io/controller-runtime/pkg/log/zap"
2827

2928
"sigs.k8s.io/gateway-api-inference-extension/pkg/epp/framework/interface/datalayer"
3029
"sigs.k8s.io/gateway-api-inference-extension/pkg/epp/framework/interface/requestcontrol"
@@ -35,7 +34,6 @@ import (
3534
var _ requestcontrol.ResponseComplete = &Plugin{}
3635

3736
func TestPluginCreation(t *testing.T) {
38-
logger := zap.New(zap.UseDevMode(true))
3937
tests := []struct {
4038
name string
4139
config Config
@@ -55,7 +53,7 @@ func TestPluginCreation(t *testing.T) {
5553
},
5654
},
5755
wantErr: false,
58-
wantNS: DefaultNamespace,
56+
wantNS: defaultNamespace,
5957
},
6058
{
6159
name: "valid config with custom namespace",
@@ -158,7 +156,7 @@ func TestPluginCreation(t *testing.T) {
158156

159157
for _, tt := range tests {
160158
t.Run(tt.name, func(t *testing.T) {
161-
plugin, err := New(tt.config, logger)
159+
plugin, err := New(context.Background(), tt.config)
162160

163161
if tt.wantErr {
164162
if err == nil {
@@ -185,7 +183,6 @@ func TestPluginCreation(t *testing.T) {
185183
}
186184

187185
func TestValueReporting(t *testing.T) {
188-
logger := zap.New(zap.UseDevMode(true))
189186
tests := []struct {
190187
name string
191188
config Config
@@ -211,7 +208,7 @@ func TestValueReporting(t *testing.T) {
211208
},
212209
wantResult: &structpb.Struct{
213210
Fields: map[string]*structpb.Value{
214-
DefaultNamespace: {
211+
defaultNamespace: {
215212
Kind: &structpb.Value_StructValue{
216213
StructValue: &structpb.Struct{
217214
Fields: map[string]*structpb.Value{
@@ -243,7 +240,7 @@ func TestValueReporting(t *testing.T) {
243240
},
244241
wantResult: &structpb.Struct{
245242
Fields: map[string]*structpb.Value{
246-
DefaultNamespace: {
243+
defaultNamespace: {
247244
Kind: &structpb.Value_StructValue{
248245
StructValue: &structpb.Struct{
249246
Fields: map[string]*structpb.Value{
@@ -359,7 +356,7 @@ func TestValueReporting(t *testing.T) {
359356
},
360357
wantResult: &structpb.Struct{
361358
Fields: map[string]*structpb.Value{
362-
DefaultNamespace: {
359+
defaultNamespace: {
363360
Kind: &structpb.Value_StructValue{
364361
StructValue: &structpb.Struct{
365362
Fields: map[string]*structpb.Value{
@@ -390,7 +387,7 @@ func TestValueReporting(t *testing.T) {
390387
},
391388
wantResult: &structpb.Struct{
392389
Fields: map[string]*structpb.Value{
393-
DefaultNamespace: {
390+
defaultNamespace: {
394391
Kind: &structpb.Value_StructValue{
395392
StructValue: &structpb.Struct{
396393
Fields: map[string]*structpb.Value{
@@ -414,7 +411,7 @@ func TestValueReporting(t *testing.T) {
414411
currentResponse.DynamicMetadata = proto.Clone(tt.response.DynamicMetadata).(*structpb.Struct)
415412
}
416413

417-
plugin, err := New(tt.config, logger)
414+
plugin, err := New(context.Background(), tt.config)
418415
if err != nil {
419416
t.Fatalf("Failed to create plugin: %v", err)
420417
}

0 commit comments

Comments
 (0)