Skip to content
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

feat: Add observability publisher deployment #550

Merged
merged 12 commits into from
Mar 15, 2024
Merged

Conversation

tiopramayudi
Copy link
Contributor

@tiopramayudi tiopramayudi commented Mar 8, 2024

Description

Add capability to deploy observability publisher (please suggest the better name if you have any) that consume prediction log that is produced by the model. This observability publisher will be deployed once the model is serving or model redeployment if the model already serve the traffic.

Modifications

  • Adding event producer (api/pkg/observability/event/event.go) for observability publisher. This event producer will produce deployment event for observability publisher, which later on is consumed by the deployment worker in merlin control plane to do actual deployment.

  • Actual action for deployment (api/queue/work/observability_publisher_deployment.go) that consume event that is produced by the producer. There are several condition:

    • Deployment is skip if the more latest revision is already queued (revision in DB is greater than the one is observed by the observability_publisher_deployment
    • Deployment will be delayed by requeue if there is still ongoing deployment for previous revision
    • Deployment will be skip if there is ongoing deployment for newer revision compare to current observed revision by the observability_publisher_deployment

    Once the conditions are met, it will deploy or undeploy k8s deployment and secret manifest

  • Add deployer (api/pkg/observability/deployment/deployer.go) to interact with k8s control plane. There are 3 methods in this deployer

    • Deploy -> Create or update k8s secret and deployment manifest
    • Undeploy -> Delete k8s secret and deployment manifest
    • GetDeployedManifest -> Get deployed manifest that contains k8s secret and manifest, and status of deployment
  • Adding new field in model observability_supported as a gate whether the model is allowed to publish observability data. Observability publisher only be deployed if the model observability_supported is true and the model endpoint also enable model observability

Tests

Checklist

  • Added PR label
  • Added unit test, integration, and/or e2e tests
  • Tested locally
  • Updated documentation
  • Update Swagger spec if the PR introduce API changes
  • Regenerated Golang and Python client if the PR introduces API changes

Release Notes


@tiopramayudi tiopramayudi added the enhancement New feature or request label Mar 8, 2024
@tiopramayudi tiopramayudi marked this pull request as draft March 8, 2024 07:54
@tiopramayudi tiopramayudi marked this pull request as ready for review March 11, 2024 01:48
@khorshuheng
Copy link
Collaborator

In general it looks alright, but there are some important things that might need to be fixed:

There's no support for Kubernetes service account in the deployment spec. The observation publisher currently requires permission to write to a bigquery dataset. Hence, we would either need to find a way to grant permission to the default Kubernetes service account, or support using a non default service account for the observation publisher deployment.

The publisher is currently deployed on the same namespace as the model. In that case, we will have to grant permission to the default service account / mlobs service account on every namespace which model observability is supposed to be enabled.

Alternatively, we can deploy the observation publisher on the same namespace as the other Merlin services.

@khorshuheng
Copy link
Collaborator

lgtm

@tiopramayudi tiopramayudi merged commit 444f9eb into main Mar 15, 2024
32 checks passed
@tiopramayudi tiopramayudi deleted the consumer-deployment branch March 15, 2024 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants