Skip to content

Commit 2ae4360

Browse files
naufalandikaMuhammad Naufal Andika Natsir Putra
andauthored
feat(api): allow enable mlobs from API directly (#633)
# Description - deprecate `Model.ObservabilitySupported` - use `VersionEndpoint.ModelObservability.Enabled` as SSOT for MLObs configuration # Modifications - remove the use of `Model.ObservabilitySupported` - use `VersionEndpoint.EnableModelObservability` value as `VersionEndpoint.ModelObservability.Enabled` if `VersionEndpoint.ModelObservability` is not set - remove unit test with `Model.ObservabilitySupported` logic - adjust unit test mock call # Checklist - [x] Added PR label - [ ] Added unit test, integration, and/or e2e tests - [x] Tested locally - [ ] Updated documentation - [ ] Update Swagger spec if the PR introduce API changes - [ ] Regenerated Golang and Python client if the PR introduces API changes --------- Co-authored-by: Muhammad Naufal Andika Natsir Putra <[email protected]>
1 parent 75ead87 commit 2ae4360

14 files changed

+275
-280
lines changed

api/api/validator.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -179,13 +179,11 @@ func modelObservabilityValidation(endpoint *models.VersionEndpoint, model *model
179179
if !endpoint.IsModelMonitoringEnabled() {
180180
return nil
181181
}
182+
182183
if !slices.Contains(supportedObservabilityModelTypes, model.Type) {
183184
return fmt.Errorf("%s: %w", model.Type, ErrUnsupportedObservabilityModelType)
184185
}
185186

186-
if !model.ObservabilitySupported {
187-
return fmt.Errorf("model observability is not supported for this model")
188-
}
189187
return nil
190188
})
191189
}

api/api/version_endpoints_api.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ func (c *EndpointsController) CreateEndpoint(r *http.Request, vars map[string]st
154154
}
155155

156156
newEndpoint.EnvironmentName = env.Name
157+
newEndpoint.SetModeObservabilityIfNil()
157158
}
158159

159160
validationRules := []requestValidator{
@@ -210,6 +211,8 @@ func (c *EndpointsController) UpdateEndpoint(r *http.Request, vars map[string]st
210211
return BadRequest("Unable to parse body as version endpoint resource")
211212
}
212213

214+
newEndpoint.SetModeObservabilityIfNil()
215+
213216
env, err := c.AppContext.EnvironmentService.GetEnvironment(newEndpoint.EnvironmentName)
214217
if err != nil {
215218
if errors.Is(err, gorm.ErrRecordNotFound) {

api/api/version_endpoints_api_test.go

Lines changed: 23 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -1068,7 +1068,9 @@ func TestCreateEndpoint(t *testing.T) {
10681068
Value: "1",
10691069
},
10701070
}),
1071-
EnableModelObservability: true,
1071+
ModelObservability: &models.ModelObservability{
1072+
Enabled: true,
1073+
},
10721074
},
10731075
modelService: func() *mocks.ModelsService {
10741076
svc := &mocks.ModelsService{}
@@ -1200,105 +1202,6 @@ func TestCreateEndpoint(t *testing.T) {
12001202
},
12011203
},
12021204
},
1203-
{
1204-
desc: "Fail when try to enable model observability but the model is not supported yet",
1205-
vars: map[string]string{
1206-
"model_id": "1",
1207-
"version_id": "1",
1208-
},
1209-
requestBody: &models.VersionEndpoint{
1210-
ID: uuid,
1211-
VersionID: models.ID(1),
1212-
VersionModelID: models.ID(1),
1213-
ServiceName: "sample",
1214-
Namespace: "sample",
1215-
EnvironmentName: "dev",
1216-
Message: "",
1217-
ResourceRequest: &models.ResourceRequest{
1218-
MinReplica: 1,
1219-
MaxReplica: 4,
1220-
CPURequest: resource.MustParse("1"),
1221-
MemoryRequest: resource.MustParse("1Gi"),
1222-
},
1223-
EnvVars: models.EnvVars([]models.EnvVar{
1224-
{
1225-
Name: "WORKER",
1226-
Value: "1",
1227-
},
1228-
}),
1229-
EnableModelObservability: true,
1230-
},
1231-
modelService: func() *mocks.ModelsService {
1232-
svc := &mocks.ModelsService{}
1233-
svc.On("FindByID", mock.Anything, models.ID(1)).Return(&models.Model{
1234-
ID: models.ID(1),
1235-
Name: "model-1",
1236-
ProjectID: models.ID(1),
1237-
Project: mlp.Project{},
1238-
ExperimentID: 1,
1239-
Type: "pyfunc",
1240-
MlflowURL: "",
1241-
Endpoints: nil,
1242-
ObservabilitySupported: false,
1243-
}, nil)
1244-
return svc
1245-
},
1246-
versionService: func() *mocks.VersionsService {
1247-
svc := &mocks.VersionsService{}
1248-
svc.On("FindByID", mock.Anything, models.ID(1), models.ID(1), mock.Anything).Return(&models.Version{
1249-
ID: models.ID(1),
1250-
ModelID: models.ID(1),
1251-
Model: &models.Model{
1252-
ID: models.ID(1),
1253-
Name: "model-1",
1254-
ProjectID: models.ID(1),
1255-
Project: mlp.Project{},
1256-
ExperimentID: 1,
1257-
Type: "pyfunc",
1258-
MlflowURL: "",
1259-
Endpoints: nil,
1260-
},
1261-
}, nil)
1262-
return svc
1263-
},
1264-
envService: func() *mocks.EnvironmentService {
1265-
svc := &mocks.EnvironmentService{}
1266-
svc.On("GetDefaultEnvironment").Return(&models.Environment{
1267-
ID: models.ID(1),
1268-
Name: "dev",
1269-
Cluster: "dev",
1270-
IsDefault: &trueBoolean,
1271-
Region: "id",
1272-
GcpProject: "dev-proj",
1273-
MaxCPU: "1",
1274-
MaxMemory: "1Gi",
1275-
}, nil)
1276-
svc.On("GetEnvironment", "dev").Return(&models.Environment{
1277-
ID: models.ID(1),
1278-
Name: "dev",
1279-
Cluster: "dev",
1280-
IsDefault: &trueBoolean,
1281-
Region: "id",
1282-
GcpProject: "dev-proj",
1283-
MaxCPU: "1",
1284-
MaxMemory: "1Gi",
1285-
}, nil)
1286-
return svc
1287-
},
1288-
endpointService: func() *mocks.EndpointsService {
1289-
svc := &mocks.EndpointsService{}
1290-
svc.On("CountEndpoints", context.Background(), mock.Anything, mock.Anything).Return(0, nil)
1291-
return svc
1292-
},
1293-
monitoringConfig: config.MonitoringConfig{},
1294-
feastCoreMock: func() *feastmocks.CoreServiceClient {
1295-
return &feastmocks.CoreServiceClient{}
1296-
},
1297-
expected: &Response{
1298-
code: http.StatusBadRequest,
1299-
data: Error{Message: "Request validation failed: model observability is not supported for this model"},
1300-
},
1301-
},
13021205
{
13031206
desc: "Should return 400 if UPI is not supported",
13041207
vars: map[string]string{
@@ -4004,7 +3907,9 @@ func TestUpdateEndpoint(t *testing.T) {
40043907
Value: "1",
40053908
},
40063909
}),
4007-
EnableModelObservability: true,
3910+
ModelObservability: &models.ModelObservability{
3911+
Enabled: true,
3912+
},
40083913
},
40093914
modelService: func() *mocks.ModelsService {
40103915
svc := &mocks.ModelsService{}
@@ -4083,7 +3988,9 @@ func TestUpdateEndpoint(t *testing.T) {
40833988
Value: "1",
40843989
},
40853990
}),
4086-
EnableModelObservability: false,
3991+
ModelObservability: &models.ModelObservability{
3992+
Enabled: false,
3993+
},
40873994
}, nil)
40883995
svc.On("DeployEndpoint", context.Background(), mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(&models.VersionEndpoint{
40893996
ID: uuid,
@@ -4114,8 +4021,10 @@ func TestUpdateEndpoint(t *testing.T) {
41144021
Value: "1",
41154022
},
41164023
}),
4117-
CreatedUpdated: models.CreatedUpdated{},
4118-
EnableModelObservability: true,
4024+
CreatedUpdated: models.CreatedUpdated{},
4025+
ModelObservability: &models.ModelObservability{
4026+
Enabled: true,
4027+
},
41194028
}, nil)
41204029
return svc
41214030
},
@@ -4150,8 +4059,10 @@ func TestUpdateEndpoint(t *testing.T) {
41504059
Value: "1",
41514060
},
41524061
}),
4153-
CreatedUpdated: models.CreatedUpdated{},
4154-
EnableModelObservability: true,
4062+
CreatedUpdated: models.CreatedUpdated{},
4063+
ModelObservability: &models.ModelObservability{
4064+
Enabled: true,
4065+
},
41554066
},
41564067
},
41574068
},
@@ -4696,7 +4607,9 @@ func TestUpdateEndpoint(t *testing.T) {
46964607
Value: "1",
46974608
},
46984609
}),
4699-
EnableModelObservability: true,
4610+
ModelObservability: &models.ModelObservability{
4611+
Enabled: true,
4612+
},
47004613
},
47014614
modelService: func() *mocks.ModelsService {
47024615
svc := &mocks.ModelsService{}
@@ -4775,7 +4688,9 @@ func TestUpdateEndpoint(t *testing.T) {
47754688
Value: "1",
47764689
},
47774690
}),
4778-
EnableModelObservability: false,
4691+
ModelObservability: &models.ModelObservability{
4692+
Enabled: false,
4693+
},
47794694
}, nil)
47804695
return svc
47814696
},

api/models/model_endpoint.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ type ModelEndpoint struct {
3838
}
3939

4040
func (me *ModelEndpoint) GetVersionEndpoint() *VersionEndpoint {
41+
if me == nil {
42+
return nil
43+
}
44+
4145
if me.Rule == nil || len(me.Rule.Destination) == 0 {
4246
return nil
4347
}

api/models/model_observability.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,14 @@ type ModelObservability struct {
1616
PredictionLogIngestionResourceRequest *WorkerResourceRequest `json:"prediction_log_ingestion_resource_request"`
1717
}
1818

19+
func (mo *ModelObservability) IsEnabled() bool {
20+
if mo == nil {
21+
return false
22+
}
23+
24+
return mo.Enabled
25+
}
26+
1927
// GroundTruthSource represents the source configuration for ground truth data.
2028
type GroundTruthSource struct {
2129
TableURN string `json:"table_urn"`

api/models/version_endpoint.go

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,10 +134,10 @@ func (ve *VersionEndpoint) IsServing() bool {
134134
}
135135

136136
func (ve *VersionEndpoint) IsModelMonitoringEnabled() bool {
137-
if ve.ModelObservability == nil {
138-
return ve.EnableModelObservability
137+
if ve == nil {
138+
return false
139139
}
140-
return ve.ModelObservability.Enabled
140+
return ve.ModelObservability.IsEnabled()
141141
}
142142

143143
func (ve *VersionEndpoint) Hostname() string {
@@ -184,6 +184,23 @@ func (ve *VersionEndpoint) ParsedURL() (*url.URL, error) {
184184
return parsedURL, nil
185185
}
186186

187+
// [TODO]: deprecate this after deprecating VersionEndpoint.EnableModelObservability
188+
// perviously we only have VersionEndpoint.EnableModelObservability and now we want to deprecate it
189+
// and only read/write to VersionEndpoint.ModelObservability instead. to allow backward compatibility if the user
190+
// only set VersionEndpoint.EnableModelObservability but not VersionEndpoint.ModelObservability we will use the
191+
// VersionEndpoint.EnableModelObservability value as VersionEndpoint.ModelObservability.EnableModelObservability
192+
func (ve *VersionEndpoint) SetModeObservabilityIfNil() {
193+
if ve == nil {
194+
return
195+
}
196+
if ve.ModelObservability != nil {
197+
return
198+
}
199+
ve.ModelObservability = &ModelObservability{
200+
Enabled: ve.EnableModelObservability,
201+
}
202+
}
203+
187204
type EndpointMonitoringURLParams struct {
188205
Cluster string
189206
Project string

api/models/version_endpoint_test.go

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -244,38 +244,20 @@ func TestVersionEndpoint_IsModelMonitoringEnabled(t *testing.T) {
244244
want bool
245245
}{
246246
{
247-
name: "model observability is nil but enable model observability is true",
248-
versionEndpoint: &VersionEndpoint{
249-
ModelObservability: nil,
250-
EnableModelObservability: true,
251-
},
252-
want: true,
253-
},
254-
{
255-
name: "model observability is nil and enable model observability is false",
256-
versionEndpoint: &VersionEndpoint{
257-
ModelObservability: nil,
258-
EnableModelObservability: false,
259-
},
260-
want: false,
261-
},
262-
{
263-
name: "model observability is not nil and enabled is true",
247+
name: "model observability enabled is true",
264248
versionEndpoint: &VersionEndpoint{
265249
ModelObservability: &ModelObservability{
266250
Enabled: true,
267251
},
268-
EnableModelObservability: true,
269252
},
270253
want: true,
271254
},
272255
{
273-
name: "model observability is not nil and enabled is false",
256+
name: "model observability enabled is false",
274257
versionEndpoint: &VersionEndpoint{
275258
ModelObservability: &ModelObservability{
276259
Enabled: false,
277260
},
278-
EnableModelObservability: false,
279261
},
280262
want: false,
281263
},

api/pkg/observability/event/event.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,6 @@ func NewEventProducer(jobProducer queue.Producer, observabilityPublisherStorage
3434
}
3535

3636
func (e *eventProducer) ModelEndpointChangeEvent(modelEndpoint *models.ModelEndpoint, model *models.Model) error {
37-
if !model.ObservabilitySupported {
38-
return nil
39-
}
40-
4137
ctx := context.Background()
4238
publisher, err := e.observabilityPublisherStorage.GetByModelID(ctx, model.ID)
4339
if err != nil {
@@ -69,6 +65,10 @@ func (e *eventProducer) ModelEndpointChangeEvent(modelEndpoint *models.ModelEndp
6965
}
7066

7167
versionEndpoint := modelEndpoint.GetVersionEndpoint()
68+
if !versionEndpoint.IsModelMonitoringEnabled() {
69+
return nil
70+
}
71+
7272
version, err := e.findVersionWithModelSchema(ctx, versionEndpoint.VersionID, model.ID)
7373
if err != nil {
7474
return err
@@ -93,10 +93,6 @@ func (e *eventProducer) ModelEndpointChangeEvent(modelEndpoint *models.ModelEndp
9393
}
9494

9595
func (e *eventProducer) VersionEndpointChangeEvent(versionEndpoint *models.VersionEndpoint, model *models.Model) error {
96-
if !model.ObservabilitySupported {
97-
return nil
98-
}
99-
10096
// check if version endpoint is used by the model endpoint
10197
// if version endpoint is not serving skipping deployment
10298
if versionEndpoint.Status != models.EndpointServing {

0 commit comments

Comments
 (0)