Skip to content

Commit 293d27b

Browse files
committed
feat: support structured advisories and log diagnostics
1 parent 0be14ab commit 293d27b

17 files changed

Lines changed: 451 additions & 57 deletions

‎README.md‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,10 @@ spec:
152152
duration_seconds: 60
153153
interval_seconds: 10
154154
report_name: pg18-mixed-diagnostics
155+
# log_depth_time_min: 15 # optional: parse server csvlog for the
156+
# last N minutes (0-1440); needs
157+
# collection_mode local or remote, since
158+
# remote-db-only never reaches the log dir
155159
benchmark: # optional
156160
database: pg_perf_bench_test # dedicated and disposable
157161
report_name: pg18-mixed-benchmark
@@ -470,7 +474,9 @@ inventory items. It stores both the original diagnostic report and a compact
470474

471475
`generate_configuration_candidate` combines those observed facts with the
472476
explicit database duty, storage class, replication mode, and PITR intent. The
473-
result remains a `pg_configurator/v1` candidate. Finally,
477+
result is a `pg_configurator/v1` or `pg_configurator/v2` candidate; the two
478+
differ in how the candidate reports its own findings, not in the parameter
479+
mappings this workflow reads. Finally,
474480
`compare_configuration_candidate` writes JSON and Markdown containing only
475481
changed or unobserved parameters, including current source, apply mode,
476482
pending-restart state, calculation rule, and warnings.
@@ -520,9 +526,16 @@ unknown step or changed core artifact blocks recovery.
520526

521527
## Determinism and safety
522528

523-
- Every component returns the exact `pg_play/component/v1` envelope in hidden
524-
machine mode and advertises `pg_play/capabilities/v1` through the common
525-
`--component-capabilities` flag.
529+
- Every component returns an exact component envelope in hidden machine mode
530+
and advertises `pg_play/capabilities/v1` through the common
531+
`--component-capabilities` flag. Two envelope versions are accepted while the
532+
components migrate: `pg_play/component/v1` carries `warnings`, a list of
533+
sentences, and `pg_play/component/v2` carries `advisories`, objects with a
534+
stable `code`, a `severity` of `warning`, `assumption` or `info`, and the
535+
`setting` each one is about. Neither may carry the other's field. Anything
536+
read through `envelope_advisories` arrives in the v2 shape either way; a v1
537+
sentence becomes a `warning` with a null `code`, because that is all v1 can
538+
express.
526539
- Plans hash normalized configuration, workload profile contents, scheduler
527540
state, and current stand state.
528541
- Parameters owned by stand topology, TLS, fixed CSV logging, or diagnostic

‎pyproject.toml‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
44

55
[project]
66
name = "pg-play"
7-
version = "0.3.0"
7+
version = "0.4.0"
88
description = "MCP orchestrator for reproducible PostgreSQL performance experiments"
99
readme = "README.md"
1010
requires-python = ">=3.10"
@@ -30,12 +30,12 @@ classifiers = [
3030
]
3131
dependencies = [
3232
"mcp>=1.28,<2",
33-
"pg-configurator>=0.9.2,<1",
33+
"pg-configurator>=0.11.0,<1",
3434
"pg-converter>=2.0b2,<3",
35-
"pg-diag>=0.10.5,<1",
36-
"pg-perf-bench>=0.2.1,<1",
35+
"pg-diag>=0.12.4,<1",
36+
"pg-perf-bench>=0.2.2,<1",
3737
"pg-stand>=0.2.2,<1",
38-
"pg-workload>=0.4.1,<1",
38+
"pg-workload>=0.4.2,<1",
3939
"PyYAML>=6.0,<7",
4040
]
4141

‎src/pg_play/configuration_review.py‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
load_configuration_facts,
1818
)
1919

20-
from pg_play.contract import canonical_hash
20+
from pg_play.contract import canonical_hash, envelope_messages
2121
from pg_play.state import write_json, write_text
2222

2323
REVIEW_ID_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_.-]{0,63}$")
@@ -363,9 +363,20 @@ def _values_equal(current: dict[str, Any], candidate: Any) -> bool:
363363
return _normalized_text(current.get("value")) == _normalized_text(candidate)
364364

365365

366+
# v2 renamed the artifact's `warnings` to `advisories` and gave each one a
367+
# severity; the parameter mappings this module reads are unchanged, so both are
368+
# accepted and the findings are read through envelope_messages.
369+
CONFIGURATOR_ARTIFACT_SCHEMAS = ("pg_configurator/v1", "pg_configurator/v2")
370+
371+
366372
def validate_configuration_candidate(candidate: Any) -> dict[str, Any]:
367-
if not isinstance(candidate, dict) or candidate.get("schema_version") != "pg_configurator/v1":
368-
raise ConfigurationReviewError("candidate must be a pg_configurator/v1 artifact")
373+
if (
374+
not isinstance(candidate, dict)
375+
or candidate.get("schema_version") not in CONFIGURATOR_ARTIFACT_SCHEMAS
376+
):
377+
raise ConfigurationReviewError(
378+
"candidate must be a {} artifact".format(" or ".join(CONFIGURATOR_ARTIFACT_SCHEMAS))
379+
)
369380
claimed_hash = candidate.get("artifact_hash")
370381
if not isinstance(claimed_hash, str) or claimed_hash != configurator_artifact_hash(candidate):
371382
raise ConfigurationReviewError("pg_configurator candidate hash does not match its content")
@@ -450,7 +461,7 @@ def compare_configuration(
450461
"unchanged_parameter_count": unchanged,
451462
"apply_mode_counts": dict(sorted(action_counts.items())),
452463
},
453-
"warnings": list(candidate.get("warnings") or []),
464+
"warnings": envelope_messages(candidate),
454465
}
455466
result["comparison_hash"] = canonical_hash(result)
456467
return result

‎src/pg_play/contract.py‎

Lines changed: 67 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,15 @@
66
import json
77
from typing import Any
88

9+
# Two component contracts are live. They differ in one field: v1 reports
10+
# `warnings`, a list of sentences, and v2 reports `advisories`, a list of
11+
# objects carrying a stable code, a severity and the setting each one is about.
12+
# Components migrate one at a time, so both are accepted and `envelope_advisories`
13+
# below gives a caller one shape to work with either way.
914
CONTRACT_VERSION = "pg_play/component/v1"
15+
CONTRACT_VERSION_V2 = "pg_play/component/v2"
16+
SUPPORTED_CONTRACT_VERSIONS = (CONTRACT_VERSION, CONTRACT_VERSION_V2)
17+
ADVISORY_SEVERITIES = ("warning", "assumption", "info")
1018
CAPABILITY_SCHEMA_VERSION = "pg_play/capabilities/v1"
1119
MACHINE_INTERFACE = {
1220
"machine_flag": "--machine",
@@ -58,6 +66,10 @@ def canonical_hash(value: Any) -> str:
5866
def validate_envelope(value: Any, *, expected_component: str | None = None) -> dict[str, Any]:
5967
if not isinstance(value, dict):
6068
raise ContractError("component response must be a JSON object")
69+
contract_version = value.get("contract_version")
70+
if contract_version not in SUPPORTED_CONTRACT_VERSIONS:
71+
raise ContractError(f"unsupported component contract: {contract_version!r}")
72+
findings_field = "advisories" if contract_version == CONTRACT_VERSION_V2 else "warnings"
6173
required = {
6274
"contract_version",
6375
"component",
@@ -67,15 +79,13 @@ def validate_envelope(value: Any, *, expected_component: str | None = None) -> d
6779
"status",
6880
"result",
6981
"artifacts",
70-
"warnings",
82+
findings_field,
7183
"error",
7284
}
7385
if set(value) != required:
7486
missing = sorted(required.difference(value))
7587
extra = sorted(set(value).difference(required))
7688
raise ContractError(f"invalid component envelope fields: missing={missing}, extra={extra}")
77-
if value["contract_version"] != CONTRACT_VERSION:
78-
raise ContractError(f"unsupported component contract: {value['contract_version']!r}")
7989
component = value["component"]
8090
if component not in COMPONENTS:
8191
raise ContractError(f"unknown component in envelope: {component!r}")
@@ -93,10 +103,13 @@ def validate_envelope(value: Any, *, expected_component: str | None = None) -> d
93103
isinstance(artifact, dict) for artifact in value["artifacts"]
94104
):
95105
raise ContractError("component artifacts must be a list of objects")
96-
if not isinstance(value["warnings"], list) or not all(
97-
isinstance(warning, str) for warning in value["warnings"]
98-
):
99-
raise ContractError("component warnings must be a list of strings")
106+
if findings_field == "warnings":
107+
if not isinstance(value["warnings"], list) or not all(
108+
isinstance(warning, str) for warning in value["warnings"]
109+
):
110+
raise ContractError("component warnings must be a list of strings")
111+
else:
112+
_validate_advisories(value["advisories"])
100113
if value["error"] is not None and not isinstance(value["error"], dict):
101114
raise ContractError("component error must be an object or null")
102115
if isinstance(value["error"], dict) and not {
@@ -107,6 +120,52 @@ def validate_envelope(value: Any, *, expected_component: str | None = None) -> d
107120
return value
108121

109122

123+
def _validate_advisories(advisories: Any) -> None:
124+
if not isinstance(advisories, list):
125+
raise ContractError("component advisories must be a list of objects")
126+
for advisory in advisories:
127+
if not isinstance(advisory, dict):
128+
raise ContractError("component advisories must be a list of objects")
129+
missing = sorted({"code", "severity", "setting", "actual", "message"}.difference(advisory))
130+
if missing:
131+
raise ContractError(f"component advisory missing fields: {missing}")
132+
if advisory["severity"] not in ADVISORY_SEVERITIES:
133+
raise ContractError(f"unknown advisory severity: {advisory['severity']!r}")
134+
for field in ("code", "message"):
135+
if not isinstance(advisory[field], str) or not advisory[field]:
136+
raise ContractError(f"component advisory {field} must be a non-empty string")
137+
for field in ("setting", "actual"):
138+
if advisory[field] is not None and not isinstance(advisory[field], str):
139+
raise ContractError(f"component advisory {field} must be a string or null")
140+
141+
142+
def envelope_advisories(envelope: dict[str, Any]) -> list[dict[str, Any]]:
143+
"""The findings in an envelope, in the v2 shape whichever version sent it.
144+
145+
A v1 component reports sentences with no severity and no stable identifier,
146+
so each one is lifted to `severity: "warning"`: that is the only thing v1
147+
can express, and pretending to know more would be an invention. `code` stays
148+
None, which is how a caller tells a lifted line from a real advisory.
149+
"""
150+
if "advisories" in envelope:
151+
return [dict(advisory) for advisory in envelope["advisories"]]
152+
return [
153+
{
154+
"code": None,
155+
"severity": "warning",
156+
"setting": None,
157+
"actual": None,
158+
"message": message,
159+
}
160+
for message in envelope.get("warnings") or []
161+
]
162+
163+
164+
def envelope_messages(envelope: dict[str, Any]) -> list[str]:
165+
"""Just the sentences, for a report that has no room for the structure."""
166+
return [advisory["message"] for advisory in envelope_advisories(envelope)]
167+
168+
110169
def validate_capabilities(
111170
value: Any,
112171
*,
@@ -132,7 +191,7 @@ def validate_capabilities(
132191
raise ContractError(
133192
f"unsupported capability schema: {value['capability_schema_version']!r}"
134193
)
135-
if value["contract_version"] != CONTRACT_VERSION:
194+
if value["contract_version"] not in SUPPORTED_CONTRACT_VERSIONS:
136195
raise ContractError(f"unsupported component contract: {value['contract_version']!r}")
137196
if value["component"] != expected_component:
138197
raise ContractError(

‎src/pg_play/manifest.py‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ class DiagnosticSpec:
5454
duration_seconds: float
5555
interval_seconds: float
5656
report_name: str
57+
log_depth_time_min: int | None
5758

5859

5960
@dataclass(frozen=True)
@@ -315,7 +316,14 @@ def load_manifest(path: str | Path) -> ExperimentManifest:
315316
diagnostics_raw = _mapping(
316317
spec.get("diagnostics", {}),
317318
"spec.diagnostics",
318-
{"mode", "collection_mode", "duration_seconds", "interval_seconds", "report_name"},
319+
{
320+
"mode",
321+
"collection_mode",
322+
"duration_seconds",
323+
"interval_seconds",
324+
"report_name",
325+
"log_depth_time_min",
326+
},
319327
)
320328
mode = _text(diagnostics_raw.get("mode", "snapshots"), "spec.diagnostics.mode")
321329
if mode not in {"one-shot", "snapshots"}:
@@ -348,6 +356,16 @@ def load_manifest(path: str | Path) -> ExperimentManifest:
348356
diagnostics_raw.get("report_name", "report"),
349357
"spec.diagnostics.report_name",
350358
),
359+
log_depth_time_min=(
360+
_integer(
361+
diagnostics_raw["log_depth_time_min"],
362+
"spec.diagnostics.log_depth_time_min",
363+
minimum=0,
364+
maximum=1440,
365+
)
366+
if diagnostics_raw.get("log_depth_time_min") is not None
367+
else None
368+
),
351369
)
352370
if diagnostics.mode == "snapshots":
353371
window_error = validate_snapshots_window(
@@ -356,6 +374,14 @@ def load_manifest(path: str | Path) -> ExperimentManifest:
356374
)
357375
if window_error is not None:
358376
raise ManifestError(window_error)
377+
if diagnostics.log_depth_time_min and diagnostics.collection_mode == "remote-db-only":
378+
# pg_diag reads csvlog files from the server host, which remote-db-only
379+
# never touches; it would skip the section and the requested evidence
380+
# would silently be missing from the reviewed plan.
381+
raise ManifestError(
382+
"spec.diagnostics.log_depth_time_min needs collection_mode local or remote: "
383+
"remote-db-only has no access to the server log directory"
384+
)
359385
if not _PROFILE_RE.fullmatch(diagnostics.report_name):
360386
raise ManifestError(f"spec.diagnostics.report_name must match {_PROFILE_RE.pattern}")
361387

‎src/pg_play/mcp_server.py‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
from mcp.server.fastmcp import FastMCP
1010

11+
from pg_play.contract import ADVISORY_SEVERITIES, SUPPORTED_CONTRACT_VERSIONS
1112
from pg_play.service import PgPlayService
1213

1314
mcp = FastMCP(
@@ -364,9 +365,20 @@ def run_event_schema() -> str:
364365
@mcp.resource("pgplay://component-contract")
365366
def component_contract() -> str:
366367
"""Machine envelope fields expected from every component."""
368+
common_fields = [
369+
"contract_version",
370+
"component",
371+
"component_version",
372+
"command",
373+
"request_id",
374+
"status",
375+
"result",
376+
"artifacts",
377+
"error",
378+
]
367379
return json.dumps(
368380
{
369-
"contract_version": "pg_play/component/v1",
381+
"contract_versions": list(SUPPORTED_CONTRACT_VERSIONS),
370382
"capability_schema_version": "pg_play/capabilities/v1",
371383
"capability_command_fields": [
372384
"mutates_target",
@@ -378,18 +390,14 @@ def component_contract() -> str:
378390
"request_id_option": "--request-id",
379391
"capabilities_option": "--component-capabilities",
380392
},
381-
"required_fields": [
382-
"contract_version",
383-
"component",
384-
"component_version",
385-
"command",
386-
"request_id",
387-
"status",
388-
"result",
389-
"artifacts",
390-
"warnings",
391-
"error",
392-
],
393+
# The versions differ in one field. v1 reports `warnings`, a list of
394+
# sentences; v2 reports `advisories`, objects with a stable code, a
395+
# severity of warning/assumption/info, and the setting each is about.
396+
"required_fields": {
397+
"pg_play/component/v1": sorted([*common_fields, "warnings"]),
398+
"pg_play/component/v2": sorted([*common_fields, "advisories"]),
399+
},
400+
"advisory_severities": list(ADVISORY_SEVERITIES),
393401
},
394402
indent=2,
395403
sort_keys=True,

‎src/pg_play/schema/pg_play-v1.schema.json‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@
7777
"collection_mode": {"enum": ["remote-db-only", "local", "remote"]},
7878
"duration_seconds": {"type": "number", "minimum": 30, "maximum": 86400},
7979
"interval_seconds": {"type": "number", "minimum": 5, "maximum": 600},
80-
"report_name": {"type": "string", "pattern": "^[A-Za-z0-9][A-Za-z0-9_.-]*$"}
80+
"report_name": {"type": "string", "pattern": "^[A-Za-z0-9][A-Za-z0-9_.-]*$"},
81+
"log_depth_time_min": {"type": "integer", "minimum": 0, "maximum": 1440}
8182
},
8283
"allOf": [
8384
{
@@ -90,6 +91,16 @@
9091
]
9192
}
9293
}
94+
},
95+
{
96+
"if": {
97+
"properties": {"log_depth_time_min": {"minimum": 1}},
98+
"required": ["log_depth_time_min"]
99+
},
100+
"then": {
101+
"properties": {"collection_mode": {"enum": ["local", "remote"]}},
102+
"required": ["collection_mode"]
103+
}
93104
}
94105
]
95106
},

0 commit comments

Comments
 (0)