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(a)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(a)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.