4380 - Inference logging to blob storage #4473

cjohannsen-cloudera · 2025-05-14T23:34:18Z

What this PR does / why we need it:
This adds support for storing model request and response logs to S3. The implementation is generic enough that adding support for GCS et al will be straightforward, I hope.

Storage is defined using the StorageSpec. The existing v1beta1.StorageSpec has been refactored to extract a base struct.

// StorageSpec defines a spec for an object in an object store
type StorageSpec struct {
	// The path to the object in the storage. Note that this path is relative to the storage URI.
	// +optional
	Path *string `json:"path,omitempty"`
	// Parameters to override the default storage credentials and config.
	// +optional
	Parameters *map[string]string `json:"parameters,omitempty"`
	// The Storage Key in the secret for this object.
	// +optional
	StorageKey *string `json:"key,omitempty"`
}

The existing StorageSpec is now ModelStorageSpec and adds the SchemaPath

type ModelStorageSpec struct {
	StorageSpec `json:",inline"`
	// The path to the model schema file in the storage.
	// +optional
	SchemaPath *string `json:"schemaPath,omitempty"`
}

This is a pure refactor.

The LoggerConfig now contains a StorageSpec that defines the logger storage output location and a service account name used to fetch credentials.

  logger: |-
    {
        "image" : "kserve/agent:latest",
        "memoryRequest": "100Mi",
        "memoryLimit": "1Gi",
        "cpuRequest": "100m",
        "cpuLimit": "1",
        "defaultUrl": "http://default-broker",
        "storage": {
            "path": "",
            "parameters": {
              "type": "s3",
              "format": "json" 
            },
            "key": ""
            "serviceAccountName": "",
        }
    }

Three new command line parameters have been added to the agent to allow the storage configuration to be passed in when injecting the agent.

--log-store-path [path] - path within the storage location to write the logger output
--log-store-format - output format of the stored data, one of "json" or "yaml", default "json"

By default the logger service account is undefined, and the logger storage feature is disabled.

If the service account name is defined, the logger will attempt to look for a Service Account with the matching name and mount the secrets it contains. When the agent is launched the secrets are available in the environment.

When an inference occurs the S3 client is constructed and the request and response are stored to the blob store path specified above. The agent uses the existing client defined for the model downloader to simplify integration and configuration, enhanced to allow for uploads.

Which issue(s) this PR fixes

Fixes #4380

Type of changes

New feature (non-breaking change which adds functionality)

Feature/Issue validation/testing:

[x] Added additional test to logger/handler_test.go
[x] Added new test, logger/store_test.go
[x] Added new agent command argument to agent_injector_test.go

To test locally create a secret in the default namespace

apiVersion: v1
kind: Secret
metadata:
  annotations:
    serving.kserve.io/s3-region: [YOUR_REGION]
  name: agent-logger-secret
  namespace: default
type: Opaque
data:
  AWS_ACCESS_KEY_ID: [YOUR_ACCESS_KEY_ID]
  AWS_DEFAULT_REGION: [YOUR_REGION]
  AWS_SECRET_ACCESS_KEY: [YOUR_SECRET_ACCESS_KEY]

Create a service account for the secret:

apiVersion: v1
kind: ServiceAccount
metadata:
  name: logger-sa
  namespace: default
secrets:
- name: agent-logger-secret

To test the feature, deploy a model with inference logging:

apiVersion: serving.kserve.io/v1beta1
kind: InferenceService
metadata:
  name: sklearn-iris
spec:
  predictor:
    logger:
      mode: all
      url: s3://[YOUR_BUCKET]/
      storage:
        path: "/logger"
        parameters:
          type: s3
          format: json
        key: credentials
    model:
      modelFormat:
        name: sklearn
      storageUri: gs://kfserving-examples/models/sklearn/1.0/model

Send a prediction to the endpoint and the request and response are stored as json in the blob storage location [YOUR_BUCKET]/logger/

Documentation updated in PR 481

Release note:

Re-running failed tests

/rerun-all - rerun all failed workflows.
/rerun-workflow <workflow name> - rerun a specific failed workflow. Only one workflow name can be specified. Multiple /rerun-workflow commands are allowed per comment.

cjohannsen-cloudera · 2025-05-15T22:22:30Z

/rerun-workflow test-llm
/rerun-workflow test-predictor
/rerun-workflow build

cmd/agent/main.go

go.mod

pkg/logger/mock.go

pkg/logger/store_test.go

sivanantha321 · 2025-05-21T08:58:53Z

@cjohannsen-cloudera Can you help update the documentation as well ?https://github.com/kserve/website/blob/main/docs/modelserving/logger/logger.md

cjohannsen-cloudera · 2025-05-21T17:27:02Z

@cjohannsen-cloudera Can you help update the documentation as well ?https://github.com/kserve/website/blob/main/docs/modelserving/logger/logger.md

@sivanantha321 yes I can do that

/rerun-workflow test

Signed-off-by: cory-johannsen <cjohannsen@cloudera.com>

Co-authored-by: Sivanantham <90966311+sivanantha321@users.noreply.github.com> Signed-off-by: cjohannsen-cloudera <cjohannsen@cloudera.com>

Signed-off-by: cory-johannsen <cjohannsen@cloudera.com>

sivanantha321 · 2025-06-13T06:45:53Z

cmd/agent/main.go

-	kfslogger.StartDispatcher(workers, logger)
+	var store kfslogger.Store
+	if kfslogger.GetStorageStrategy(*logUrl) != kfslogger.HttpStorage {
+		if logStoreFormat != nil && *logStoreFormat != "" && logStorePath != nil && *logStorePath != "" {


Maybe we can have a default for logFromat ?

sivanantha321 · 2025-06-13T06:57:54Z

pkg/credentials/service_account_credentials.go

-			storageSecretName = secretName
+
+	// if the secret name is provided use it
+	if secretName != nil {


Can we have a test case for this ?

sivanantha321 · 2025-06-13T07:10:50Z

pkg/webhook/admission/pod/agent_injector.go

@@ -46,6 +48,7 @@ const (
 	LoggerArgumentTlsSkipVerify       = "--logger-tls-skip-verify"
 	LoggerArgumentMetadataHeaders     = "--metadata-headers"
 	LoggerArgumentMetadataAnnotations = "--metadata-annotations"
+	LoggerDefaultServiceAccountName   = "logger-sa"


Is this default name configurable ?

sivanantha321 · 2025-06-13T07:15:01Z

pkg/webhook/admission/pod/agent_injector.go

@@ -56,16 +59,22 @@ type AgentConfig struct {
 	MemoryLimit   string `json:"memoryLimit"`
 }

+type LoggerStorageSpec struct {


This type is already defined in the pkg/apis/serving/v1beta1/inference_service.go. We can reuse that.

cjohannsen-cloudera force-pushed the cj/4380-inference-logging branch from bfc0b10 to 013211f Compare May 15, 2025 16:32

cjohannsen-cloudera force-pushed the cj/4380-inference-logging branch from bd59e67 to b15dca7 Compare May 16, 2025 17:07

sivanantha321 reviewed May 21, 2025

View reviewed changes

cmd/agent/main.go Outdated Show resolved Hide resolved

sivanantha321 reviewed May 21, 2025

View reviewed changes

go.mod Outdated Show resolved Hide resolved

sivanantha321 reviewed May 21, 2025

View reviewed changes

pkg/logger/mock.go Outdated Show resolved Hide resolved

sivanantha321 reviewed May 21, 2025

View reviewed changes

pkg/logger/store_test.go Outdated Show resolved Hide resolved

cjohannsen-cloudera mentioned this pull request May 21, 2025

4380 - Add documentation for inference logging to blob storage kserve/website#481

Open

cjohannsen-cloudera requested a review from sivanantha321 May 21, 2025 18:34

cjohannsen-cloudera force-pushed the cj/4380-inference-logging branch 3 times, most recently from fc89c0a to 2a25e55 Compare May 28, 2025 16:37

cjohannsen-cloudera force-pushed the cj/4380-inference-logging branch from 2a25e55 to c03cfdc Compare May 29, 2025 16:01

Copy link

Author

cjohannsen-cloudera commented May 29, 2025

/rerun-workflow test

cjohannsen-cloudera force-pushed the cj/4380-inference-logging branch 3 times, most recently from e7a8f44 to e902eb4 Compare June 6, 2025 16:22

cjohannsen-cloudera force-pushed the cj/4380-inference-logging branch 2 times, most recently from 498eb54 to 4d2450b Compare June 11, 2025 17:37

cjohannsen-cloudera and others added 9 commits June 12, 2025 08:44

4380 - Inference logging to blob storage

01a3b2f

Signed-off-by: cory-johannsen <cjohannsen@cloudera.com>

make precommit

a6f22f0

Signed-off-by: cory-johannsen <cjohannsen@cloudera.com>

update python test

7387023

Signed-off-by: cory-johannsen <cjohannsen@cloudera.com>

4380 - Inference logging to blob storage

848209b

Signed-off-by: cory-johannsen <cjohannsen@cloudera.com>

make precommit

2634ab9

Signed-off-by: cory-johannsen <cjohannsen@cloudera.com>

Update pkg/logger/mock.go

37f51cc

Co-authored-by: Sivanantham <90966311+sivanantha321@users.noreply.github.com> Signed-off-by: cjohannsen-cloudera <cjohannsen@cloudera.com>

Update pkg/logger/store_test.go

bdae874

Co-authored-by: Sivanantham <90966311+sivanantha321@users.noreply.github.com> Signed-off-by: cjohannsen-cloudera <cjohannsen@cloudera.com>

remove unwanted dependency

2906d04

Signed-off-by: cory-johannsen <cjohannsen@cloudera.com>

fix: check for nil/empty on logStorePath

7082cbd

Signed-off-by: cory-johannsen <cjohannsen@cloudera.com>

cjohannsen-cloudera force-pushed the cj/4380-inference-logging branch from 4d2450b to 7082cbd Compare June 12, 2025 15:44

sivanantha321 reviewed Jun 13, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

4380 - Inference logging to blob storage #4473

4380 - Inference logging to blob storage #4473

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

4380 - Inference logging to blob storage #4473

Are you sure you want to change the base?

4380 - Inference logging to blob storage #4473

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!