metrics: Add a `reason` label to the HS error metrics.

This adds a `reason` label to the `hs_intro_rejected_intro_req_count` and
`hs_rdv_error_count` metrics introduced in #40755.

Metric look up and intialization is now more a bit more involved. This may be
fine for now, but it will become unwieldy if/when we add more labels (and as
such will need to be refactored).

Also, in the future, we may want to introduce finer grained `reason` labels.
For example, the `invalid_introduce2` label actually covers multiple types of
errors that can happen during the processing of an INTRODUCE2 cell (such as
cell parse errors, replays, decryption errors).

Signed-off-by: Gabriela Moldovan <gabi@torproject.org>

1 files changed, 102 insertions, 30 deletions

diff --git a/src/feature/hs/hs_metrics.c b/src/feature/hs/hs_metrics.c
index 854cd294c4..4c6d957cf8 100644
--- a/src/feature/hs/hs_metrics.c
+++ b/src/feature/hs/hs_metrics.c
@@ -13,6 +13,7 @@
 #include "lib/malloc/malloc.h"
 #include "lib/container/smartlist.h"
 #include "lib/metrics/metrics_store.h"
+#include "lib/log/util_bug.h"
 
 #include "feature/hs/hs_metrics.h"
 #include "feature/hs/hs_metrics_entry.h"
@@ -29,51 +30,118 @@ port_to_str(const uint16_t port)
   return buf;
 }
 
-/** Initialize a metrics store for the given service.
+/** Add a new metric to the metrics store of the service.
  *
- * Essentially, this goes over the base_metrics array and adds them all to the
- * store set with their label(s) if any. */
+ * <b>metric</b> is the index of the metric in the <b>base_metrics</b> array.
+ */
 static void
-init_store(hs_service_t *service)
+add_metric_with_labels(hs_service_t *service, hs_metrics_key_t metric,
+                       bool port_as_label, uint16_t port)
 {
   metrics_store_t *store;
+  const char **error_reasons = NULL;
+  size_t num_error_reasons = 0;
 
   tor_assert(service);
 
+  if (BUG(metric >= base_metrics_size))
+    return;
+
   store = service->metrics.store;
 
+  /* Check whether the current metric is an error metric, because error metrics
+   * require an additional `reason` label. */
+  switch (metric) {
+    case HS_METRICS_NUM_REJECTED_INTRO_REQ:
+      error_reasons = hs_metrics_intro_req_error_reasons;
+      num_error_reasons = hs_metrics_intro_req_error_reasons_size;
+      break;
+    case HS_METRICS_NUM_FAILED_RDV:
+      error_reasons = hs_metrics_rend_error_reasons;
+      num_error_reasons = hs_metrics_rend_error_reasons_size;
+      break;
+    /* Fall through for all other metrics, as they don't need a
+     * reason label. */
+    case HS_METRICS_NUM_INTRODUCTIONS: FALLTHROUGH;
+    case HS_METRICS_APP_WRITE_BYTES: FALLTHROUGH;
+    case HS_METRICS_APP_READ_BYTES: FALLTHROUGH;
+    case HS_METRICS_NUM_ESTABLISHED_RDV: FALLTHROUGH;
+    case HS_METRICS_NUM_RDV: FALLTHROUGH;
+    case HS_METRICS_NUM_ESTABLISHED_INTRO: FALLTHROUGH;
+    default:
+      break;
+  }
+
+  /* We don't need a reason label for this metric */
+  if (!num_error_reasons) {
+      metrics_store_entry_t *entry = metrics_store_add(
+          store, base_metrics[metric].type, base_metrics[metric].name,
+          base_metrics[metric].help);
+
+      metrics_store_entry_add_label(entry,
+              metrics_format_label("onion", service->onion_address));
+
+      if (port_as_label) {
+        metrics_store_entry_add_label(entry,
+                metrics_format_label("port", port_to_str(port)));
+      }
+
+      return;
+  }
+
+  tor_assert(error_reasons);
+
+  /* Add entries with reason as label. We need one metric line per
+   * reason. */
+  for (size_t i = 0; i < num_error_reasons; ++i) {
+    metrics_store_entry_t *entry =
+      metrics_store_add(store, base_metrics[metric].type,
+                        base_metrics[metric].name,
+                        base_metrics[metric].help);
+    /* Add labels to the entry. */
+    metrics_store_entry_add_label(entry,
+            metrics_format_label("onion", service->onion_address));
+    metrics_store_entry_add_label(entry,
+            metrics_format_label("reason", error_reasons[i]));
+
+    if (port_as_label) {
+      metrics_store_entry_add_label(entry,
+              metrics_format_label("port", port_to_str(port)));
+    }
+  }
+}
+
+/** Initialize a metrics store for the given service.
+ *
+ * Essentially, this goes over the base_metrics array and adds them all to the
+ * store set with their label(s) if any. */
+static void
+init_store(hs_service_t *service)
+{
+  tor_assert(service);
+
   for (size_t i = 0; i < base_metrics_size; ++i) {
     /* Add entries with port as label. We need one metric line per port. */
     if (base_metrics[i].port_as_label && service->config.ports) {
       SMARTLIST_FOREACH_BEGIN(service->config.ports,
                               const hs_port_config_t *, p) {
-        metrics_store_entry_t *entry =
-          metrics_store_add(store, base_metrics[i].type, base_metrics[i].name,
-                            base_metrics[i].help);
-
-        /* Add labels to the entry. */
-        metrics_store_entry_add_label(entry,
-                metrics_format_label("onion", service->onion_address));
-        metrics_store_entry_add_label(entry,
-                metrics_format_label("port", port_to_str(p->virtual_port)));
+        add_metric_with_labels(service, base_metrics[i].key, true,
+                               p->virtual_port);
       } SMARTLIST_FOREACH_END(p);
     } else {
-      metrics_store_entry_t *entry =
-        metrics_store_add(store, base_metrics[i].type, base_metrics[i].name,
-                          base_metrics[i].help);
-      metrics_store_entry_add_label(entry,
-              metrics_format_label("onion", service->onion_address));
+      add_metric_with_labels(service, base_metrics[i].key, false, 0);
     }
   }
 }
 
 /** Update the metrics key entry in the store in the given service. The port,
- * if non 0, is used to find the correct metrics entry. The value n is the
- * value used to update the entry. */
+ * if non 0, and the reason label, if non NULL, are used to find the correct
+ * metrics entry. The value n is the value used to update the entry. */
 void
 hs_metrics_update_by_service(const hs_metrics_key_t key,
                              const hs_service_t *service,
-                             uint16_t port, int64_t n)
+                             uint16_t port, const char *reason,
+                             int64_t n)
 {
   tor_assert(service);
 
@@ -89,25 +157,29 @@ hs_metrics_update_by_service(const hs_metrics_key_t key,
    * XXX: This is not the most optimal due to the string format. Maybe at some
    * point turn this into a kvline and a map in a metric entry? */
   SMARTLIST_FOREACH_BEGIN(entries, metrics_store_entry_t *, entry) {
-    if (port == 0 ||
-        metrics_store_entry_has_label(entry,
-                      metrics_format_label("port", port_to_str(port)))) {
-      metrics_store_entry_update(entry, n);
-      break;
+    if ((port == 0 ||
+         metrics_store_entry_has_label(
+             entry, metrics_format_label("port", port_to_str(port)))) &&
+        ((!reason || metrics_store_entry_has_label(
+                         entry, metrics_format_label("reason", reason))))) {
+        metrics_store_entry_update(entry, n);
+        break;
     }
   } SMARTLIST_FOREACH_END(entry);
 }
 
 /** Update the metrics key entry in the store of a service identified by the
- * given identity public key. The port, if non 0, is used to find the correct
- * metrics entry. The value n is the value used to update the entry.
+ * given identity public key. The port, if non 0, and the reason label, if non
+ * NULL, are used to find the correct metrics entry. The value n is the value
+ * used to update the entry.
  *
  * This is used by callsite that have access to the key but not the service
  * object so an extra lookup is done to find the service. */
 void
 hs_metrics_update_by_ident(const hs_metrics_key_t key,
                            const ed25519_public_key_t *ident_pk,
-                           const uint16_t port, int64_t n)
+                           const uint16_t port, const char *reason,
+                           int64_t n)
 {
   hs_service_t *service;
 
@@ -121,7 +193,7 @@ hs_metrics_update_by_ident(const hs_metrics_key_t key,
      * service and thus the only way to know is to lookup the service. */
     return;
   }
-  hs_metrics_update_by_service(key, service, port, n);
+  hs_metrics_update_by_service(key, service, port, reason, n);
 }
 
 /** Return a list of all the onion service metrics stores. This is the