Skip to content

Fix broken metrics_server dependencies

Matthias Käppler requested to merge mk-fix-missing-metrics-server-dependency into master

What does this MR do and why?

Related to #350548 (closed)

We recently merged !82478 (merged), which starts a separate server process to serve Puma metrics. This change is behind an ENV var.

We found that if metrics are not enabled in Omnibus, and the ENV var is enabled, then workers could run into missing constant errors because MetricsServer (a top-level module not on the default load path) was only required in a hook that runs in the Puma primary and when metrics are enabled.

I fixed this by always requiring the top-level module when running the Prometheus initializer. This also meant I had to move all its dependencies out of this module since for some reason this conflicted with Zeitwerk resolution, which attempts to require constants as soon as it encounters them, and we have a circular dependency between Gitlab::Metrics and Gitlab::Metrics::Prometheus (the former includes the latter but the latter is defined inside the former -- I am not even sure how Zeitwerk handles this because dependency graphs should be acyclic?)

The dependencies module exists because we run this server outside of a Rails/Zeitwerk context in separate non-Rails Ruby processes.

NOTE: This is not customer-facing. So I labeled it as maintenance not typebug. Since the change is behind an env var that acts as a feature toggle, the change only surfaced during testing on staging-ref.

Screenshots or screen recordings

These are strongly recommended to assist reviewers and reduce the time to merge your change.

How to set up and validate locally

We want to test the following matrix, with Omnibus or otherwise:

PUMA_EXTERNAL_METRICS_SERVER exporter enabled works
false false ✅
false true ✅
true false ✅
true true ✅

"works" meaning:

  • we do not crash during boot
  • we export metrics via :8083 whenever the exporter is enabled

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Matthias Käppler

Merge request reports