[tor-commits] [tor/main] relay: For metrics, don't report DNS errors by query type

[dgoulet at torproject.org]

commit e7abab878241592e262bad1bd64a14b10cc392a4
Author: David Goulet <dgoulet at 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 at 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. */