[tor-commits] [tor] branch main updated: metrics: Add metrics for rendezvous and introduction request failures.

gitolite role git at cupani.torproject.org
Thu Feb 16 18:58:18 UTC 2023


This is an automated email from the git hooks/post-receive script.

dgoulet pushed a commit to branch main
in repository tor.

The following commit(s) were added to refs/heads/main by this push:
     new db4c4d656a metrics: Add metrics for rendezvous and introduction request failures.
db4c4d656a is described below

commit db4c4d656a8d3cc58bd6179c8dedd3f3b5f68dd6
Author: Gabriela Moldovan <gabi at torproject.org>
AuthorDate: Fri Feb 10 12:20:23 2023 +0000

    metrics: Add metrics for rendezvous and introduction request failures.
    
    This introduces a couple of new service side metrics:
    * `hs_intro_rejected_intro_req_count`, which counts the number of introduction
      requests rejected by the hidden service
    * `hs_rdv_error_count`, which counts the number of rendezvous errors as seen by
      the hidden service (this number includes the number of circuit establishment
      failures, failed retries, end-to-end circuit setup failures)
    
    Closes #40755. This partially addresses #40717.
    
    Signed-off-by: Gabriela Moldovan <gabi at torproject.org>
---
 changes/ticket40755               |  3 +++
 src/core/or/circuituse.c          |  7 ++++++-
 src/feature/hs/hs_circuit.c       | 19 +++++++++++++------
 src/feature/hs/hs_metrics.c       |  4 ++--
 src/feature/hs/hs_metrics.h       | 12 ++++++++++--
 src/feature/hs/hs_metrics_entry.c | 16 ++++++++++++++--
 src/feature/hs/hs_metrics_entry.h |  6 +++++-
 src/feature/hs/hs_service.c       |  6 +++++-
 src/test/test_hs_metrics.c        | 17 +++++++++++++++++
 src/test/test_hs_service.c        | 17 +++++++++++++++++
 10 files changed, 92 insertions(+), 15 deletions(-)

diff --git a/changes/ticket40755 b/changes/ticket40755
new file mode 100644
index 0000000000..a40bfd9239
--- /dev/null
+++ b/changes/ticket40755
@@ -0,0 +1,3 @@
+  o Minor features (metrics):
+    - Add service side metrics for REND and introduction request failures.
+      Closes ticket 40755.
diff --git a/src/core/or/circuituse.c b/src/core/or/circuituse.c
index dbeea10821..421d3662af 100644
--- a/src/core/or/circuituse.c
+++ b/src/core/or/circuituse.c
@@ -51,6 +51,7 @@
 #include "feature/hs/hs_client.h"
 #include "feature/hs/hs_common.h"
 #include "feature/hs/hs_ident.h"
+#include "feature/hs/hs_metrics.h"
 #include "feature/hs/hs_stats.h"
 #include "feature/nodelist/describe.h"
 #include "feature/nodelist/networkstatus.h"
@@ -1751,8 +1752,10 @@ circuit_build_failed(origin_circuit_t *circ)
               circuit_purpose_to_string(TO_CIRCUIT(circ)->purpose));
 
     /* If the path failed on an RP, retry it. */
-    if (TO_CIRCUIT(circ)->purpose == CIRCUIT_PURPOSE_S_CONNECT_REND)
+    if (TO_CIRCUIT(circ)->purpose == CIRCUIT_PURPOSE_S_CONNECT_REND) {
+      hs_metrics_failed_rdv(&circ->hs_ident->identity_pk);
       hs_circ_retry_service_rendezvous_point(circ);
+    }
 
     /* In all other cases, just bail. The rest is just failure accounting
      * that we don't want to do */
@@ -1862,6 +1865,8 @@ circuit_build_failed(origin_circuit_t *circ)
                "(%s hop failed).",
                escaped(build_state_get_exit_nickname(circ->build_state)),
                failed_at_last_hop?"last":"non-last");
+
+      hs_metrics_failed_rdv(&circ->hs_ident->identity_pk);
       hs_circ_retry_service_rendezvous_point(circ);
       break;
     /* default:
diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c
index 53855d40a9..bf8a2f3382 100644
--- a/src/feature/hs/hs_circuit.c
+++ b/src/feature/hs/hs_circuit.c
@@ -494,6 +494,7 @@ retry_service_rendezvous_point(const origin_circuit_t *circ)
   if (new_circ == NULL) {
     log_warn(LD_REND, "Failed to launch rendezvous circuit to %s",
              safe_str_client(extend_info_describe(bstate->chosen_exit)));
+    hs_metrics_failed_rdv(&circ->hs_ident->identity_pk);
     goto done;
   }
 
@@ -831,6 +832,7 @@ hs_circ_service_rp_has_opened(const hs_service_t *service,
 {
   size_t payload_len;
   uint8_t payload[RELAY_PAYLOAD_SIZE] = {0};
+  int reason = 0;
 
   tor_assert(service);
   tor_assert(circ);
@@ -862,10 +864,11 @@ hs_circ_service_rp_has_opened(const hs_service_t *service,
     payload_len = HS_LEGACY_RENDEZVOUS_CELL_SIZE;
   }
 
-  if (relay_send_command_from_edge(CONTROL_CELL_ID, TO_CIRCUIT(circ),
-                                   RELAY_COMMAND_RENDEZVOUS1,
-                                   (const char *) payload, payload_len,
-                                   circ->cpath->prev) < 0) {
+  if ((reason = relay_send_command_from_edge(CONTROL_CELL_ID, TO_CIRCUIT(circ),
+                                             RELAY_COMMAND_RENDEZVOUS1,
+                                             (const char *) payload,
+                                             payload_len,
+                                             circ->cpath->prev)) < 0) {
     /* On error, circuit is closed. */
     log_warn(LD_REND, "Unable to send RENDEZVOUS1 cell on circuit %u "
                       "for service %s",
@@ -875,15 +878,19 @@ hs_circ_service_rp_has_opened(const hs_service_t *service,
   }
 
   /* Setup end-to-end rendezvous circuit between the client and us. */
-  if (hs_circuit_setup_e2e_rend_circ(circ,
+  if ((reason = hs_circuit_setup_e2e_rend_circ(circ,
                        circ->hs_ident->rendezvous_ntor_key_seed,
                        sizeof(circ->hs_ident->rendezvous_ntor_key_seed),
-                       1) < 0) {
+                       1)) < 0) {
     log_warn(LD_GENERAL, "Failed to setup circ");
     goto done;
   }
 
  done:
+  if (reason < 0) {
+    hs_metrics_failed_rdv(&service->keys.identity_pk);
+  }
+
   memwipe(payload, 0, sizeof(payload));
 }
 
diff --git a/src/feature/hs/hs_metrics.c b/src/feature/hs/hs_metrics.c
index e80d98c2dd..854cd294c4 100644
--- a/src/feature/hs/hs_metrics.c
+++ b/src/feature/hs/hs_metrics.c
@@ -72,8 +72,8 @@ init_store(hs_service_t *service)
  * value used to update the entry. */
 void
 hs_metrics_update_by_service(const hs_metrics_key_t key,
-                             hs_service_t *service, const uint16_t port,
-                             int64_t n)
+                             const hs_service_t *service,
+                             uint16_t port, int64_t n)
 {
   tor_assert(service);
 
diff --git a/src/feature/hs/hs_metrics.h b/src/feature/hs/hs_metrics.h
index 2e0fa5048d..db02fefcea 100644
--- a/src/feature/hs/hs_metrics.h
+++ b/src/feature/hs/hs_metrics.h
@@ -28,13 +28,17 @@ 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);
 void hs_metrics_update_by_service(const hs_metrics_key_t key,
-                                  hs_service_t *service, const uint16_t port,
-                                  int64_t n);
+                                  const hs_service_t *service,
+                                  uint16_t port, int64_t n);
 
 /** New introducion request received. */
 #define hs_metrics_new_introduction(s) \
   hs_metrics_update_by_service(HS_METRICS_NUM_INTRODUCTIONS, (s), 0, 1)
 
+/** Introducion request rejected. */
+#define hs_metrics_reject_intro_req(s) \
+  hs_metrics_update_by_service(HS_METRICS_NUM_REJECTED_INTRO_REQ, (s), 0, 1)
+
 /** Number of bytes written to the application from the service. */
 #define hs_metrics_app_write_bytes(i, port, n) \
   hs_metrics_update_by_ident(HS_METRICS_APP_WRITE_BYTES, (i), (port), (n))
@@ -48,6 +52,10 @@ void hs_metrics_update_by_service(const hs_metrics_key_t key,
 #define hs_metrics_new_established_rdv(s) \
   hs_metrics_update_by_service(HS_METRICS_NUM_ESTABLISHED_RDV, (s), 0, 1)
 
+/** New rendezvous circuit failure. */
+#define hs_metrics_failed_rdv(i) \
+  hs_metrics_update_by_ident(HS_METRICS_NUM_FAILED_RDV, (i), 0, 1)
+
 /** Established rendezvous closed. This is called when the circuit in
  * REND_JOINED state is marked for close. */
 #define hs_metrics_close_established_rdv(i) \
diff --git a/src/feature/hs/hs_metrics_entry.c b/src/feature/hs/hs_metrics_entry.c
index 46d2d88aca..2d4d3ce4f6 100644
--- a/src/feature/hs/hs_metrics_entry.c
+++ b/src/feature/hs/hs_metrics_entry.c
@@ -45,13 +45,19 @@ const hs_metrics_entry_t base_metrics[] =
     .key = HS_METRICS_NUM_ESTABLISHED_RDV,
     .type = METRICS_TYPE_GAUGE,
     .name = METRICS_NAME(hs_rdv_established_count),
-    .help = "Total number of established rendezvous circuit",
+    .help = "Total number of established rendezvous circuits",
   },
   {
     .key = HS_METRICS_NUM_RDV,
     .type = METRICS_TYPE_COUNTER,
     .name = METRICS_NAME(hs_rdv_num_total),
-    .help = "Total number of rendezvous circuit created",
+    .help = "Total number of rendezvous circuits created",
+  },
+  {
+    .key = HS_METRICS_NUM_FAILED_RDV,
+    .type = METRICS_TYPE_COUNTER,
+    .name = METRICS_NAME(hs_rdv_error_count),
+    .help = "Total number of rendezvous circuit errors",
   },
   {
     .key = HS_METRICS_NUM_ESTABLISHED_INTRO,
@@ -59,6 +65,12 @@ const hs_metrics_entry_t base_metrics[] =
     .name = METRICS_NAME(hs_intro_established_count),
     .help = "Total number of established introduction circuit",
   },
+  {
+    .key = HS_METRICS_NUM_REJECTED_INTRO_REQ,
+    .type = METRICS_TYPE_COUNTER,
+    .name = METRICS_NAME(hs_intro_rejected_intro_req_count),
+    .help = "Total number of rejected introduction circuits",
+  },
 };
 
 /** Size of base_metrics array that is number of entries. */
diff --git a/src/feature/hs/hs_metrics_entry.h b/src/feature/hs/hs_metrics_entry.h
index b9786ac6f7..2f732aa614 100644
--- a/src/feature/hs/hs_metrics_entry.h
+++ b/src/feature/hs/hs_metrics_entry.h
@@ -25,8 +25,12 @@ typedef enum {
   HS_METRICS_NUM_ESTABLISHED_RDV = 3,
   /** Number of rendezsvous circuits created. */
   HS_METRICS_NUM_RDV = 4,
+  /** Number of failed rendezsvous. */
+  HS_METRICS_NUM_FAILED_RDV = 5,
   /** Number of established introducton points. */
-  HS_METRICS_NUM_ESTABLISHED_INTRO = 5,
+  HS_METRICS_NUM_ESTABLISHED_INTRO = 6,
+  /** Number of rejected introducton requests. */
+  HS_METRICS_NUM_REJECTED_INTRO_REQ = 7,
 } hs_metrics_key_t;
 
 /** The metadata of an HS metrics. */
diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c
index 479ac4670f..9c792b71c7 100644
--- a/src/feature/hs/hs_service.c
+++ b/src/feature/hs/hs_service.c
@@ -144,7 +144,7 @@ hs_service_ht_hash(const hs_service_t *service)
                                    sizeof(service->keys.identity_pk.pubkey));
 }
 
-/** This is _the_ global hash map of hidden services which indexed the service
+/** This is _the_ global hash map of hidden services which indexes the services
  * contained in it by master public identity key which is roughly the onion
  * address of the service. */
 static struct hs_service_ht *hs_service_map;
@@ -3524,6 +3524,10 @@ service_handle_introduce2(origin_circuit_t *circ, const uint8_t *payload,
 
   return 0;
  err:
+  if (service) {
+    hs_metrics_reject_intro_req(service);
+  }
+
   return -1;
 }
 
diff --git a/src/test/test_hs_metrics.c b/src/test/test_hs_metrics.c
index 8625933df7..03f6aedbb4 100644
--- a/src/test/test_hs_metrics.c
+++ b/src/test/test_hs_metrics.c
@@ -56,6 +56,23 @@ test_metrics(void *arg)
                                service, 0, 42);
   tt_int_op(metrics_store_entry_get_value(entry), OP_EQ, 84);
 
+  /* Update tor_hs_intro_rejected_intro_req_count */
+  hs_metrics_update_by_ident(HS_METRICS_NUM_REJECTED_INTRO_REQ,
+                             &service->keys.identity_pk, 0, 112);
+
+  entries = metrics_store_get_all(service->metrics.store,
+                                  "tor_hs_intro_rejected_intro_req_count");
+  tt_assert(entries);
+  tt_int_op(smartlist_len(entries), OP_EQ, 1);
+  entry = smartlist_get(entries, 0);
+  tt_assert(entry);
+  tt_int_op(metrics_store_entry_get_value(entry), OP_EQ, 112);
+
+  /* Update tor_hs_intro_rejected_intro_req_count entry by service now. */
+  hs_metrics_update_by_service(HS_METRICS_NUM_REJECTED_INTRO_REQ,
+                               service, 0, 10);
+  tt_int_op(metrics_store_entry_get_value(entry), OP_EQ, 122);
+
  done:
   hs_free_all();
 }
diff --git a/src/test/test_hs_service.c b/src/test/test_hs_service.c
index 482ee1a014..7e675620bd 100644
--- a/src/test/test_hs_service.c
+++ b/src/test/test_hs_service.c
@@ -1183,6 +1183,8 @@ test_bad_introduce2(void *arg)
   origin_circuit_t *circ = NULL;
   hs_service_t *service = NULL;
   hs_service_intro_point_t *ip = NULL;
+  const smartlist_t *entries = NULL;
+  const metrics_store_entry_t *entry = NULL;
 
   (void) arg;
 
@@ -1227,6 +1229,17 @@ test_bad_introduce2(void *arg)
                             "an INTRODUCE2 cell on circuit");
   teardown_capture_of_logs();
 
+  /* Make sure the tor_hs_intro_rejected_intro_req_count metric was
+   * incremented */
+  entries = metrics_store_get_all(service->metrics.store,
+                                  "tor_hs_intro_rejected_intro_req_count");
+
+  tt_assert(entries);
+  tt_int_op(smartlist_len(entries), OP_EQ, 1);
+  entry = smartlist_get(entries, 0);
+  tt_assert(entry);
+  tt_int_op(metrics_store_entry_get_value(entry), OP_EQ, 1);
+
   /* Set an IP object now for this circuit. */
   {
     ip = helper_create_service_ip();
@@ -1243,6 +1256,10 @@ test_bad_introduce2(void *arg)
   tt_int_op(ret, OP_EQ, -1);
   tt_u64_op(ip->introduce2_count, OP_EQ, 0);
 
+  /* Make sure the tor_hs_intro_rejected_intro_req_count metric was incremented
+   * a second time */
+  tt_int_op(metrics_store_entry_get_value(entry), OP_EQ, 2);
+
  done:
   or_state_free(dummy_state);
   dummy_state = NULL;

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the tor-commits mailing list