feat(extractor): add spoke-epp engine type for multi-cluster#1928
feat(extractor): add spoke-epp engine type for multi-cluster#1928yehuditkerido wants to merge 1 commit into
Conversation
github-actions
Bot
added
size/L
github-actions
Bot
added
kind/feature
Auto-detect cluster endpoints via empty PodName and map Spoke EPP's pool-average metrics (llm_d_epp_average_*) to the standard Metrics struct. Signed-off-by: Yehudit Kerido <ykerido@redhat.com>
49fe1d2 to
c8b053e
Compare
|
@yehuditkerido zooming out for a second - does a spoke cluster endpoint need the standard metrics extractor and metrics fields or is it scraping a different format? |
| } | ||
|
|
||
| // getEngineTypeFromEndpoint extracts the engine type from endpoint metadata labels. | ||
| // getEngineTypeFromEndpoint extracts the engine type from endpoint metadata. |
There was a problem hiding this comment.
can the same be achieved by defining a custom set of metrics mapping instead of a new type?
|
|
||
| // getEngineTypeFromEndpoint extracts the engine type from endpoint metadata labels. | ||
| // getEngineTypeFromEndpoint extracts the engine type from endpoint metadata. | ||
| // Cluster endpoints (identified by empty PodName) automatically use SpokeEPPEngineType. |
There was a problem hiding this comment.
how are cluster endpoints named - which field is used? is the reliance on "magic" (empty PodName means its a cluster endpoint) the right long term approach or should we use an endpoint attribute instead (the file based discovery can populate the attribute)?
| // Case 1: Engine = vllm (uses default) | ||
| epVllm := fwkdl.NewEndpoint(&fwkdl.EndpointMetadata{ | ||
| Labels: map[string]string{DefaultEngineTypeLabelKey: "vllm"}, | ||
| PodName: "vllm-pod", |
There was a problem hiding this comment.
the empty == cluster seems fragile. Case in point, it was not an assumption that holds for the current code base.
| registry := NewMappingRegistry() | ||
|
|
||
| // Register spoke-epp mapping for cluster endpoints | ||
| spokeMapping, err := NewMapping( |
There was a problem hiding this comment.
the empty mappings indicate that EPP metrics and endpoint metrics are the same shape. I would strongly recommend a new Extractor for EPP endpoints.
It can fill in the existing metrics fields or create a new endpoint attribute with only the relevant fields.
I would suspect that Cluster endpoints have different attributes than inference server endpoints (not just metrics).
| CacheBlockSizeSpec: "", | ||
| CacheNumBlocksSpec: "", | ||
| }, | ||
| // "spoke-epp" defines metrics for multi-cluster Hub-and-Spoke deployments where the Hub EPP |
There was a problem hiding this comment.
Please reconsider the overloading of the current metrics struct and mechanisms for carrying EPP information. While doable, feels hackish and there should be cleaner alterntiaves using core datalayer concepts for new plugins.
2 participants
What type of PR is this?
/kind feature
What this PR does / why we need it:
Extends the
core-metrics-extractorplugin with a newspoke-eppengine type for Hub-and-Spoke multi-cluster deployments. This enables Hub EPP to scrape pool-average metrics from Spoke EPPs and map them to the standardMetricsstruct.Key changes:
SpokeEPPEngineTypeconstant and engine config mappingllm_d_epp_average_*metricsPodName == ""(no labels required)Which issue(s) this PR fixes:
Fixes #1913 Related to #1907
Release note (write
NONEif no user-facing change):