Phase 0 for OpenMetrics flow: Inspect Endpoint#23848
Conversation
luisorofino
changed the title
This comment has been minimized.
This comment has been minimized.
luisorofino
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7695ec2f13
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if flow_phases_dir.is_dir(): | ||
| ai_root = Path(__file__).parent.parent | ||
| try: | ||
| rel = flow_phases_dir.relative_to(ai_root) |
There was a problem hiding this comment.
When callers pass a relative flow_yaml_path for a packaged flow, flow_phases_dir is also relative but ai_root comes from __file__ and is absolute, so this relative_to() call raises FlowConfigError even though the phases/ directory is actually under ddev.ai. This makes the new OpenMetrics flow fail to initialize depending only on whether the path was supplied as relative or absolute; resolve both paths before doing the containment check/import-prefix calculation.
Useful? React with 👍 / 👎.
| def _parse_exposition(body: str, content_type: str) -> tuple[list[Metric], str]: | ||
| """Parse body with the parser matching content_type. | ||
|
|
||
| Returns (families, exposition_format) where exposition_format is |
There was a problem hiding this comment.
HTTP media types are case-insensitive, but this check only recognizes lowercase application/openmetrics-text. If an endpoint returns a valid OpenMetrics body with a capitalized header such as Application/OpenMetrics-Text; version=1.0.0, the phase falls back to the Prometheus parser, which can reject or miscatalog OpenMetrics-specific features like units/exemplars. Lowercase the media type portion before choosing the parser.
Useful? React with 👍 / 👎.
Codecov Report❌ Patch coverage is Additional details and impacted files🚀 New features to boost your workflow:
|
luisorofino
force-pushed
the
loa/phase0-openmetrics
branch
from
7695ec2 to
e76a198
Compare
luisorofino
force-pushed
the
loa/subagent-tool
branch
from
c6bddc5 to
a5757e5
Compare
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
luisorofino
force-pushed
the
loa/phase0-openmetrics
branch
from
e76a198 to
9ecf0b2
Compare
Validation ReportAll 21 validations passed. Show details
|
* First implementation of Phase0 * Merge phase0 and phase1 into phase0. Add metrics dumping to jsonl. * Raise if flow not in ddev.ai * Check if endpoint returns response bigger than 10MB * Improve test
2 participants
What does this PR do?
Adds Phase 0 (
InspectEndpointPhase) for the OpenMetrics integration-generation flow, plus the orchestrator plumbing needed to support flow-specific phase modules.InspectEndpointPhaseis a fully deterministic (non-agentic) phase that:<phase_id>_metrics.jsonlsidecar next to the checkpoint file — one JSON row per metric family — carryingname,type,help,unit,label_keys, andsample_count. This catalog is the ground-truth input later phases will use for metric renaming,metrics.pymapping, andmetadata.csvgeneration.Orchestrator change –
_discover_and_register_phasesis refactored to accept an explicit(phases_dir, import_prefix)pair. On startup the orchestrator now runs discovery twice: once for the built-in phases underddev.ai.phases, and once for anyphases/subdirectory sitting next to the activeflow.yaml. This lets OpenMetrics (and future flows) ship their own phase modules without touching the core framework.Minor checkpoint fix –
CheckpointManagergains amemory_dirproperty (the directory of the checkpoint file) so phases can resolve sidecar paths without re-deriving it themselves. Also, keys are now sorted so that chekpoints preserve a natural structure and become clearer to understand.Dependency –
prometheus-clientis added as an optional[ai]dependency inpyproject.toml; thehatch.tomldefault environment now installs this extra so all AI tests run with the full dependency set.Motivation
Phase 0 is the mandatory first step of the OpenMetrics flow: before any agent can propose a configuration, the framework needs a structured, exhaustive catalog of every metric family the endpoint actually exposes. A deterministic phase is ideal here — no LLM is needed and the output is fully reproducible, keeping token costs at zero for this step.
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