Skip to content

Add health-checks settings keys for Sidekiq

Matthias Käppler requested to merge mk-add-sk-health-check-config into master

What does this MR do?

We will soon be serving Sidekiq metrics from a separate server process. During this transition, we temporarily plan to instantiate SidekiqExporter twice: once where it is used now (in the worker), and again in the new server process. This is because we want to keep health-checks being served from workers, and only serve metrics from the new server process.

To that end, we need to make a clearer distinction between health-checks and metrics endpoint config, which are currently both part of SidekiqExporter and its settings bucket. This will be a breaking change, so we are taking small incremental steps to remain backwards-compatible.

This MR starts by splitting up settings keys between metrics and health-checks for Sidekiq, so that those servers can be configured independently. For this to remain backwards compatible, we default the new settings keys for health checks to the existing exporter settings. In the application MR, we will start using the former to instantiate the in-worker server.

NOTE: since Prometheus is still set up to scrape metrics from 3807, and because the server extraction isn't complete yet, setting two different ports for metrics and health-checks would currently not work (since it either breaks metrics or health-checks), which is why we are leaving this undocumented for now.

Related issues

Test plan

We should verify two things: first, that by default i.e. without changing any configuration, both metrics and health-checks still work as expected. Just install the chart for this and check that:

  1. The sidekiq container comes up and is ready
  2. Shell into the sidekiq container
  3. curling :3807/metrics serves prometheus metrics
  4. curling :3807/[liveness|readiness] serves health-checks

Second, we want to verify that altering the health_checks port will let SidekiqExporter start on that port and serve both metrics and health-checks from it:

  1. helm upgrade gitlab . --set certmanager-issuer.email=me@somewhere.com --set gitlab.sidekiq.health_checks.port=3999 --set gitlab.global.image.tag=master --set gitlab.global.image.pullPolicy=Always (we need changes from master)
  2. Verify sidekiq container comes up and is ready
  3. Shell into the sidekiq container
  4. curling :3999/metrics serves prometheus metrics
  5. curling :3999/[liveness|readiness] serves health-checks

Checklist

See Definition of done.

For anything in this list which will not be completed, please provide a reason in the MR discussion.

Required

  • Merge Request Title and Description are up to date, accurate, and descriptive
  • MR targeting the appropriate branch
  • MR has a green pipeline on GitLab.com

Expected (please provide an explanation if not completing)

  • Test plan indicating conditions for success has been posted and passes
  • [-] Documentation created/updated (I think it could be confusing to document these settings at this point, since we do not yet have the capability to spawn a separate server)
  • Tests added
  • Integration tests added to GitLab QA
  • Equivalent MR/issue for omnibus-gitlab opened: gitlab-org/omnibus-gitlab!5743 (merged)
Edited by Matthias Käppler

Merge request reports

Loading