commit e7abab878241592e262bad1bd64a14b10cc392a4 Author: David Goulet <dgoulet@torproject.org> Date: Wed Oct 20 10:40:56 2021 -0400 relay: For metrics, don't report DNS errors by query type This is due to the libevent bug https://github.com/libevent/libevent/issues/1219 that fails to return back the DNS record type on error. And so, the MetricsPort now only reports the errors as a global counter and not a per record type. Closes #40490 Signed-off-by: David Goulet <dgoulet@torproject.org> --- changes/ticket40490 | 5 +++++ src/feature/relay/relay_metrics.c | 14 ++++++++++++-- src/feature/stats/rephist.c | 18 ++++++++++++++++++ 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/changes/ticket40490 b/changes/ticket40490 new file mode 100644 index 0000000000..6e9ef50b42 --- /dev/null +++ b/changes/ticket40490 @@ -0,0 +1,5 @@ + o Major bugfix (relay, metrics): + - On the MetricsPort, the DNS error statistics are not reported by record + type ("record=...") anymore due to a libevent bug + (https://github.com/libevent/libevent/issues/1219). Fixes bug 40490; + bugfix on 0.4.7.1-alpha. diff --git a/src/feature/relay/relay_metrics.c b/src/feature/relay/relay_metrics.c index 07f89fc043..82abeeeee5 100644 --- a/src/feature/relay/relay_metrics.c +++ b/src/feature/relay/relay_metrics.c @@ -166,21 +166,24 @@ fill_dns_error_values(void) static const size_t num_errors = ARRAY_LENGTH(errors); for (size_t i = 0; i < num_dns_types; i++) { + /* NOTE: Disable the record type label until libevent is fixed. */ +#if 0 /* Dup the label because metrics_format_label() returns a pointer to a * string on the stack and we need that label for all metrics. */ char *record_label = tor_strdup(metrics_format_label("record", dns_types[i].name)); +#endif for (size_t j = 0; j < num_errors; j++) { sentry = metrics_store_add(the_store, rentry->type, rentry->name, rentry->help); - metrics_store_entry_add_label(sentry, record_label); + //metrics_store_entry_add_label(sentry, record_label); metrics_store_entry_add_label(sentry, metrics_format_label("reason", errors[j].name)); metrics_store_entry_update(sentry, rep_hist_get_n_dns_error(dns_types[i].type, errors[j].key)); } - tor_free(record_label); + //tor_free(record_label); } } @@ -192,6 +195,8 @@ fill_dns_query_values(void) const relay_metrics_entry_t *rentry = &base_metrics[RELAY_METRICS_NUM_DNS]; + /* NOTE: Disable the record type label until libevent is fixed (#40490). */ +#if 0 for (size_t i = 0; i < num_dns_types; i++) { /* Dup the label because metrics_format_label() returns a pointer to a * string on the stack and we need that label for all metrics. */ @@ -204,6 +209,11 @@ fill_dns_query_values(void) rep_hist_get_n_dns_request(dns_types[i].type)); tor_free(record_label); } +#endif + + sentry = metrics_store_add(the_store, rentry->type, rentry->name, + rentry->help); + metrics_store_entry_update(sentry, rep_hist_get_n_dns_request(0)); } /** Fill function for the RELAY_METRICS_NUM_GLOBAL_RW_LIMIT metrics. */ diff --git a/src/feature/stats/rephist.c b/src/feature/stats/rephist.c index aef12e5628..b4431f5a76 100644 --- a/src/feature/stats/rephist.c +++ b/src/feature/stats/rephist.c @@ -296,12 +296,29 @@ typedef struct { uint64_t stats_n_request; } dns_stats_t; +/* This is disabled because of the libevent bug where on error we don't get the + * DNS query type back. Once it is fixed, we can re-enable this. */ +#if 0 /** DNS statistics store for each DNS record type for which tor supports only * three at the moment: A, PTR and AAAA. */ static dns_stats_t dns_A_stats; static dns_stats_t dns_PTR_stats; static dns_stats_t dns_AAAA_stats; +#endif + +/** DNS query statistics store. It covers all type of queries. */ +static dns_stats_t dns_all_stats; + +/** Return the point to the DNS statistics store. Ignore the type for now + * because of a libevent problem. */ +static inline dns_stats_t * +get_dns_stats_by_type(const int type) +{ + (void) type; + return &dns_all_stats; +} +#if 0 /** From a libevent record type, return a pointer to the corresponding DNS * statistics store. NULL is returned if the type is unhandled. */ static inline dns_stats_t * @@ -318,6 +335,7 @@ get_dns_stats_by_type(const int type) return NULL; } } +#endif /** Assess the DNS timeout errors and if we have enough to trigger a general * overload. */