Skip to content

Handle zone names without uuids.#10568

Draft
jmcarp wants to merge 1 commit into
mainfrom
jmcarp/zone-stat-parse
Draft

Handle zone names without uuids.#10568
jmcarp wants to merge 1 commit into
mainfrom
jmcarp/zone-stat-parse

Conversation

@jmcarp

@jmcarp jmcarp commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

The zone:cpu_nsec metric parses zone names into zone types and uuids, for zone names formatted as oxz_TYPE_UUID. However, zone names like "global" and "oxz_switch" don't follow that format, leaving the zone type as empty. This patch adds zone types for those zone names, parsing "global" to "global" and "oxz_switch" to "switch".

The zone:cpu_nsec metric parses zone names into zone types and uuids, for zone
names formatted as oxz_TYPE_UUID. However, zone names like "global" and
"oxz_switch" don't follow that format, leaving the zone type as empty. This
patch adds zone types for those zone names, parsing "global" to "global" and
"oxz_switch" to "switch".
Comment on lines +50 to +54
} else if let Some(rest) = zone_name.strip_prefix(ZONE_PREFIX) {
ZoneMetadata { zone_type: rest.to_string(), zone_id: Uuid::nil() }
} else {
ZoneMetadata { zone_type: zone_name.into(), zone_id: Uuid::nil() }
}

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.

Two notes:

  1. We should use OmicronZoneUuid for a strongly-typed UUID, if possible.
  2. I think we should use an Option here, and if a zone ID isn't present we should mark it as None.

Thoughts?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Oximeter field types aren't optional today, so we have to pick a real value, unfortunately.

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.

Ah I see. In that case I guess nil is okay (though we'll probably want to document this limitation here)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants