fix(reporting): resolve unified resource IDs to metrics targets and render honest report subjects

Performance reports were structurally disconnected from the v6 ID
space: the UI (and any API caller working from /api/state) addresses
resources by canonical unified ID, while the metrics store is keyed by
each platform's native source ID (the resource's metricsTarget). The
engine queried the store with the unified ID verbatim, so every report
rendered 'Data Points: 0' regardless of how much history existed, and
covers showed raw hash IDs a report reader cannot map to a machine.

- MetricReportRequest gains MetricsResourceID: handlers resolve the
  unified ID through the tenant monitor's resource store (new
  Monitor.MetricsTargetForResource accessor; the registry computes
  targets on demand, they are not persisted on snapshot structs) and
  the engine uses it for store queries only. Recovery points and
  Patrol findings stay keyed by the unified ID.
- Legacy snapshot models and their alerts are keyed by the metrics
  target ID, so enrichment now matches either ID space and resource
  names/status resolve again on covers, headers, and fleet rows.
- Fleet summaries mirror the single-report guard: zero data points
  across the fleet renders a muted NO DATA card instead of a green
  HEALTHY 'All systems operating normally' - false reassurance is the
  worst failure mode for a client-facing stability report.
- Em dashes in PDF-bound literals become hyphens; fpdf core fonts are
  cp1252 and rendered them as mojibake.
This commit is contained in:
rcourtman 2026-06-10 17:10:30 +01:00
parent 3badd0a840
commit e6c0c4d385
8 changed files with 268 additions and 19 deletions

View file

@ -635,6 +635,7 @@ func sanitizeFilename(s string) string {
// enrichReportRequest populates enrichment data from the monitor state
func (h *ReportingHandlers) enrichReportRequest(ctx context.Context, orgID string, req *reporting.MetricReportRequest, snapshot reportingEnrichmentSnapshot, start, end time.Time) {
h.resolveReportSubject(orgID, req, snapshot)
switch req.ResourceType {
case "node":
h.enrichNodeReport(req, snapshot, start, end)
@ -645,12 +646,71 @@ func (h *ReportingHandlers) enrichReportRequest(ctx context.Context, orgID strin
}
}
// resolveReportSubject maps the request's canonical unified-resource ID onto
// the metrics-store ID space and seeds a display name. The v6 UI and API
// callers address resources by unified ID (`vm-<hash>`), while the metrics
// store is keyed by each platform's native source ID (the resource's
// metricsTarget) — without this translation the store query silently
// returns zero points. Recovery points and Patrol findings stay keyed by
// the unified ID, so the translation rides MetricsResourceID instead of
// rewriting ResourceID.
func (h *ReportingHandlers) resolveReportSubject(orgID string, req *reporting.MetricReportRequest, snapshot reportingEnrichmentSnapshot) {
if req == nil {
return
}
for i := range snapshot.Resources {
resource := &snapshot.Resources[i]
if resource.ID != req.ResourceID {
continue
}
// The registry computes metrics targets on demand; the Resource
// structs in the snapshot only carry one when a fixture populated
// it explicitly, so ask the tenant monitor first and fall back to
// the struct field.
var target *unifiedresources.MetricsTarget
if h != nil && h.mtMonitor != nil {
if monitor, err := h.mtMonitor.GetMonitor(orgID); err == nil && monitor != nil {
target = monitor.MetricsTargetForResource(req.ResourceID)
}
}
if target == nil {
target = resource.MetricsTarget
}
if target != nil {
if targetID := strings.TrimSpace(target.ResourceID); targetID != "" {
req.MetricsResourceID = targetID
}
}
if req.Resource == nil && strings.TrimSpace(resource.Name) != "" {
req.Resource = &reporting.ResourceInfo{
Name: resource.Name,
Status: string(resource.Status),
}
}
return
}
}
// reportSubjectIDMatches reports whether a legacy state-model ID refers to
// the report subject. Legacy snapshots (and the alerts raised on them) are
// keyed by the metrics-target ID, while the request carries the canonical
// unified ID, so both are accepted.
func reportSubjectIDMatches(req *reporting.MetricReportRequest, candidateID string) bool {
if req == nil || candidateID == "" {
return false
}
if candidateID == req.ResourceID {
return true
}
return req.MetricsResourceID != "" && candidateID == req.MetricsResourceID
}
// enrichNodeReport adds node-specific data to the report request
func (h *ReportingHandlers) enrichNodeReport(req *reporting.MetricReportRequest, snapshot reportingEnrichmentSnapshot, start, end time.Time) {
// Find the node
var node *models.Node
for i := range snapshot.Nodes {
if snapshot.Nodes[i].ID == req.ResourceID {
if reportSubjectIDMatches(req, snapshot.Nodes[i].ID) {
node = &snapshot.Nodes[i]
break
}
@ -685,7 +745,7 @@ func (h *ReportingHandlers) enrichNodeReport(req *reporting.MetricReportRequest,
// Find alerts for this node
for _, alert := range snapshot.ActiveAlerts {
if alert.ResourceID == req.ResourceID || alert.Node == node.Name {
if reportSubjectIDMatches(req, alert.ResourceID) || alert.Node == node.Name {
req.Alerts = append(req.Alerts, reporting.AlertInfo{
Type: alert.Type,
Level: alert.Level,
@ -697,7 +757,7 @@ func (h *ReportingHandlers) enrichNodeReport(req *reporting.MetricReportRequest,
}
}
for _, resolved := range snapshot.RecentlyResolved {
if (resolved.ResourceID == req.ResourceID || resolved.Node == node.Name) &&
if (reportSubjectIDMatches(req, resolved.ResourceID) || resolved.Node == node.Name) &&
resolved.ResolvedTime.After(start) && resolved.ResolvedTime.Before(end) {
resolvedTime := resolved.ResolvedTime
req.Alerts = append(req.Alerts, reporting.AlertInfo{
@ -789,7 +849,7 @@ func (h *ReportingHandlers) enrichVMReport(ctx context.Context, orgID string, re
// Find the VM
var vm *models.VM
for i := range snapshot.VMs {
if snapshot.VMs[i].ID == req.ResourceID {
if reportSubjectIDMatches(req, snapshot.VMs[i].ID) {
vm = &snapshot.VMs[i]
break
}
@ -816,7 +876,7 @@ func (h *ReportingHandlers) enrichVMReport(ctx context.Context, orgID string, re
// Find alerts for this VM
for _, alert := range snapshot.ActiveAlerts {
if alert.ResourceID == req.ResourceID {
if reportSubjectIDMatches(req, alert.ResourceID) {
req.Alerts = append(req.Alerts, reporting.AlertInfo{
Type: alert.Type,
Level: alert.Level,
@ -828,7 +888,7 @@ func (h *ReportingHandlers) enrichVMReport(ctx context.Context, orgID string, re
}
}
for _, resolved := range snapshot.RecentlyResolved {
if resolved.ResourceID == req.ResourceID &&
if reportSubjectIDMatches(req, resolved.ResourceID) &&
resolved.ResolvedTime.After(start) && resolved.ResolvedTime.Before(end) {
resolvedTime := resolved.ResolvedTime
req.Alerts = append(req.Alerts, reporting.AlertInfo{
@ -1022,7 +1082,7 @@ func (h *ReportingHandlers) enrichContainerReport(ctx context.Context, orgID str
// Find the container
var ct *models.Container
for i := range snapshot.Containers {
if snapshot.Containers[i].ID == req.ResourceID {
if reportSubjectIDMatches(req, snapshot.Containers[i].ID) {
ct = &snapshot.Containers[i]
break
}
@ -1048,7 +1108,7 @@ func (h *ReportingHandlers) enrichContainerReport(ctx context.Context, orgID str
// Find alerts for this container
for _, alert := range snapshot.ActiveAlerts {
if alert.ResourceID == req.ResourceID {
if reportSubjectIDMatches(req, alert.ResourceID) {
req.Alerts = append(req.Alerts, reporting.AlertInfo{
Type: alert.Type,
Level: alert.Level,
@ -1060,7 +1120,7 @@ func (h *ReportingHandlers) enrichContainerReport(ctx context.Context, orgID str
}
}
for _, resolved := range snapshot.RecentlyResolved {
if resolved.ResourceID == req.ResourceID &&
if reportSubjectIDMatches(req, resolved.ResourceID) &&
resolved.ResolvedTime.After(start) && resolved.ResolvedTime.Before(end) {
resolvedTime := resolved.ResolvedTime
req.Alerts = append(req.Alerts, reporting.AlertInfo{

View file

@ -914,3 +914,60 @@ func TestBuildVMInventoryRows_UsesCanonicalFieldsAndDiskFallback(t *testing.T) {
t.Fatalf("expected disk status reason to be preserved, got %+v", row)
}
}
// TestResolveReportSubject_TranslatesUnifiedIDToMetricsTarget pins the
// unified-ID → metrics-target translation. The v6 UI addresses report
// subjects by canonical unified ID while the metrics store is keyed by the
// platform-native source ID carried in the resource's metricsTarget;
// without the translation every report rendered "Data Points: 0".
func TestResolveReportSubject_TranslatesUnifiedIDToMetricsTarget(t *testing.T) {
handlers := NewReportingHandlers(nil, nil)
snapshot := reportingEnrichmentSnapshot{
Resources: []unifiedresources.Resource{
{
ID: "vm-7f8b2b6cd98c2089",
Name: "checkout-web-01",
Type: unifiedresources.ResourceTypeVM,
MetricsTarget: &unifiedresources.MetricsTarget{
ResourceType: "vm",
ResourceID: "mock-cluster-pve1-100",
},
},
},
}
req := reporting.MetricReportRequest{
ResourceType: "vm",
ResourceID: "vm-7f8b2b6cd98c2089",
}
handlers.resolveReportSubject("default", &req, snapshot)
if req.MetricsResourceID != "mock-cluster-pve1-100" {
t.Fatalf("MetricsResourceID = %q, want mock-cluster-pve1-100", req.MetricsResourceID)
}
if req.ResourceID != "vm-7f8b2b6cd98c2089" {
t.Fatalf("ResourceID must stay canonical, got %q", req.ResourceID)
}
if req.Resource == nil || req.Resource.Name != "checkout-web-01" {
t.Fatalf("expected display name seeded from unified resource, got %+v", req.Resource)
}
// Legacy snapshot models and alerts are keyed by the metrics-target ID;
// both ID spaces must match the subject.
if !reportSubjectIDMatches(&req, "vm-7f8b2b6cd98c2089") {
t.Fatal("expected canonical unified ID to match subject")
}
if !reportSubjectIDMatches(&req, "mock-cluster-pve1-100") {
t.Fatal("expected metrics-target ID to match subject")
}
if reportSubjectIDMatches(&req, "mock-cluster-pve1-101") {
t.Fatal("unrelated ID must not match subject")
}
// Unknown resources pass through untouched.
unknown := reporting.MetricReportRequest{ResourceType: "vm", ResourceID: "vm-missing"}
handlers.resolveReportSubject("default", &unknown, snapshot)
if unknown.MetricsResourceID != "" || unknown.Resource != nil {
t.Fatalf("expected no translation for unknown resource, got %+v", unknown)
}
}

View file

@ -4101,6 +4101,31 @@ func (m *Monitor) GetUnifiedResources() []unifiedresources.Resource {
return store.GetAll()
}
// MetricsTargetForResource resolves the metrics-store target for a canonical
// unified resource ID via the wired resource store. The metrics store is
// keyed by platform-native source IDs, and the target is computed on demand
// by the resource registry rather than persisted on the Resource structs
// GetUnifiedResources returns. Returns nil when no store is wired, the
// store cannot resolve targets, or the resource has none.
func (m *Monitor) MetricsTargetForResource(resourceID string) *unifiedresources.MetricsTarget {
if m == nil {
return nil
}
m.mu.RLock()
store := m.resourceStore
m.mu.RUnlock()
if store == nil {
return nil
}
resolver, ok := store.(MetricsTargetResourceStore)
if !ok {
return nil
}
return resolver.MetricsTargetForResource(resourceID)
}
type monitorUnifiedStateView struct {
resources []unifiedresources.Resource
readState unifiedresources.ReadState

View file

@ -291,13 +291,18 @@ func (e *ReportEngine) queryMetrics(req MetricReportRequest) (*ReportData, error
storeTypes = []string{canonicalType}
}
queryID := strings.TrimSpace(req.MetricsResourceID)
if queryID == "" {
queryID = req.ResourceID
}
var metricsMap map[string][]metrics.MetricPoint
if req.MetricType != "" {
// Query specific metric, merging across storage aliases during migrations.
var points []metrics.MetricPoint
for _, storeType := range storeTypes {
aliasPoints, queryErr := store.Query(storeType, req.ResourceID, req.MetricType, req.Start, req.End, 0)
aliasPoints, queryErr := store.Query(storeType, queryID, req.MetricType, req.Start, req.End, 0)
if queryErr != nil {
return nil, queryErr
}
@ -310,7 +315,7 @@ func (e *ReportEngine) queryMetrics(req MetricReportRequest) (*ReportData, error
// Query all metrics for the resource, merging across storage aliases.
metricsMap = make(map[string][]metrics.MetricPoint)
for _, storeType := range storeTypes {
aliasMap, queryErr := store.QueryAll(storeType, req.ResourceID, req.Start, req.End, 0)
aliasMap, queryErr := store.QueryAll(storeType, queryID, req.Start, req.End, 0)
if queryErr != nil {
return nil, queryErr
}
@ -322,6 +327,7 @@ func (e *ReportEngine) queryMetrics(req MetricReportRequest) (*ReportData, error
log.Warn().
Str("resourceType", data.ResourceType).
Str("resourceID", req.ResourceID).
Str("metricsQueryID", queryID).
Str("metricType", req.MetricType).
Time("start", req.Start).
Time("end", req.End).

View file

@ -144,9 +144,9 @@ func fleetHeuristicHealthMessage(in FleetNarrativeInput) string {
case a.TotalCriticalAlerts > 1:
return fmt.Sprintf("%d critical alerts across the fleet require immediate attention", a.TotalCriticalAlerts)
case a.TotalActiveAlerts == 1:
return "1 active alert across the fleet review recommended"
return "1 active alert across the fleet - review recommended"
case a.TotalActiveAlerts > 1:
return fmt.Sprintf("%d active alerts across the fleet review recommended", a.TotalActiveAlerts)
return fmt.Sprintf("%d active alerts across the fleet - review recommended", a.TotalActiveAlerts)
case a.ResourceCount == 0:
return "No resources in this fleet report"
default:
@ -204,7 +204,7 @@ func fleetHeuristicOutliers(in FleetNarrativeInput) []FleetOutlier {
outlier: FleetOutlier{
ResourceID: r.ResourceID,
ResourceName: displayResourceName(r),
Reason: fmt.Sprintf("Memory averaging %.1f%% sustained pressure", r.AvgMemory),
Reason: fmt.Sprintf("Memory averaging %.1f%% - sustained pressure", r.AvgMemory),
Severity: NarrativeSeverityWarning,
},
rank: 500 + int(r.AvgMemory),
@ -374,7 +374,7 @@ func fleetHeuristicRecommendations(in FleetNarrativeInput) []string {
recs = append(recs, "Replace failing or end-of-life disks before they cause outage")
}
if len(recs) == 0 {
recs = append(recs, "No fleet-wide action required continue routine monitoring")
recs = append(recs, "No fleet-wide action required - continue routine monitoring")
}
return recs
}

View file

@ -218,6 +218,21 @@ func (g *PDFGenerator) Generate(data *ReportData) ([]byte, error) {
return buf.Bytes(), nil
}
// reportSubjectDisplayName returns the human-readable name for the report
// subject, falling back to the raw resource ID when enrichment did not
// resolve one.
func reportSubjectDisplayName(data *ReportData) string {
if data == nil {
return ""
}
if data.Resource != nil {
if name := strings.TrimSpace(data.Resource.Name); name != "" {
return name
}
}
return data.ResourceID
}
// writeCoverPage creates a professional cover page.
func (g *PDFGenerator) writeCoverPage(pdf *fpdf.Fpdf, data *ReportData) {
pdf.AddPage()
@ -258,7 +273,9 @@ func (g *PDFGenerator) writeCoverPage(pdf *fpdf.Fpdf, data *ReportData) {
pdf.SetDrawColor(colorGridLine[0], colorGridLine[1], colorGridLine[2])
pdf.RoundedRect(boxX, pdf.GetY(), boxWidth, boxHeight, 3, "1234", "FD")
// Resource details inside box
// Resource details inside box. Lead with the human-readable resource
// name when enrichment resolved one; the raw resource ID is operator
// plumbing, not something a report reader can act on.
pdf.SetY(pdf.GetY() + 10)
pdf.SetFont("Arial", "B", 11)
pdf.SetTextColor(colorTextMuted[0], colorTextMuted[1], colorTextMuted[2])
@ -266,7 +283,7 @@ func (g *PDFGenerator) writeCoverPage(pdf *fpdf.Fpdf, data *ReportData) {
pdf.SetFont("Arial", "B", 16)
pdf.SetTextColor(colorTextDark[0], colorTextDark[1], colorTextDark[2])
pdf.CellFormat(0, 10, data.ResourceID, "", 1, "C", false, 0, "")
pdf.CellFormat(0, 10, reportSubjectDisplayName(data), "", 1, "C", false, 0, "")
pdf.SetFont("Arial", "", 11)
pdf.SetTextColor(colorTextMuted[0], colorTextMuted[1], colorTextMuted[2])
@ -707,7 +724,7 @@ func (g *PDFGenerator) addPageHeader(pdf *fpdf.Fpdf, data *ReportData, section s
pdf.SetFont("Arial", "", 9)
pdf.SetTextColor(colorTextMuted[0], colorTextMuted[1], colorTextMuted[2])
pdf.SetXY(20, 18)
pdf.CellFormat(pageWidth-40, 5, data.ResourceID, "", 1, "R", false, 0, "")
pdf.CellFormat(pageWidth-40, 5, reportSubjectDisplayName(data), "", 1, "R", false, 0, "")
// Section title
pdf.SetY(30)
@ -1878,6 +1895,11 @@ func (g *PDFGenerator) writeFleetSummary(pdf *fpdf.Fpdf, data *MultiReportData)
}
}
fleetTotalPoints := 0
for _, rd := range data.Resources {
fleetTotalPoints += rd.TotalPoints
}
if totalCritical > 0 {
healthStatus = "CRITICAL"
healthColor = colorDanger
@ -1886,6 +1908,16 @@ func (g *PDFGenerator) writeFleetSummary(pdf *fpdf.Fpdf, data *MultiReportData)
healthStatus = "WARNING"
healthColor = colorWarning
healthMessage = fmt.Sprintf("%d warnings across fleet", totalWarning)
} else if fleetTotalPoints == 0 {
// No metrics arrived for any resource in the window. Reporting
// HEALTHY would green-light an empty report and tell the reader
// their fleet is fine when Pulse has nothing to evaluate — the
// worst failure mode for a report whose job is reassurance.
// Mirror the single-resource executive summary's muted NO DATA
// card instead.
healthStatus = "NO DATA"
healthColor = colorTextMuted
healthMessage = "No metrics reported during the selected window"
}
// Health status card
@ -2108,7 +2140,7 @@ func writeFleetNarrativeSection(pdf *fpdf.Fpdf, fn *FleetNarrative) {
if label == "" {
label = o.ResourceID
}
pdf.CellFormat(0, 6, fmt.Sprintf("%s %s", label, o.Reason), "", 1, "L", false, 0, "")
pdf.CellFormat(0, 6, fmt.Sprintf("%s - %s", label, o.Reason), "", 1, "L", false, 0, "")
pdf.Ln(1)
}
}

View file

@ -164,6 +164,66 @@ func TestFleetSummary_HeuristicSourceShowsDiscoverabilityTip(t *testing.T) {
}
}
// TestFleetSummary_EmptyDataShowsNoData mirrors the single-resource
// empty-window guard for fleet reports. A fleet PDF whose every resource
// returned zero data points must not render the green HEALTHY card —
// "All systems operating normally" over no evidence is false assurance,
// which is the worst failure mode for a report whose job is to prove
// stability to a client.
func TestFleetSummary_EmptyDataShowsNoData(t *testing.T) {
now := time.Now()
multi := &MultiReportData{
Title: "Fleet",
Start: now.Add(-time.Hour),
End: now,
GeneratedAt: now,
Resources: []*ReportData{
{
ResourceID: "vm-aaaa",
ResourceType: "vm",
Summary: MetricSummary{ByMetric: map[string]MetricStats{}},
TotalPoints: 0,
},
{
ResourceID: "vm-bbbb",
ResourceType: "vm",
Summary: MetricSummary{ByMetric: map[string]MetricStats{}},
TotalPoints: 0,
},
},
}
text := renderFleetSummaryText(t, multi)
if !strings.Contains(text, "NO DATA") {
t.Errorf("expected 'NO DATA' card on empty fleet data, got:\n%s", text)
}
if strings.Contains(text, "All systems operating normally") {
t.Errorf("HEALTHY message should not appear on empty fleet data, got:\n%s", text)
}
}
// TestCoverPage_PrefersResourceName asserts the cover page and the page
// header lead with the human-readable resource name when enrichment
// resolved one. Canonical v6 resource IDs are opaque hashes
// (vm-7f8b2b6cd98c2089); a client reading a monthly report cannot map
// those to their machines.
func TestCoverPage_PrefersResourceName(t *testing.T) {
data := &ReportData{
Title: "Named",
ResourceType: "vm",
ResourceID: "vm-7f8b2b6cd98c2089",
Resource: &ResourceInfo{Name: "checkout-web-01", Status: "running"},
Start: time.Now().Add(-time.Hour),
End: time.Now(),
GeneratedAt: time.Now(),
Metrics: map[string][]MetricDataPoint{},
Summary: MetricSummary{ByMetric: map[string]MetricStats{}},
}
text := renderExecutiveSummaryText(t, data)
if !strings.Contains(text, "checkout-web-01") {
t.Errorf("expected resource name on cover, got:\n%s", text)
}
}
// renderExecutiveSummaryText runs writeExecutiveSummary against a
// fresh fpdf, extracts text by parsing the resulting PDF, and returns
// the rendered text content. Used by the UX assertions above.

View file

@ -23,6 +23,15 @@ type MetricReportRequest struct {
Title string
Branding ReportBranding
// MetricsResourceID overrides ResourceID for metrics-store queries.
// The v6 API addresses resources by canonical unified ID while the
// metrics store is keyed by each platform's native source ID (the
// resource's metricsTarget); recovery points and Patrol findings stay
// keyed by the unified ID. Handlers set this from the resource's
// metricsTarget so the store query can match without changing the
// identity the rest of the report pipeline sees.
MetricsResourceID string
// Optional enrichment data (populated by handler from monitor state)
Resource *ResourceInfo // Details about the resource being reported on
Alerts []AlertInfo // Active and recently resolved alerts for this resource