Skip to content

Add config discovery support to envoy#24562

Draft
martavicentenavarro wants to merge 6 commits into
masterfrom
vicentenavarrom/envoy-discovery-v2
Draft

Add config discovery support to envoy#24562
martavicentenavarro wants to merge 6 commits into
masterfrom
vicentenavarrom/envoy-discovery-v2

Conversation

@martavicentenavarro

@martavicentenavarro martavicentenavarro commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds config discovery support to the Envoy integration: a from_ports discovery strategy on port 8001 with two candidates covering both the OpenMetrics (openmetrics_endpoint) and legacy (stats_url) dispatch modes, regenerated config models, auto_conf.yaml, get_e2e_discovery_metadata() wiring in the E2E fixture, and new test_e2e_discovery / test_e2e_discovery_all_candidates E2E tests.

Known limitation: the legacy /stats candidate is restricted (via discovery_overrides.py) to the hinted admin port (8001), since probing arbitrary exposed ports with /server_info + /stats risks misidentifying an unrelated upstream as Envoy's admin endpoint. Envoy deployments exposing the admin interface on a different port won't be discovered via the legacy path and still require a hand-written static config. openmetrics_endpoint keeps the default fallback across all candidate ports.

Motivation

Part of the config discovery rollout (DSCVR-484). This is a fresh implementation built with the restructured create-discovery-pr skill, superseding #24121 which was built with the older version of the workflow.

Related work: #24121 (older implementation) has been closed in favor of this PR.

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Add qa/required if this PR needs QA validation, or qa/skip-qa if it does not. Exactly one of the two is required.
  • If you need to backport this PR to another branch, you can add the backport/<branch-name> label to the PR and it will automatically open a backport PR once this one is merged

Adds a from_ports discovery strategy covering both the OpenMetrics and
legacy stats_url dispatch modes on port 8001, regenerates config
models, wires E2E discovery metadata, and adds discovery E2E tests.
@datadog-datadog-prod-us1-2

datadog-datadog-prod-us1-2 Bot commented Jul 15, 2026

Copy link
Copy Markdown

Tests  Code Coverage

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 70.07% (-18.22%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: e156d2c | Docs | Datadog PR Page | Give us feedback!

@martavicentenavarro martavicentenavarro self-assigned this Jul 16, 2026
martavicentenavarro and others added 2 commits July 16, 2026 10:55
candidate_ports() yields every exposed port (hints only reorder), so
the generated stats_url candidate was being probed against any port
on the container, not just Envoy's admin port. The legacy check also
calls /server_info before scraping /stats, risking hitting an
unrelated upstream and misidentifying it as Envoy's admin endpoint.
InstanceConfig's field validator fills any unconfigured field with its
default, so a strategy with more than one candidate (e.g. envoy's
openmetrics_endpoint/stats_url pair) ended up with every candidate
carrying a phantom default for the field it never set. That corrupted
envoy's legacy-vs-modern dispatch, which keys off field presence.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
martavicentenavarro and others added 2 commits July 16, 2026 12:10
…rride

Codex review flagged that restricting stats_url to port 8001 means
non-default admin ports won't be autodiscovered via the legacy path.
Accepted as a known limitation for now, matching the integration's
documented default admin port.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
8001 is the port used by this integration's test environment and
discovery strategy hint, not an Envoy-documented default (upstream
docs use 9901 as the example admin port).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@dd-octo-sts

dd-octo-sts Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Validation Report

All 21 validations passed.

Show details
Validation Description Status
agent-reqs Verify check versions match the Agent requirements file
ci Validate CI configuration and code coverage settings
codeowners Validate every integration has a CODEOWNERS entry
config Validate default configuration files against spec.yaml
dep Verify dependency pins are consistent and Agent-compatible
http Validate integrations use the HTTP wrapper correctly
imports Validate check imports do not use deprecated modules
integration-style Validate check code style conventions
jmx-metrics Validate JMX metrics definition files and config
labeler Validate PR labeler config matches integration directories
legacy-signature Validate no integration uses the legacy Agent check signature
license-headers Validate Python files have proper license headers
licenses Validate third-party license attribution list
metadata Validate metadata.csv metric definitions
models Validate configuration data models match spec.yaml
openmetrics Validate OpenMetrics integrations disable the metric limit
package Validate Python package metadata and naming
qa-label Validate the pull request declares whether it needs QA for the next Agent release
readmes Validate README files have required sections
saved-views Validate saved view JSON file structure and fields
version Validate version consistency between package and changelog

View full run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant