-
Notifications
You must be signed in to change notification settings - Fork 76
Use structured logging #330
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,19 +26,21 @@ func (c *InferenceModelReconciler) Reconcile(ctx context.Context, req ctrl.Reque | |
if req.Namespace != c.PoolNamespacedName.Namespace { | ||
return ctrl.Result{}, nil | ||
} | ||
klog.V(1).Infof("Reconciling InferenceModel %v", req.NamespacedName) | ||
|
||
klogV := klog.V(logutil.DEFAULT) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like this pattern, but what happens when I want to use a non-default log level, mixing the logging objects might be odd. WDYT? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, you can always just do Actually doing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that I will reconsider this when implementing contextual logging, don't feel like postponing this. |
||
klogV.InfoS("Reconciling InferenceModel", "name", req.NamespacedName) | ||
|
||
infModel := &v1alpha1.InferenceModel{} | ||
if err := c.Get(ctx, req.NamespacedName, infModel); err != nil { | ||
if errors.IsNotFound(err) { | ||
klog.V(1).Infof("InferenceModel %v not found. Removing from datastore since object must be deleted", req.NamespacedName) | ||
klogV.InfoS("InferenceModel not found. Removing from datastore since object must be deleted", "name", req.NamespacedName) | ||
c.Datastore.InferenceModels.Delete(infModel.Spec.ModelName) | ||
return ctrl.Result{}, nil | ||
} | ||
klog.Error(err, "Unable to get InferenceModel") | ||
klogV.ErrorS(err, "Unable to get InferenceModel", "name", req.NamespacedName) | ||
return ctrl.Result{}, err | ||
} else if !infModel.DeletionTimestamp.IsZero() { | ||
klog.V(1).Infof("InferenceModel %v is marked for deletion. Removing from datastore", req.NamespacedName) | ||
klogV.InfoS("InferenceModel is marked for deletion. Removing from datastore", "name", req.NamespacedName) | ||
c.Datastore.InferenceModels.Delete(infModel.Spec.ModelName) | ||
return ctrl.Result{}, nil | ||
} | ||
|
@@ -48,13 +50,15 @@ func (c *InferenceModelReconciler) Reconcile(ctx context.Context, req ctrl.Reque | |
} | ||
|
||
func (c *InferenceModelReconciler) updateDatastore(infModel *v1alpha1.InferenceModel) { | ||
klogV := klog.V(logutil.DEFAULT) | ||
|
||
if infModel.Spec.PoolRef.Name == c.PoolNamespacedName.Name { | ||
klog.V(1).Infof("Incoming pool ref %v, server pool name: %v", infModel.Spec.PoolRef, c.PoolNamespacedName.Name) | ||
klog.V(1).Infof("Adding/Updating InferenceModel: %v", infModel.Spec.ModelName) | ||
klogV.InfoS("Updating datastore", "poolRef", infModel.Spec.PoolRef, "serverPoolName", c.PoolNamespacedName) | ||
klogV.InfoS("Adding/Updating InferenceModel", "modelName", infModel.Spec.ModelName) | ||
c.Datastore.InferenceModels.Store(infModel.Spec.ModelName, infModel) | ||
return | ||
} | ||
klog.V(logutil.DEFAULT).Infof("Removing/Not adding InferenceModel: %v", infModel.Spec.ModelName) | ||
klogV.InfoS("Removing/Not adding InferenceModel", "modelName", infModel.Spec.ModelName) | ||
// If we get here. The model is not relevant to this pool, remove. | ||
c.Datastore.InferenceModels.Delete(infModel.Spec.ModelName) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,23 +19,24 @@ import ( | |
// parameter. | ||
// Envoy sends the request body to ext proc before sending the request to the backend server. | ||
func (s *Server) HandleRequestBody(reqCtx *RequestContext, req *extProcPb.ProcessingRequest) (*extProcPb.ProcessingResponse, error) { | ||
klog.V(logutil.VERBOSE).Infof("Handling request body") | ||
klogV := klog.V(logutil.VERBOSE) | ||
klogV.InfoS("Handling request body") | ||
|
||
// Unmarshal request body (must be JSON). | ||
v := req.Request.(*extProcPb.ProcessingRequest_RequestBody) | ||
var rb map[string]interface{} | ||
if err := json.Unmarshal(v.RequestBody.Body, &rb); err != nil { | ||
klog.Errorf("Error unmarshaling request body: %v", err) | ||
klog.V(logutil.DEFAULT).ErrorS(err, "Error unmarshaling request body") | ||
return nil, fmt.Errorf("error unmarshaling request body: %v", err) | ||
} | ||
klog.V(logutil.VERBOSE).Infof("Request body: %v", rb) | ||
klogV.InfoS("Request body unmarshalled", "body", rb) | ||
|
||
// Resolve target models. | ||
model, ok := rb["model"].(string) | ||
if !ok { | ||
return nil, errors.New("model not found in request") | ||
} | ||
klog.V(logutil.VERBOSE).Infof("Model requested: %v", model) | ||
klogV.InfoS("Model requested", "model", model) | ||
modelName := model | ||
|
||
// NOTE: The nil checking for the modelObject means that we DO allow passthrough currently. | ||
|
@@ -56,7 +57,7 @@ func (s *Server) HandleRequestBody(reqCtx *RequestContext, req *extProcPb.Proces | |
ResolvedTargetModel: modelName, | ||
Critical: backend.IsCritical(modelObj), | ||
} | ||
klog.V(logutil.VERBOSE).Infof("LLM Request: %+v", llmReq) | ||
klogV.InfoS("LLM request assembled", "request", llmReq) | ||
|
||
requestBody := v.RequestBody.Body | ||
var err error | ||
|
@@ -65,17 +66,17 @@ func (s *Server) HandleRequestBody(reqCtx *RequestContext, req *extProcPb.Proces | |
rb["model"] = llmReq.ResolvedTargetModel | ||
requestBody, err = json.Marshal(rb) | ||
if err != nil { | ||
klog.Errorf("Error marshaling request body: %v", err) | ||
klog.V(logutil.DEFAULT).ErrorS(err, "Error marshaling request body") | ||
return nil, fmt.Errorf("error marshaling request body: %v", err) | ||
} | ||
klog.V(logutil.VERBOSE).Infof("Updated body: %v", string(requestBody)) | ||
klogV.InfoS("Updated request body marshalled", "body", string(requestBody)) | ||
} | ||
|
||
targetPod, err := s.scheduler.Schedule(llmReq) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to find target pod: %w", err) | ||
} | ||
klog.V(logutil.VERBOSE).Infof("Selected target model %v in target pod: %v\n", llmReq.ResolvedTargetModel, targetPod) | ||
klogV.InfoS("Target model and pod selected", "model", llmReq.ResolvedTargetModel, "pod", targetPod) | ||
|
||
reqCtx.Model = llmReq.Model | ||
reqCtx.ResolvedTargetModel = llmReq.ResolvedTargetModel | ||
|
@@ -101,7 +102,7 @@ func (s *Server) HandleRequestBody(reqCtx *RequestContext, req *extProcPb.Proces | |
} | ||
// Print headers for debugging | ||
for _, header := range headers { | ||
klog.V(logutil.VERBOSE).Infof("[request_body] Header Key: %s, Header Value: %s\n", header.Header.Key, header.Header.RawValue) | ||
klog.V(logutil.DEBUG).InfoS("Request body header", "key", header.Header.Key, "value", header.Header.RawValue) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like DEBUG level to me. |
||
} | ||
|
||
resp := &extProcPb.ProcessingResponse{ | ||
|
@@ -136,10 +137,9 @@ func (s *Server) HandleRequestBody(reqCtx *RequestContext, req *extProcPb.Proces | |
} | ||
|
||
func HandleRequestHeaders(reqCtx *RequestContext, req *extProcPb.ProcessingRequest) *extProcPb.ProcessingResponse { | ||
klog.V(logutil.VERBOSE).Info("Handling request headers ...") | ||
r := req.Request | ||
h := r.(*extProcPb.ProcessingRequest_RequestHeaders) | ||
klog.V(logutil.VERBOSE).Infof("Headers: %+v\n", h) | ||
klog.V(logutil.VERBOSE).InfoS("Handling request headers", "headers", h) | ||
|
||
resp := &extProcPb.ProcessingResponse{ | ||
Response: &extProcPb.ProcessingResponse_RequestHeaders{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like a VERBOSE message to me, but not sure.