feat: add Keystone metrics #34
Conversation
Signed-off-by: Tadas Sutkaitis <[email protected]>
Signed-off-by: Tadas Sutkaitis <[email protected]>
mnaser
left a comment
mnaser
left a comment
There was a problem hiding this comment.
The test data matches the upstream openstack-exporter fixtures and the file structure is well organized. A couple of changes needed for consistency:
- ❌ Logger pattern needs to match the established
namespace/subsystem/collectorpattern in all sub-collectors - ❌ The
_upmetric handling should reflect actual success/failure of queries (currently always emitsup=1)
| func NewDomainsCollector(db *sql.DB, logger *slog.Logger) *DomainsCollector { | ||
| return &DomainsCollector{ | ||
| db: db, | ||
| logger: logger, |
There was a problem hiding this comment.
The logger should include structured context to match the established pattern in cinder/volumes.go:
logger: logger.With(
"namespace", collector.Namespace,
"subsystem", Subsystem,
"collector", "domains",
),| func NewProjectsCollector(db *sql.DB, logger *slog.Logger) *ProjectsCollector { | ||
| return &ProjectsCollector{ | ||
| db: db, | ||
| logger: logger, |
There was a problem hiding this comment.
Same here - please add structured logger context:
logger: logger.With(
"namespace", collector.Namespace,
"subsystem", Subsystem,
"collector", "projects",
),| db: db, | ||
| logger: logger, | ||
| } | ||
| } |
There was a problem hiding this comment.
Same here - please add structured logger context:
logger: logger.With(
"namespace", collector.Namespace,
"subsystem", Subsystem,
"collector", "groups",
),| db: db, | ||
| logger: logger, | ||
| } | ||
| } |
There was a problem hiding this comment.
Same here - please add structured logger context:
logger: logger.With(
"namespace", collector.Namespace,
"subsystem", Subsystem,
"collector", "regions",
),| db: db, | ||
| logger: logger, | ||
| } | ||
| } |
There was a problem hiding this comment.
Same here - please add structured logger context:
logger: logger.With(
"namespace", collector.Namespace,
"subsystem", Subsystem,
"collector", "users",
),| c.groupsCollector.Collect(ch) | ||
| c.regionsCollector.Collect(ch) | ||
| c.usersCollector.Collect(ch) | ||
| } |
There was a problem hiding this comment.
The _up metric is always set to 1 here, but it should reflect whether the sub-collectors succeeded. Looking at cinder/volumes.go, the pattern is:
- Emit
up=0immediately when a query fails and return early - Emit
up=1only at the end after all metrics are successfully collected
Consider having sub-collectors return an error, or track success state, so the umbrella collector can emit up=0 when any sub-collector fails.
No description provided.