feat: add metrics for repository migration status
Adds a gauge to track migration status.
changes are treated like transactions (eg. PreImportInProgress -> PreImportComplete) so that there is always have a reliable snapshot of the active repositories.
Note: repositories with import status RepositoryStatusNative
are not tracked.
Closes #512 (closed)
Merge request reports
Activity
Thank you for your contribution to GitLab. We believe that everyone can contribute and contributions like yours are what make GitLab great!
- Our Merge Request Coaches will ensure your contribution is reviewed in a timely manner*.
- You can comment
@gitlab-bot label ~"group::"
to add a group label. - When you feel your MR (merge request) is ready for a review, just ask
@gitlab-bot request_review
and someone will take a look. - If you are stuck, feel free to ask
@gitlab-bot help
or ping a Merge Request Coach. - Read more on how to get help.
- You can provide feedback on the GitLab Contributor Experience in this survey
This message was generated automatically. You're welcome to improve it.
added Community contribution label
assigned to @feistel
mentioned in issue #512 (closed)
requested review from @jaime
mentioned in issue gitlab-org/quality/triage-reports#6776 (closed)
changed milestone to %14.9
- Resolved by feistel
- Resolved by feistel
- Resolved by feistel
@feistel thank you! I left some comments for you. Let's add a test to
registry/handlers/internal/metrics/metrics_test.go
too@jaime Thank you for the feedback! I've pushed some changes
Looks great @feistel! On to @jdrpereira for maintainer review
added 1 commit
- f47417ad - refactor: decouple metrics and migration package and move metrics hooks
317 registry_http_import_migrations{status="pre_import_complete"} 1 318 registry_http_import_migrations{status="pre_import_in_progress"} 2 319 `) 320 321 err = testutil.GatherAndCompare(prometheus.DefaultGatherer, &expected, migrationStatusFullName) 322 require.NoError(t, err) 323 324 // Simulate remaining imports finishing. 325 MigrationStatus(string(migration.RepositoryStatusImportInProgress), string(migration.RepositoryStatusImportComplete)) 326 MigrationStatus(string(migration.RepositoryStatusPreImportInProgress), string(migration.RepositoryStatusPreImportComplete)) 327 MigrationStatus(string(migration.RepositoryStatusPreImportInProgress), string(migration.RepositoryStatusPreImportComplete)) 328 329 expected.WriteString(` 330 # HELP registry_http_import_migrations A gauge to track repositories that are actively being migrated. 331 # TYPE registry_http_import_migrations gauge 332 registry_http_import_migrations{status="import_complete"} 2 question:
import_complete
will never decrease. Is that something we need to worry about @jdrpereira ?
requested review from @jdrpereira
added 18 commits
-
6aeed207...4d4fbe34 - 15 commits from branch
gitlab-org:master
- 17a8d16f - feat: add metrics for repository migration status
- df095b73 - refactor: decouple metrics and migration package and move metrics hooks
- c9b90255 - test: add migration status metric test
Toggle commit list-
6aeed207...4d4fbe34 - 15 commits from branch
@feistel, thanks for another contribution! I reviewed this, and although all looks good, I'm not sure we can build relevant graphs from these metrics. I see that the linked issue is a few months old and sparse in details, so a lot has changed since then.
We already have the
in_flight_imports
metric, which allows us to monitor the number of ongoing imports. I'm afraid monitoring state transitions on an instance level with a gauge (which resets on every instance restart, and we have a lot of them on .com, which come and go at a fast pace) will not add value to the metrics we already have.This is also because we have already created metrics to track the count of repositories per migration statuses but on the Rails side and globally (using a periodic count query directly on the DB). This hasn't been deployed yet, but I believe it will provide us with all the required visibility.
Considering the above, I'd suggest we don't merge this now. Sorry that we didn't catch this before. We should have reviewed the issue to ensure it was still relevant and accurate. Is this OK for you?
Thanks again for all your contributions so far! We'll do a better job at ensuring that issues are still relevant and provide precise direction/requirements.
Edited by João PereiraHey @jdrpereira
Don't worry, you and the package team have been doing a wonderful job with community contributions
Thank you for all the details and providing additional context! I agree this can be closed
Note: I picked this up from the milestone planning so it should probably also be noted/updated that the issue is no longer relevant. (I left a comment in the linked issue)
Edited by feistel
removed workflowin dev label
Hi @feistel,
We would love to know how you found your code review experience in this merge request! Please leave a
or a on this comment to describe your experience.Once done, please comment
@gitlab-bot feedback
below and feel free to leave any additional feedback you have in the same comment.You can also fill out a 5 minute survey to provided additional feedback on how GitLab can improve the contributor experience.
Thanks for your help!
mentioned in merge request gitlab-com/www-gitlab-com!100983 (merged)