Skip to content

Fix JSON tag mismatch for metrics responses#137

Open
Nehanth67 wants to merge 1 commit into
apache:mainfrom
Nehanth67:fix_filedmismatches
Open

Fix JSON tag mismatch for metrics responses#137
Nehanth67 wants to merge 1 commit into
apache:mainfrom
Nehanth67:fix_filedmismatches

Conversation

@Nehanth67

Copy link
Copy Markdown

CloudStack metrics APIs return lists under updated JSON keys (e.g. "host" for host metrics).
The cloudstack-go response structs still used older json tags (e.g. "hostmetric"/"hostmetrics"),
causing unmarshalling issues where metrics lists become nil/null.
This PR updates the relevant response struct json tags to match the actual CloudStack API response keys.

Copilot AI review requested due to automatic review settings December 30, 2025 00:07

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes JSON unmarshalling issues in CloudStack metrics APIs by updating response struct tags to match the actual API response keys. The CloudStack API returns metrics lists under singular entity names (e.g., "host", "zone"), but the Go structs were using incorrect plural/compound tags (e.g., "hostsmetric", "zonesmetric").

  • Updated JSON tags for 6 metrics response structs to use singular entity names
  • Aligned naming convention with other existing metrics responses in the codebase (ManagementServersMetrics, StoragePoolsMetrics, VirtualMachinesMetrics already use singular names)

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
ZoneService.go Changed ZonesMetrics JSON tag from "zonesmetric" to "zone"
VolumeService.go Changed VolumesMetrics JSON tag from "volumesmetric" to "volume"
UsageService.go Changed UsageServerMetrics JSON tag from "usageservermetric" to "usageserver"
InfrastructureUsageService.go Changed DbMetrics JSON tag from "dbMetrics" to "db"
HostService.go Changed HostsMetrics JSON tag from "hostsmetric" to "host"
ClusterService.go Changed ClustersMetrics JSON tag from "clustersmetric" to "cluster"

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@DaanHoogland DaanHoogland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clgtm

@DaanHoogland

Copy link
Copy Markdown
Contributor

@Pearl1594 @vishesh92 , please review this?

@jmsperu

jmsperu commented Jun 20, 2026

Copy link
Copy Markdown

Reviewed these against the CloudStack 4.22 source (plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java). The host / cluster / volume / zone changes look correct (they match the object names inherited from the base responses / set in the service). Two look off, though:

  • DbMetricsResponse — this changes json:"dbMetrics"json:"db", but listDbMetrics() calls response.setObjectName("dbMetrics") (MetricsServiceImpl.java:1114). So the original dbMetrics tag was already correct, and db would actually break DB-metrics unmarshalling. Suggest reverting this one to dbMetrics.
  • UsageServerMetrics — changed to json:"usageserver", but listUsageServerMetrics() calls response.setObjectName("usageMetrics") (MetricsServiceImpl.java:1087). The key is usageMetrics, not usageserver.

The other four are good to merge. Happy to confirm against a live 4.22 deployment once I have one to hand.

@jmsperu

jmsperu commented Jun 20, 2026

Copy link
Copy Markdown

Confirmed now against a live CloudStack 4.22 deployment (raw response=json output):

API actual response key
listDbMetrics dbMetrics
listUsageServerMetrics usageMetrics
listHostsMetrics host
listClustersMetrics cluster

So host / cluster (and volume / zone) are correct as in this PR. The two to fix:

  • DbMetrics: keep json:"dbMetrics" (the original) — db returns nothing.
  • UsageServerMetrics: use json:"usageMetrics", not usageserver.

With those two tweaked, this is good to merge. 👍

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

Labels

None yet

Projects

None yet

6 participants