Skip suspended hosts when fetching deployment metrics#36045
Open
Skip suspended hosts when fetching deployment metrics#36045
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces the ability to exclude suspended hosts from deployment metrics collection by adding a suspension lookup interface and wiring it into metrics retrieval in configserver.
Changes:
- Add
NodeSuspensionProviderinterface (with a default empty implementation) for querying suspended hostnames per application. - Update
DeploymentMetricsRetrieverto filter out suspended hostnames when building the metrics target host list. - Wire
DeploymentMetricsRetrieverthroughApplicationRepositoryso deployment metrics requests use the injected suspension provider (intended).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| configserver/src/test/java/com/yahoo/vespa/config/server/metrics/DeploymentMetricsRetrieverTest.java | Adds a unit test asserting suspended hosts are filtered out of metrics retrieval. |
| configserver/src/main/java/com/yahoo/vespa/config/server/metrics/DeploymentMetricsRetriever.java | Fetches suspended hostnames via NodeSuspensionProvider and filters them from the host list used for metrics. |
| configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java | Adds DI plumbing for a shared DeploymentMetricsRetriever instance (currently miswired). |
| config-provisioning/src/main/java/com/yahoo/config/provision/NodeSuspensionProvider.java | Introduces the new suspension provider API with a no-op default. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
Outdated
Show resolved
Hide resolved
bjormel
force-pushed
the
bjormel/skip-suspended-hosts-in-deployment-metrics
branch
from
f5b7d7d to
0c6ce07
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
config-provisioning/src/main/java/com/yahoo/config/provision/NodeSuspensionProvider.java
Outdated
Show resolved
Hide resolved
configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
Outdated
Show resolved
Hide resolved
...rver/src/test/java/com/yahoo/vespa/config/server/metrics/DeploymentMetricsRetrieverTest.java
Show resolved
Hide resolved
bjormel
force-pushed
the
bjormel/skip-suspended-hosts-in-deployment-metrics
branch
2 times, most recently
from
89f1935 to
914e2a5
Compare
bjormel
force-pushed
the
bjormel/skip-suspended-hosts-in-deployment-metrics
branch
from
914e2a5 to
c7a9489
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
Outdated
Show resolved
Hide resolved
configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
Outdated
Show resolved
Hide resolved
...rver/src/test/java/com/yahoo/vespa/config/server/metrics/DeploymentMetricsRetrieverTest.java
Show resolved
Hide resolved
...rver/src/test/java/com/yahoo/vespa/config/server/metrics/DeploymentMetricsRetrieverTest.java
Show resolved
Hide resolved
bjormel
force-pushed
the
bjormel/skip-suspended-hosts-in-deployment-metrics
branch
2 times, most recently
from
133546a to
ff3ff8d
Compare
Adds NodeSuspensionProvider interface to config-provisioning and wires it into DeploymentMetricsRetriever so suspended nodes are excluded from metrics collection. ApplicationRepository injects the provider via DI. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
bjormel
force-pushed
the
bjormel/skip-suspended-hosts-in-deployment-metrics
branch
from
ff3ff8d to
4d932b5
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
NodeSuspensionProviderinterface toconfig-provisioningfor querying suspended hosts per applicationDeploymentMetricsRetrieverso suspended nodes are excluded from metrics collectionApplicationRepositoryvia DI, defaulting to a no-op implementationTest plan
DeploymentMetricsRetrieverTestcovers the filtering behaviour🤖 Generated with Claude Code