-
Notifications
You must be signed in to change notification settings - Fork 2
fix: cinder potential duplicate volume metrics #110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Tadas Sutkaitis (fitbeard)
wants to merge
1
commit into
main
Choose a base branch
from
fix/cinder_duplicated_metrics
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+44
−4
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| # Repository Guidelines | ||
|
|
||
| ## Project Structure & Module Organization | ||
|
|
||
| This is a Go Prometheus exporter for OpenStack database metrics. The entrypoint is `cmd/openstack-database-exporter/main.go`. Collector code lives in `internal/collector/<service>/`, shared database and DSN logic in `internal/db/`, and shared test helpers in `internal/testutil/`. SQL source files are grouped by OpenStack service under `sql/<service>/` with `schema.sql`, `queries.sql`, and optional `indexes.sql` or `prereqs.sql`. Files under `internal/db/<service>/` are generated by `sqlc`; update the SQL inputs instead of editing generated Go directly. | ||
|
|
||
| ## Build, Test, and Development Commands | ||
|
|
||
| - `go build -o /dev/null ./cmd/openstack-database-exporter`: compile the exporter, matching CI. | ||
| - `go run ./cmd/openstack-database-exporter --help`: inspect runtime flags and supported database URL environment variables. | ||
| - `go test -count=1 -short -race ./...`: run the unit test suite used by CI. | ||
| - `go test -v -count=1 -race -tags integration -timeout 15m ./...`: run container-backed integration tests; Docker must be available. | ||
| - `sqlc generate`: regenerate `internal/db/<service>/` after changing SQL definitions. | ||
| - `docker build -t openstack-database-exporter .`: build the scratch-based runtime image. | ||
|
|
||
| ## Coding Style & Naming Conventions | ||
|
|
||
| Use standard Go formatting (`gofmt`) and keep packages lowercase. Follow the existing service-oriented layout: service packages such as `cinder`, `nova`, and `neutron` contain collectors and tests for that service. Prefer table-driven tests and explicit Prometheus metric comparisons, as seen in existing collector tests. Metric names should continue the `openstack_<service>_<resource>` pattern. | ||
|
|
||
| ## Testing Guidelines | ||
|
|
||
| Unit tests use `testing`, `go-sqlmock`, `testify`, and Prometheus `testutil`; name files `*_test.go` and tests `Test<Behavior>`. Integration tests use the `integration` build tag and `testcontainers-go` with MariaDB 11; name them `TestIntegration_<Collector>` where practical. Set `SKIP_INTEGRATION=1` only when intentionally skipping tagged integration tests. | ||
|
|
||
| ## Commit & Pull Request Guidelines | ||
|
|
||
| Recent history mostly follows Conventional Commit subjects such as `fix: ...`, `feat: ...`, `ci: ...`, and `chore(deps): ...`; use the same concise imperative style. Pull requests should describe the exporter behavior changed, list affected services or metrics, link related issues, and note any SQL or generated-code updates. Run the relevant unit, integration, build, and `sqlc generate` checks before requesting review. | ||
|
|
||
| ## Local Configuration & Security | ||
|
|
||
| Configure database access with oslo.db-style MySQL URLs in environment variables. Keep examples generic and never commit real credentials, endpoint names, local `exporter.sh` values, logs, or captured metric output. | ||
|
|
||
| ```sh | ||
| export NOVA_DATABASE_URL="mysql+pymysql://<user>:<password>@<db-host>:<db-port>/nova" | ||
| export NOVA_API_DATABASE_URL="mysql+pymysql://<user>:<password>@<db-host>:<db-port>/nova_api" | ||
| export KEYSTONE_DATABASE_URL="mysql+pymysql://<user>:<password>@<db-host>:<db-port>/keystone" | ||
| export PLACEMENT_DATABASE_URL="mysql+pymysql://<user>:<password>@<db-host>:<db-port>/placement" | ||
| ``` | ||
|
|
||
| Set only the service URLs needed for the collectors under test. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In deployments or fixtures where
volume_attachment.attach_statusis still NULL for a non-deleted attachment, this join no longer matches the row and the exporter emitsserver_id=""for an in-use volume even thoughinstance_uuidis present. The column is nullable/defaults to NULL in the repo schema, and the end-to-end Cinder seed incmd/openstack-database-exporter/main_test.gocurrently inserts an attachment withoutattach_status, so this change silently loses the server label for that valid row shape rather than only filtering detached attachments.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After creating and attaching the volume, the real DB shows attach_status='attached', not NULL.
Key results:
Join comparison:
Latest active attachment:
So on current real Cinder behavior, strict
va.attach_status = 'attached'works and does not drop the server label.