torchserve: add container-based config discovery support - #24609
torchserve: add container-based config discovery support#24609martavicentenavarro wants to merge 8 commits into
Conversation
Adds a discovery stanza to the spec (one from_ports strategy per mode — OpenMetrics, Inference API, Management API — since each port maps to exactly one candidate config), wires the E2E fixture to expose auto_conf.yaml/container labels to the Agent, and adds a combined discovery E2E test alongside a stability check across all candidates. Also fixes _get_instance_option_names in datadog_checks_dev's spec validator: it only read top-level instance group names, so specs using multiple_instances_defined together with a discovery stanza (like this one) failed validation because the validator couldn't see the actual field names nested under each instance mode. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 19ff436 | Docs | Datadog PR Page | Give us feedback! |
Add container-based config discovery for TorchServe, restricted to its OpenMetrics endpoint (port 8082): - Generate an `openmetrics_endpoint` candidate from discovered ports, with 8082 preferred as the configured port hint. - Exclude TorchServe's known non-metrics ports (8080 inference API, 8081 management API) from candidate generation, while still falling back to other exposed ports in case OpenMetrics is served on a custom port. - Default the openmetrics/inference/management endpoint options to `None` instead of a hardcoded host:port, so discovered instances don't collide with a stale default. - Fix `_get_instance_option_names` in datadog_checks_dev's spec validator to unwrap `multiple_instances_defined` mode groups by exactly one level instead of recursing arbitrarily, which the new discovery config triggered. - Add unit tests for the discovery candidate generation (including the custom-port fallback and candidate ordering) and E2E coverage exercising discovery end-to-end.
a08ee03 to
d8fae0e
Compare
The comment and changelog entry incorrectly implied both the Inference and Management APIs submit only service checks. Management actually submits gauge metrics; the reason it isn't discovered is that discovery is scoped to OpenMetrics only.
…llback The TorchServe image exposes gRPC ports 7070/7071 in addition to the HTTP 8080/8081/8082 ports, even though the test compose file only publishes the latter three. The custom-port fallback treated 7070/7071 as OpenMetrics candidates, and probing a gRPC port over HTTP logs container-side errors, which failed test_e2e_discovery_all_candidates in CI. Exclude them alongside the HTTP Inference/Management ports.
The README had no mention of the new discovery support, so a customer relying on autodiscovery had no signal that only the OpenMetrics endpoint is auto-configured, or that metrics_mode must be set to prometheus for discovery to work at all. Document both, matching the style already used on gitlab_runner's README for its own discovery scoping note. Environment: Datadog workspace Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b3f8a80ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…'s default param Codex flagged the untyped default parameter as violating AGENTS.md's type-hinting rule for newly generated code. Typed it as the callable it actually is (matches _generated_candidates' own signature). Environment: Datadog workspace Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
|
||
| Follow the instructions below to install and configure this check for an Agent running on a host. For containerized environments, see the [Autodiscovery Integration Templates][3] for guidance on applying these instructions. | ||
|
|
||
| #### Container-based configuration discovery |
There was a problem hiding this comment.
| #### Container-based configuration discovery | |
| ### Container-based configuration discovery |
|
|
||
| #### Container-based configuration discovery | ||
|
|
||
| This check supports [Autodiscovery][3] based on containers exposing TorchServe's OpenMetrics endpoint (default port `8082`). Discovery only generates an `openmetrics_endpoint` instance; the Inference API and Management API endpoints cannot be derived automatically and remain manually configurable, as described below. Discovery also requires `metrics_mode` to be set to `prometheus` in `config.properties`, since TorchServe defaults to `log` mode, which the Agent cannot scrape. |
There was a problem hiding this comment.
| This check supports [Autodiscovery][3] based on containers exposing TorchServe's OpenMetrics endpoint (default port `8082`). Discovery only generates an `openmetrics_endpoint` instance; the Inference API and Management API endpoints cannot be derived automatically and remain manually configurable, as described below. Discovery also requires `metrics_mode` to be set to `prometheus` in `config.properties`, since TorchServe defaults to `log` mode, which the Agent cannot scrape. | |
| This check supports [Autodiscovery][3] based on containers exposing TorchServe's OpenMetrics endpoint (default port `8082`). Discovery only generates an `openmetrics_endpoint` instance; Autodiscovery cannot derive the Inference API and Management API endpoints, configure those manually using the steps in the [Configuration](#configuration) section. Discovery also requires `metrics_mode` to be set to `prometheus` in `config.properties`, since TorchServe defaults to `log` mode, which the Agent cannot scrape. |
Validation ReportAll 21 validations passed. Show details
|
What does this PR do?
Adds container-based config discovery to torchserve, scoped to the OpenMetrics endpoint.
_get_instance_option_namesindatadog_checks_dev's spec validator, which didn't recurse intomultiple_instances_defined's nested option groups (torchserve's three instance modes). Bundled here since it's what unblocked this PR; the fix is general and covers any integration with this spec shape.Motivation
https://datadoghq.atlassian.net/browse/DSCVR-556
Review checklist (to be filled by reviewers)
qa/requiredif this PR needs QA validation, orqa/skip-qaif it does not. Exactly one of the two is required.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged