Conversation
Signed-off-by: Tadas Sutkaitis <tadas.sutkaitis@vexxhost.com>
Signed-off-by: Tadas Sutkaitis <tadas.sutkaitis@vexxhost.com>
|
|
||
| func mapServerStatus(status string) int { | ||
| for idx, s := range knownServerStatuses { | ||
| if status == s { |
There was a problem hiding this comment.
Use: strings.ToUpper(status)
In case where the status has different casing
Furthermore, since Niedbalski exporter exports capitalized status, we should do the same:
Replace:
instance.VmState.String
with
strings.ToUpper(instance.VmState.String)
for the server_status metric
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive Nova metrics collection by implementing database queries against Nova's main database, Nova API database, and Placement database. The implementation supports both legacy DbQuotaDriver and newer UnifiedLimitsDriver quota systems by leveraging placement allocations data.
Changes:
- Added SQLC configuration and SQL schemas for nova and nova_api databases
- Implemented collectors for services, flavors, quotas, limits, compute nodes, and servers
- Extended placement schema to include projects, users, and consumers tables
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| sqlc.yaml | Added nova and nova_api database configurations |
| sql/placement/schema.sql | Added projects, users, and consumers tables to placement schema |
| sql/placement/queries.sql | Added queries for allocation tracking and consumer information |
| sql/nova_api/schema.sql | Created schema for flavors, quotas, aggregates, and quota_usages |
| sql/nova_api/queries.sql | Added queries for flavors, quotas, aggregates, and quota usage |
| sql/nova/schema.sql | Created schema for instances, services, and compute_nodes |
| sql/nova/queries.sql | Added queries for instances, services, and compute nodes |
| internal/collector/nova/*.go | Implemented collectors for services, flavors, quotas, limits, compute nodes, and servers |
| cmd/openstack-database-exporter/main.go | Added Nova database URL configuration flags |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "", // address_ipv4 - would need separate query for fixed IPs | ||
| "", // address_ipv6 - would need separate query for fixed IPs | ||
| instance.AvailabilityZone.String, | ||
| strconv.FormatInt(int64(instance.InstanceTypeID.Int32), 10), |
There was a problem hiding this comment.
instance.InstanceTypeID.Int32 is not the correct ID. The Niedbalski exporter will export a flavorID, whereas this will export the internl DB primary key:
Refer to this comment in Nova codebase
For user of the exporter, this id is not meaningful.
Proposal: You can use novaapi to get all flavors, create an id to flavorID map and then place the correct flavor id in the metric.
There was a problem hiding this comment.
We should use the nova_api database to get the flavors, indeed.
| adminState, | ||
| disabledReason, | ||
| nullStringToString(service.Host), | ||
| fmt.Sprintf("%d", service.ID), |
There was a problem hiding this comment.
This is not the correct id. The Niedbalski exporter provides the ID found in the "uuid" column of the services table in Nova db.
I.e., replace:
fmt.Sprintf("%d", service.ID)
with
service.Uuid.String
|
|
||
| for _, flavor := range flavors { | ||
| // Format labels to match original test order: disk, id, is_public, name, ram, vcpus | ||
| id := fmt.Sprintf("%d", flavor.ID) |
There was a problem hiding this comment.
Similar to before, wrong id:
Replace
id := fmt.Sprintf("%d", flavor.ID)
With:
id := flavor.Flavorid
| SELECT | ||
| p.external_id as project_id, | ||
| rc.name as resource_type, | ||
| COALESCE(SUM(a.used), 0) as used |
There was a problem hiding this comment.
Is there a reason why we aren't casting this?
Because we could do:
CAST(COALESCE(SUM(a.used), 0) AS UNSIGNED) AS used
And this would force MySQL to return an Unsigned, 32-bit integer based on docs
Sqlc will for some reason generate code involving a 64-bit integer. But this will avoid us having to guess which type MySQL generates, since it's now an integer and not a Golang interface.
I mention this because I noticed that at least this code here:
Isn't working nicely... The metrics I got for VCPUs were all 0s, which indicates to me that something is broken here and it is not assigning the VCPU value correctly, defaulting to 0.
| uuid, | ||
| host, | ||
| `binary`, | ||
| topic, |
There was a problem hiding this comment.
I noticed that the vexxhost openstack exporter exports much more openstack_nova_agent_state metrics than Niedbalski. This query does not exclude services where topic IS NULL. This should be either changed or declared when exporter is deemed ready as it can potentially change what users see in their Dashboards and alerts.
| // Determine admin state and disabled reason | ||
| adminState := "enabled" | ||
| disabledReason := "" | ||
| agentValue := float64(1) // 1 for enabled, 0 for disabled |
There was a problem hiding this comment.
The agentValue is not only bound to disabled/enabled.
The openstack cli, with
openstack compute service list
Provides you with an up/down values. That's what the agent value of the Niedbalski exporter is using.
See https://github.com/openstack-exporter/openstack-exporter/blob/6b5671234b4fbc6da47512b9623f42a1d97a13df/exporters/nova.go#L169
3 participants
This heavily depends on
placementdatabase which we implemented earlier. Allocations and usage are collected fromplacementdatabase because all data is already there. This also will cover both drivers for quotas:This should be also valid for: https://docs.openstack.org/nova/2025.2/admin/quotas.html#quota-usage-from-placement