Skip to content
Snippets Groups Projects

feat: add metrics for repository migration status

Closed feistel requested to merge feistel/container-registry:feat/track-migration-status into master
4 unresolved threads

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)

Edited by feistel

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Jaime Martinez
  • Jaime Martinez
  • feistel added 1 commit

    added 1 commit

    • f47417ad - refactor: decouple metrics and migration package and move metrics hooks

    Compare with previous version

  • feistel added 1 commit

    added 1 commit

    • 6aeed207 - test: add migration status metric test

    Compare with previous version

  • feistel resolved all threads

    resolved all threads

  • Jaime Martinez approved this merge request

    approved this merge request

  • 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
  • 277 279 require.NoError(suite.T(), err)
    278 280 }
    279 281
    282 func (suite *metricsSuite) TestImportMigrationStatus() {
  • Jaime Martinez requested review from @jdrpereira

    requested review from @jdrpereira

  • feistel added 18 commits

    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

    Compare with previous version

  • Author Contributor

    rebased on the main branch and fixed the conflicts

    • @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 Pereira
    • Author Contributor

      Hey @jdrpereira :wave:

      Don't worry, you and the package team have been doing a wonderful job with community contributions :smile:

      Thank you for all the details and providing additional context! I agree this can be closed :thumbsup:

      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
    • Please register or sign in to reply
  • Hi @feistel,

    We would love to know how you found your code review experience in this merge request! Please leave a :thumbsup: or a :thumbsdown: 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! :heart:

  • Please register or sign in to reply
    Loading