-
Notifications
You must be signed in to change notification settings - Fork 693
feat: Add a "model" label to Component metrics #2389
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 9 commits
528626e
7ff3c75
e654668
506a495
84f8d92
940b726
0fba028
667ebe1
86d1811
e182fd5
52b53f9
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 |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ use anyhow::Context as _; | |
| use tokio::sync::{mpsc::Receiver, Notify}; | ||
|
|
||
| use dynamo_runtime::{ | ||
| metrics::MetricsRegistry, | ||
| pipeline::{ | ||
| network::egress::push_router::PushRouter, ManyOut, Operator, RouterMode, SegmentSource, | ||
| ServiceBackend, SingleIn, Source, | ||
|
|
@@ -169,7 +170,8 @@ impl ModelWatcher { | |
| let component = self | ||
| .drt | ||
| .namespace(&endpoint_id.namespace)? | ||
| .component(&endpoint_id.component)?; | ||
| .component(&endpoint_id.component) | ||
| .and_then(|c| c.add_labels(&[("model", &model_entry.name)]))?; | ||
|
Contributor
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. same question about client here- will take offline.
Contributor
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 removed this in: #2292 |
||
| let client = component.endpoint(&endpoint_id.name).client().await?; | ||
|
|
||
| let Some(etcd_client) = self.drt.etcd_client() else { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,10 +22,12 @@ use crate::{ | |
| Annotated, | ||
| }, | ||
| }; | ||
|
|
||
| use dynamo_runtime::{ | ||
| component::Client, | ||
| distributed::DistributedConfig, | ||
| engine::{AsyncEngineStream, Data}, | ||
| metrics::MetricsRegistry, | ||
| pipeline::{ | ||
| Context, ManyOut, Operator, PushRouter, RouterMode, SegmentSource, ServiceBackend, | ||
| ServiceEngine, ServiceFrontend, SingleIn, Source, | ||
|
|
@@ -109,7 +111,9 @@ pub async fn prepare_engine( | |
| let endpoint_id = local_model.endpoint_id(); | ||
| let component = distributed_runtime | ||
| .namespace(&endpoint_id.namespace)? | ||
| .component(&endpoint_id.component)?; | ||
| .component(&endpoint_id.component) | ||
| .and_then(|c| c.add_labels(&[("model", card.slug().to_string().as_str())]))?; | ||
|
Contributor
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. @keivenchang , @grahamking , @tzulingk - similar question here - is this relevant on a client?
Contributor
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 removed this in #2292 |
||
|
|
||
| let client = component.endpoint(&endpoint_id.name).client().await?; | ||
|
|
||
| let kv_chooser = if router_mode == RouterMode::KV { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,7 +15,9 @@ use crate::{ | |
| Annotated, | ||
| }, | ||
| }; | ||
|
|
||
| use dynamo_runtime::engine::AsyncEngineStream; | ||
| use dynamo_runtime::metrics::MetricsRegistry; | ||
| use dynamo_runtime::pipeline::{ | ||
| network::Ingress, Context, ManyOut, Operator, SegmentSource, ServiceBackend, SingleIn, Source, | ||
| }; | ||
|
|
@@ -31,9 +33,25 @@ pub async fn run( | |
| let cancel_token = distributed_runtime.primary_token().clone(); | ||
| let endpoint_id: EndpointId = path.parse()?; | ||
|
|
||
| let model_name = match &engine_config { | ||
| EngineConfig::StaticFull { model, .. } | EngineConfig::StaticCore { model, .. } => { | ||
| Some(model.card().slug().to_string()) | ||
tzulingk marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
| EngineConfig::StaticRemote(model) | EngineConfig::Dynamic(model) => { | ||
| Some(model.card().slug().to_string()) | ||
| } | ||
grahamking marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }; | ||
|
|
||
| let component = distributed_runtime | ||
| .namespace(&endpoint_id.namespace)? | ||
| .component(&endpoint_id.component)?; | ||
| .component(&endpoint_id.component) | ||
|
Contributor
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. question: @grahamking is this for the in process engines? @keivenchang do we have these hooked up into the frontend metrics?
Contributor
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 removed this in #2292 |
||
| .and_then(|c| { | ||
| if let Some(ref name) = model_name { | ||
| c.add_labels(&[("model", name.as_str())]) | ||
| } else { | ||
| Ok(c) | ||
| } | ||
| })?; | ||
| let endpoint = component | ||
| .service_builder() | ||
| .create() | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.