Merge pull request #180 from alltilla/http-request-metrics-fix

http: bind `output_http_requests_total` counter lifecycle to worker's

lib/metrics/CMakeLists.txt

@@ -1,12 +1,14 @@
 set(METRICS_HEADERS
     metrics/metrics.h
+    metrics/metrics-cache.h
     metrics/metrics-tls-cache.h
     metrics/metrics-template.h
     metrics/label-template.h
     PARENT_SCOPE)
 
 set(METRICS_SOURCES
     metrics/metrics.c
+    metrics/metrics-cache.c
     metrics/metrics-tls-cache.c
     metrics/metrics-template.c
     metrics/label-template.c

lib/metrics/Makefile.am

@@ -4,12 +4,14 @@ EXTRA_DIST += lib/metrics/CMakeLists.txt
 
 metricsinclude_HEADERS = \
 	lib/metrics/metrics.h	\
+	lib/metrics/metrics-cache.h	\
 	lib/metrics/metrics-tls-cache.h	\
 	lib/metrics/metrics-template.h	\
 	lib/metrics/label-template.h
 
 metrics_sources = \
 	lib/metrics/metrics.c	\
+	lib/metrics/metrics-cache.c	\
 	lib/metrics/metrics-tls-cache.c	\
 	lib/metrics/metrics-template.c	\
 	lib/metrics/label-template.c

lib/metrics/metrics-cache.c

@@ -0,0 +1,120 @@
+/*
+ * Copyright (c) 2023-2024 Attila Szakacs <[email protected]>
+ * Copyright (c) 2024 Balazs Scheidler <[email protected]>
+ * Copyright (c) 2024 Axoflow
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ * As an additional exemption you are allowed to compile & link against the
+ * OpenSSL libraries as published by the OpenSSL project. See the file
+ * COPYING for details.
+ *
+ */
+
+#include "metrics-cache.h"
+#include "stats/stats-cluster-single.h"
+
+struct _MetricsCache
+{
+  GHashTable *clusters;
+  GArray *label_buffers;
+};
+
+static StatsCluster *
+_register_single_cluster_locked(StatsClusterKey *key, gint stats_level)
+{
+  StatsCluster *cluster;
+
+  stats_lock();
+  {
+    StatsCounterItem *counter;
+    cluster = stats_register_dynamic_counter(stats_level, key, SC_TYPE_SINGLE_VALUE, &counter);
+  }
+  stats_unlock();
+
+  return cluster;
+}
+
+static void
+_unregister_single_cluster_locked(StatsCluster *cluster)
+{
+  stats_lock();
+  {
+    StatsCounterItem *counter = stats_cluster_single_get_counter(cluster);
+    stats_unregister_dynamic_counter(cluster, SC_TYPE_SINGLE_VALUE, &counter);
+  }
+  stats_unlock();
+}
+
+MetricsCache *
+metrics_cache_new(void)
+{
+  MetricsCache *self = g_new0(MetricsCache, 1);
+
+  self->clusters = g_hash_table_new_full((GHashFunc) stats_cluster_key_hash,
+                                         (GEqualFunc) stats_cluster_key_equal,
+                                         NULL,
+                                         (GDestroyNotify) _unregister_single_cluster_locked);
+  self->label_buffers = g_array_new(FALSE, FALSE, sizeof(StatsClusterLabel));
+
+  return self;
+}
+
+void
+metrics_cache_free(MetricsCache *self)
+{
+  g_hash_table_destroy(self->clusters);
+  g_array_free(self->label_buffers, TRUE);
+  g_free(self);
+}
+
+StatsCounterItem *
+metrics_cache_get_counter(MetricsCache *self, StatsClusterKey *key, gint level)
+{
+  StatsCluster *cluster = g_hash_table_lookup(self->clusters, key);
+  if (!cluster)
+    {
+      cluster = _register_single_cluster_locked(key, level);
+      if (cluster)
+        g_hash_table_insert(self->clusters, &cluster->key, cluster);
+    }
+
+  return stats_cluster_single_get_counter(cluster);
+}
+
+void
+metrics_cache_reset_labels(MetricsCache *self)
+{
+  self->label_buffers = g_array_set_size(self->label_buffers, 0);
+}
+
+StatsClusterLabel *
+metrics_cache_alloc_label(MetricsCache *self)
+{
+  self->label_buffers = g_array_set_size(self->label_buffers, self->label_buffers->len + 1);
+  return &g_array_index(self->label_buffers, StatsClusterLabel, self->label_buffers->len - 1);
+}
+
+StatsClusterLabel *
+metrics_cache_get_labels(MetricsCache *self)
+{
+  return (StatsClusterLabel *) self->label_buffers->data;
+}
+
+guint
+metrics_cache_get_labels_len(MetricsCache *self)
+{
+  return self->label_buffers->len;
+}

lib/metrics/metrics-cache.h

@@ -0,0 +1,62 @@
+/*
+ * Copyright (c) 2023-2024 Attila Szakacs <[email protected]>
+ * Copyright (c) 2024 Balazs Scheidler <[email protected]>
+ * Copyright (c) 2024 Axoflow
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ * As an additional exemption you are allowed to compile & link against the
+ * OpenSSL libraries as published by the OpenSSL project. See the file
+ * COPYING for details.
+ *
+ */
+
+#ifndef METRICS_CACHE_H_INCLUDED
+#define METRICS_CACHE_H_INCLUDED
+
+#include "stats/stats-registry.h"
+
+/*
+ * There is a recurring inconvenience with dynamic counters.
+ *
+ * Registering, changing and unregistering a counter makes it orphaned,
+ * as no one is keeping it alive. Non-dynamic counters do not have this
+ * issue, as they are always stored on their call-site, binding their
+ * lifecycle to the call-site.
+ *
+ * This class intends to solve this problem by providing a cache,
+ * which keeps alive its counters until the cache is freed. On the
+ * call-site you only need to keep the cache alive, to keep the
+ * counters alive.
+ *
+ * It also grants a label cache for performance optimization needs.
+ *
+ * Note: The cache is NOT thread safe, make sure to eliminate
+ * concurrency on the call site. If you need a cache that is bound
+ * to the current thread, see metrics/metrics-tls-cache.h.
+ */
+
+typedef struct _MetricsCache MetricsCache;
+
+MetricsCache *metrics_cache_new(void);
+void metrics_cache_free(MetricsCache *self);
+
+StatsCounterItem *metrics_cache_get_counter(MetricsCache *self, StatsClusterKey *key, gint level);
+void metrics_cache_reset_labels(MetricsCache *self);
+StatsClusterLabel *metrics_cache_alloc_label(MetricsCache *self);
+StatsClusterLabel *metrics_cache_get_labels(MetricsCache *self);
+guint metrics_cache_get_labels_len(MetricsCache *self);
+
+#endif

lib/metrics/metrics-template.c

@@ -62,7 +62,7 @@ _add_dynamic_labels_vp_helper(const gchar *name, LogMessageValueType type, const
   g_string_assign(name_buffer, name);
   g_string_append_len(value_buffer, value, value_len);
 
-  StatsClusterLabel *label = metrics_tls_cache_alloc_label();
+  StatsClusterLabel *label = metrics_cache_alloc_label(metrics_tls_cache());
   label->name = name_buffer->str;
   label->value = value_buffer->str;
 
@@ -87,21 +87,25 @@ metrics_template_build_sck(MetricsTemplate *self,
                            LogTemplateOptions *template_options,
                            LogMessage *msg, StatsClusterKey *key)
 {
-  metrics_tls_cache_reset_labels();
+  MetricsCache *tls_cache = metrics_tls_cache();
+
+  metrics_cache_reset_labels(tls_cache);
 
   for (GList *elem = g_list_first(self->label_templates); elem; elem = elem->next)
     {
       LabelTemplate *label_template = (LabelTemplate *) elem->data;
       GString *value_buffer = scratch_buffers_alloc();
 
       label_template_format(label_template, template_options, msg, value_buffer,
-                            metrics_tls_cache_alloc_label());
+                            metrics_cache_alloc_label(tls_cache));
     }
 
   if (self->vp)
     _add_dynamic_labels(self, template_options, msg);
 
-  stats_cluster_single_key_set(key, self->key, metrics_tls_cache_get_labels(), metrics_tls_cache_get_labels_len());
+  stats_cluster_single_key_set(key, self->key,
+                               metrics_cache_get_labels(tls_cache),
+                               metrics_cache_get_labels_len(tls_cache));
 }
 
 StatsCounterItem *
@@ -115,7 +119,7 @@ metrics_template_get_stats_counter(MetricsTemplate *self,
   scratch_buffers_mark(&marker);
   metrics_template_build_sck(self, template_options, msg, &key);
 
-  StatsCounterItem *counter = metrics_tls_cache_get_counter(&key, self->level);
+  StatsCounterItem *counter = metrics_cache_get_counter(metrics_tls_cache(), &key, self->level);
 
   scratch_buffers_reclaim_marked(marker);
   return counter;

lib/metrics/metrics-tls-cache.c

@@ -24,102 +24,35 @@
  */
 
 #include "metrics-tls-cache.h"
-#include "stats/stats-cluster-single.h"
 #include "apphook.h"
 #include "tls-support.h"
 
 TLS_BLOCK_START
 {
-  GHashTable *clusters;
-  GArray *label_buffers;
+  MetricsCache *metrics_cache;
 }
 TLS_BLOCK_END;
 
-#define clusters __tls_deref(clusters)
-#define label_buffers __tls_deref(label_buffers)
-
-static StatsCluster *
-_register_single_cluster_locked(StatsClusterKey *key, gint stats_level)
-{
-  StatsCluster *cluster;
-
-  stats_lock();
-  {
-    StatsCounterItem *counter;
-    cluster = stats_register_dynamic_counter(stats_level, key, SC_TYPE_SINGLE_VALUE, &counter);
-  }
-  stats_unlock();
-
-  return cluster;
-}
-
-static void
-_unregister_single_cluster_locked(StatsCluster *cluster)
-{
-  stats_lock();
-  {
-    StatsCounterItem *counter = stats_cluster_single_get_counter(cluster);
-    stats_unregister_dynamic_counter(cluster, SC_TYPE_SINGLE_VALUE, &counter);
-  }
-  stats_unlock();
-}
+#define metrics_cache __tls_deref(metrics_cache)
 
 static void
 _init_tls_cache(gpointer user_data)
 {
-  g_assert(!clusters && !label_buffers);
+  g_assert(!metrics_cache);
 
-  clusters = g_hash_table_new_full((GHashFunc) stats_cluster_key_hash,
-                                   (GEqualFunc) stats_cluster_key_equal,
-                                   NULL,
-                                   (GDestroyNotify) _unregister_single_cluster_locked);
-  label_buffers = g_array_new(FALSE, FALSE, sizeof(StatsClusterLabel));
+  metrics_cache = metrics_cache_new();
 }
 
 static void
 _deinit_tls_cache(gpointer user_data)
 {
-  g_hash_table_destroy(clusters);
-  g_array_free(label_buffers, TRUE);
-}
-
-StatsCounterItem *
-metrics_tls_cache_get_counter(StatsClusterKey *key, gint level)
-{
-  StatsCluster *cluster = g_hash_table_lookup(clusters, key);
-  if (!cluster)
-    {
-      cluster = _register_single_cluster_locked(key, level);
-      if (cluster)
-        g_hash_table_insert(clusters, &cluster->key, cluster);
-    }
-
-  return stats_cluster_single_get_counter(cluster);
-}
-
-void
-metrics_tls_cache_reset_labels(void)
-{
-  label_buffers = g_array_set_size(label_buffers, 0);
-}
-
-StatsClusterLabel *
-metrics_tls_cache_alloc_label(void)
-{
-  label_buffers = g_array_set_size(label_buffers, label_buffers->len + 1);
-  return &g_array_index(label_buffers, StatsClusterLabel, label_buffers->len - 1);
-}
-
-StatsClusterLabel *
-metrics_tls_cache_get_labels(void)
-{
-  return (StatsClusterLabel *) label_buffers->data;
+  metrics_cache_free(metrics_cache);
 }
 
-guint
-metrics_tls_cache_get_labels_len(void)
+MetricsCache *
+metrics_tls_cache(void)
 {
-  return label_buffers->len;
+  return metrics_cache;
 }
 
 void

lib/metrics/metrics-tls-cache.h

@@ -27,13 +27,9 @@
 #define METRICS_TLS_CACHE_H_INCLUDED
 
 #include "stats/stats-registry.h"
+#include "metrics-cache.h"
 
-StatsCounterItem *metrics_tls_cache_get_counter(StatsClusterKey *key, gint level);
-
-void metrics_tls_cache_reset_labels(void);
-StatsClusterLabel *metrics_tls_cache_alloc_label(void);
-StatsClusterLabel *metrics_tls_cache_get_labels(void);
-guint metrics_tls_cache_get_labels_len(void);
+MetricsCache *metrics_tls_cache(void);
 
 void metrics_tls_cache_global_init(void);
 void metrics_tls_cache_global_deinit(void);