Skip to content

Add prometheus metrics

📇 Context

The path traversal middleware is working pretty well. The context is explained here.

As we're preparing the next steps, we noted that the observability of the middleware was lacking.

We want to have a clear view on:

  • the impact of the middleware in terms of execution time.
  • the amount of rejected requests.

Thus, https://gitlab.com/gitlab-org/gitlab/-/issues/498510+ was created and the very first steps is to update the middleware code to start generating (prometheus) metrics.

🤔 What does this MR do and why?

  • Add an apdex SLI based on the middleware execution time. The threshold that we're using is 1ms. Everything above will be tagged as non success.
    • On top of that, we're going to use labels to indicate if a request has been rejected or not.
  • Update the GitLab log instrumentation to include the middleware execution, no matter if it rejects request or not.
  • Update the middleware logs to include the remote IP.
  • Add/Update the related specs.

The path traversal check middleware is still behind a feature flag. Thus, we don't need a changelog here.

We followed https://docs.gitlab.com/ee/development/prometheus_metrics.html.

🏎️ MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

🦄 Screenshots or screen recordings

From the /-/metrics url:

Screenshot_2024-10-29_at_17.48.41

⚙️ How to set up and validate locally

In a rails console:

  • Enable the general middleware feature flag : Feature.enable(:check_path_traversal_middleware)

1️⃣ Regular url, no rejection:

  • Browse http://gdk.test:8000
  • Check $ tail -f log/development_json.log | jq:
    {
      ...,
      "path_traversal_check_duration_ms": 0.04700000863522291,
      ...,
    }

The path traversal middleware execution is measured and logged

2️⃣ Path traversal attempt, no rejection:

  • Browse http://gdk.test:8000/foo%2F..%2Fbar
  • Check tail -f log/application_json.log | jq:
    {
      ...,
      "message": "Potential path traversal attempt detected.",
      "remote_ip": "172.16.123.1",
      "path_traversal_check_duration_ms": 0.5979999987175688,
      ...,
    }

The path traversal execution time and the remote ip are logged

3️⃣ Regular url, rejection enabled

  • In a rails console: Feature.enable(:check_path_traversal_middleware_reject_requests)
  • Browse http://gdk.test:8000
  • Check $ tail -f log/development_json.log | jq:
    {
      ...,
      "path_traversal_check_duration_ms": 0.07500000356230885
      ...,
    }

The path traversal middleware execution is measured and logged

4️⃣ Path traversal attempt, rejection enabled

  • In a rails console: Feature.enable(:check_path_traversal_middleware_reject_requests)
  • Browse http://gdk.test:8000/foo%2F..%2Fbar
  • Check tail -f log/application_json.log | jq:
    {
      ...,
      "message": "Potential path traversal attempt detected.",
      "remote_ip": "172.16.123.1",
      "path_traversal_attempt_rejected": true,
      "remote_ip": "172.16.123.1",
      "path_traversal_check_duration_ms": 0.9830000053625554,
      ...,
    }

The path traversal execution time and the remote ip are logged

🔮 Conclusions

The changes are behaving the way we want.

Edited by David Fernandez

Merge request reports

Loading