From acacc6b6c2022b000b234cdf77ce9599d831b49a Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Fri, 5 Aug 2022 19:32:49 +0200 Subject: [PATCH] telemetry: Remove singleton BIFs and the C++ pieces The low-level singleton Telemetry BIFs have been removed with the that there haven't been any users. Singleton metrics can be instantiated by providing an empty label vector instead and aren't in any way a special concept. Closes #2262. --- NEWS | 4 + src/telemetry/Manager.cc | 204 ------------------------- src/telemetry/Manager.h | 75 --------- src/telemetry/telemetry.bif | 72 --------- testing/btest/telemetry/counter.zeek | 6 +- testing/btest/telemetry/gauge.zeek | 6 +- testing/btest/telemetry/histogram.zeek | 6 +- 7 files changed, 16 insertions(+), 357 deletions(-) diff --git a/NEWS b/NEWS index 9d2c25dba4..65fb7c4728 100644 --- a/NEWS +++ b/NEWS @@ -21,6 +21,10 @@ Breaking Changes - The Dictionary and PDict classes are now C++ templates. This may cause plugin/package builds to fail due to needing to modify uses of them to match. +- The low-level singleton Telemetry BIFs have been removed with the assumption that + there haven't been any users. Singleton metrics can be instantiated by providing + an empty label vector instead and aren't in any way a special concept. + New Functionality ----------------- diff --git a/src/telemetry/Manager.cc b/src/telemetry/Manager.cc index 2ff0a5063a..d263081414 100644 --- a/src/telemetry/Manager.cc +++ b/src/telemetry/Manager.cc @@ -515,60 +515,6 @@ template auto toVector(zeek::Span xs) } // namespace -SCENARIO("telemetry managers provide access to counter singletons") - { - GIVEN("a telemetry manager") - { - Manager mgr; - WHEN("retrieving an IntCounter singleton") - { - auto first = mgr.CounterSingleton("zeek", "int-count", "test"); - THEN("its initial value is zero") { CHECK_EQ(first.Value(), 0); } - AND_THEN("calling Inc() or operator++ changes the value") - { - first.Inc(); - CHECK_EQ(first.Value(), 1); - first.Inc(2); - CHECK_EQ(first.Value(), 3); - CHECK_EQ(++first, 4); - CHECK_EQ(first.Value(), 4); - } - AND_THEN("calling counterSingleton again for the same name returns the same handle") - { - auto second = mgr.CounterSingleton("zeek", "int-count", "test"); - CHECK_EQ(first, second); - } - AND_THEN("calling counterSingleton for a different name returns another handle") - { - auto third = mgr.CounterSingleton("zeek", "int-count-2", "test"); - CHECK_NE(first, third); - } - } - WHEN("retrieving a DblCounter singleton") - { - auto first = mgr.CounterSingleton("zeek", "dbl-count", "test"); - THEN("its initial value is zero") { CHECK_EQ(first.Value(), 0.0); } - AND_THEN("calling Inc() changes the value") - { - first.Inc(); - CHECK_EQ(first.Value(), 1.0); - first.Inc(3.0); - CHECK_EQ(first.Value(), 4.0); - } - AND_THEN("calling counterSingleton again for the same name returns the same handle") - { - auto second = mgr.CounterSingleton("zeek", "dbl-count", "test"); - CHECK_EQ(first, second); - } - AND_THEN("calling counterSingleton for a different name returns another handle") - { - auto third = mgr.CounterSingleton("zeek", "dbl-count-2", "test"); - CHECK_NE(first, third); - } - } - } - } - SCENARIO("telemetry managers provide access to counter families") { GIVEN("a telemetry manager") @@ -628,70 +574,6 @@ SCENARIO("telemetry managers provide access to counter families") } } -SCENARIO("telemetry managers provide access to gauge singletons") - { - GIVEN("a telemetry manager") - { - Manager mgr; - WHEN("retrieving an IntGauge singleton") - { - auto first = mgr.GaugeSingleton("zeek", "int-gauge", "test"); - THEN("its initial value is zero") { CHECK_EQ(first.Value(), 0); } - AND_THEN("calling Inc(), Dec(), operator++ or operator-- changes the value") - { - first.Inc(); - CHECK_EQ(first.Value(), 1); - first.Inc(2); - CHECK_EQ(first.Value(), 3); - first.Dec(); - CHECK_EQ(first.Value(), 2); - CHECK_EQ(++first, 3); - CHECK_EQ(first.Value(), 3); - CHECK_EQ(--first, 2); - CHECK_EQ(first.Value(), 2); - first.Dec(2); - CHECK_EQ(first.Value(), 0); - } - AND_THEN("calling gaugeSingleton again for the same name returns the same handle") - { - auto second = mgr.GaugeSingleton("zeek", "int-gauge", "test"); - CHECK_EQ(first, second); - } - AND_THEN("calling gaugeSingleton for a different name returns another handle") - { - auto third = mgr.GaugeSingleton("zeek", "int-gauge-2", "test"); - CHECK_NE(first, third); - } - } - WHEN("retrieving a DblGauge singleton") - { - auto first = mgr.GaugeSingleton("zeek", "dbl-gauge", "test"); - THEN("its initial value is zero") { CHECK_EQ(first.Value(), 0.0); } - AND_THEN("calling Inc() or Dec() changes the value") - { - first.Inc(); - CHECK_EQ(first.Value(), 1.0); - first.Inc(3.0); - CHECK_EQ(first.Value(), 4.0); - first.Dec(2.0); - CHECK_EQ(first.Value(), 2.0); - first.Dec(); - CHECK_EQ(first.Value(), 1.0); - } - AND_THEN("calling gaugeSingleton again for the same name returns the same handle") - { - auto second = mgr.GaugeSingleton("zeek", "dbl-gauge", "test"); - CHECK_EQ(first, second); - } - AND_THEN("calling gaugeSingleton for a different name returns another handle") - { - auto third = mgr.GaugeSingleton("zeek", "dbl-gauge-2", "test"); - CHECK_NE(first, third); - } - } - } - } - SCENARIO("telemetry managers provide access to gauge families") { GIVEN("a telemetry manager") @@ -751,92 +633,6 @@ SCENARIO("telemetry managers provide access to gauge families") } } -SCENARIO("telemetry managers provide access to histogram singletons") - { - GIVEN("a telemetry manager") - { - Manager mgr; - WHEN("retrieving an IntHistogram singleton") - { - const auto max_int = std::numeric_limits::max(); - int64_t buckets[] = {10, 20}; - auto first = mgr.HistogramSingleton("zeek", "int-hist", buckets, "test"); - THEN("it initially has no observations") - { - REQUIRE_EQ(first.NumBuckets(), 3u); - CHECK_EQ(first.Sum(), 0); - CHECK_EQ(first.CountAt(0), 0); - CHECK_EQ(first.CountAt(1), 0); - CHECK_EQ(first.CountAt(2), 0); - CHECK_EQ(first.UpperBoundAt(0), 10); - CHECK_EQ(first.UpperBoundAt(1), 20); - CHECK_EQ(first.UpperBoundAt(2), max_int); - } - AND_THEN("calling Observe() increments bucket counters") - { - first.Observe(1); - first.Observe(9); - first.Observe(10); - first.Observe(11); - first.Observe(19); - first.Observe(20); - first.Observe(21); - CHECK_EQ(first.Sum(), 91); - CHECK_EQ(first.CountAt(0), 3); - CHECK_EQ(first.CountAt(1), 3); - CHECK_EQ(first.CountAt(2), 1); - } - AND_THEN("calling HistogramSingleton again for the same name returns the same handle") - { - auto second = mgr.HistogramSingleton("zeek", "int-hist", buckets, "test"); - CHECK_EQ(first, second); - } - AND_THEN("calling HistogramSingleton for a different name returns another handle") - { - auto third = mgr.HistogramSingleton("zeek", "int-hist-2", buckets, "test"); - CHECK_NE(first, third); - } - } - WHEN("retrieving a DblHistogram singleton") - { - double buckets[] = {10.0, 20.0}; - auto first = mgr.HistogramSingleton("zeek", "dbl-count", buckets, "test"); - THEN("it initially has no observations") - { - REQUIRE_EQ(first.NumBuckets(), 3u); - CHECK_EQ(first.Sum(), 0.0); - CHECK_EQ(first.CountAt(0), 0); - CHECK_EQ(first.CountAt(1), 0); - CHECK_EQ(first.CountAt(2), 0); - CHECK_EQ(first.UpperBoundAt(0), 10.0); - CHECK_EQ(first.UpperBoundAt(1), 20.0); - } - AND_THEN("calling Observe() increments bucket counters") - { - first.Observe(2.0); - first.Observe(4.0); - first.Observe(8.0); - first.Observe(16.0); - first.Observe(32.0); - CHECK_EQ(first.Sum(), 62.0); - CHECK_EQ(first.CountAt(0), 3); - CHECK_EQ(first.CountAt(1), 1); - CHECK_EQ(first.CountAt(2), 1); - } - AND_THEN("calling histogramSingleton again for the same name returns the same handle") - { - auto second = mgr.HistogramSingleton("zeek", "dbl-count", buckets, "test"); - CHECK_EQ(first, second); - } - AND_THEN("calling histogramSingleton for a different name returns another handle") - { - auto third = mgr.HistogramSingleton("zeek", "dbl-count-2", buckets, "test"); - CHECK_NE(first, third); - } - } - } - } - SCENARIO("telemetry managers provide access to histogram families") { GIVEN("a telemetry manager") diff --git a/src/telemetry/Manager.h b/src/telemetry/Manager.h index acf951e9e2..49293fdff7 100644 --- a/src/telemetry/Manager.h +++ b/src/telemetry/Manager.h @@ -255,27 +255,6 @@ public: return CounterInstance(prefix, name, lbl_span, helptext, unit, is_sum); } - /** - * Accesses a counter singleton, i.e., a counter that belongs to a family - * without label dimensions (which thus only has a single member). Creates - * the hosting metric family as well as the counter lazily if necessary. - * @param prefix The prefix (namespace) this family belongs to. - * @param name The human-readable name of the metric, e.g., `requests`. - * @param helptext Short explanation of the metric. - * @param unit Unit of measurement. - * @param is_sum Indicates whether this metric accumulates something, where - * only the total value is of interest. - */ - template - Counter CounterSingleton(std::string_view prefix, std::string_view name, - std::string_view helptext, std::string_view unit = "1", - bool is_sum = false) - { - auto labels = Span{}; - auto fam = CounterFamily(prefix, name, labels, helptext, unit, is_sum); - return fam.GetOrAdd({}); - } - /** * @return A gauge metric family. Creates the family lazily if necessary. * @param prefix The prefix (namespace) this family belongs to. @@ -351,27 +330,6 @@ public: return GaugeInstance(prefix, name, lbl_span, helptext, unit, is_sum); } - /** - * Accesses a gauge singleton, i.e., a gauge that belongs to a family - * without label dimensions (which thus only has a single member). Creates - * the hosting metric family as well as the gauge lazily if necessary. - * @param prefix The prefix (namespace) this family belongs to. - * @param name The human-readable name of the metric, e.g., `requests`. - * @param helptext Short explanation of the metric. - * @param unit Unit of measurement. - * @param is_sum Indicates whether this metric accumulates something, where - * only the total value is of interest. - */ - template - Gauge GaugeSingleton(std::string_view prefix, std::string_view name, - std::string_view helptext, std::string_view unit = "1", - bool is_sum = false) - { - auto labels = Span{}; - auto fam = GaugeFamily(prefix, name, labels, helptext, unit, is_sum); - return fam.GetOrAdd({}); - } - // Forces the compiler to use the type `Span` instead of trying to // match paremeters to a `span`. template struct ConstSpanOracle @@ -485,39 +443,6 @@ public: return HistogramInstance(prefix, name, lbls, default_upper_bounds, helptext, unit, is_sum); } - /** - * Returns a histogram metric singleton, i.e., the single instance of a - * family without label dimensions. Creates all objects lazily if necessary, - * but fails if the full name already belongs to a different family. - * @param prefix The prefix (namespace) this family belongs to. Usually the - * application or protocol name, e.g., `http`. The prefix `caf` - * as well as prefixes starting with an underscore are - * reserved. - * @param name The human-readable name of the metric, e.g., `requests`. - * @param default_upper_bounds Upper bounds for the metric buckets. - * @param helptext Short explanation of the metric. - * @param unit Unit of measurement. Please use base units such as `bytes` or - * `seconds` (prefer lowercase). The pseudo-unit `1` identifies - * dimensionless counts. - * @param is_sum Setting this to `true` indicates that this metric adds - * something up to a total, where only the total value is of - * interest. For example, the total number of HTTP requests. - * @note The first call wins when calling this function multiple times with - * different bucket settings. Users may also override - * @p default_upper_bounds via run-time configuration. - */ - template - Histogram HistogramSingleton(std::string_view prefix, std::string_view name, - ConstSpan default_upper_bounds, - std::string_view helptext, std::string_view unit = "1", - bool is_sum = false) - { - auto lbls = Span{}; - auto fam = HistogramFamily(prefix, name, lbls, default_upper_bounds, helptext, - unit, is_sum); - return fam.GetOrAdd({}); - } - protected: template static void WithLabelNames(Span xs, F continuation) { diff --git a/src/telemetry/telemetry.bif b/src/telemetry/telemetry.bif index 3a8ad64f8a..7b3b423a86 100644 --- a/src/telemetry/telemetry.bif +++ b/src/telemetry/telemetry.bif @@ -147,17 +147,6 @@ function Telemetry::__int_counter_metric_get_or_add%(family: opaque of int_count } %} -function Telemetry::__int_counter_singleton%(prefix: string, - name: string, - helptext: string &default = "Zeek Script Metric", - unit: string &default = "1", - is_sum: bool &default = F%): opaque of int_counter_metric - %{ - auto hdl = telemetry_mgr->CounterSingleton(sv(prefix), sv(name), - sv(helptext), sv(unit), is_sum); - return zeek::make_intrusive(hdl); - %} - function Telemetry::__int_counter_inc%(val: opaque of int_counter_metric, amount: int &default = 1%): bool %{ @@ -218,17 +207,6 @@ function Telemetry::__dbl_counter_metric_get_or_add%(family: opaque of dbl_count } %} -function Telemetry::__dbl_counter_singleton%(prefix: string, - name: string, - helptext: string &default = "Zeek Script Metric", - unit: string &default = "1", - is_sum: bool &default = F%): opaque of dbl_counter_metric - %{ - auto hdl = telemetry_mgr->CounterSingleton(sv(prefix), sv(name), - sv(helptext), sv(unit), is_sum); - return zeek::make_intrusive(hdl); - %} - function Telemetry::__dbl_counter_inc%(val: opaque of dbl_counter_metric, amount: double &default = 1.0%): bool %{ @@ -289,17 +267,6 @@ function Telemetry::__int_gauge_metric_get_or_add%(family: opaque of int_gauge_m } %} -function Telemetry::__int_gauge_singleton%(prefix: string, - name: string, - helptext: string &default = "Zeek Script Metric", - unit: string &default = "1", - is_sum: bool &default = F%): opaque of int_gauge_metric - %{ - auto hdl = telemetry_mgr->GaugeSingleton(sv(prefix), sv(name), - sv(helptext), sv(unit), is_sum); - return zeek::make_intrusive(hdl); - %} - function Telemetry::__int_gauge_inc%(val: opaque of int_gauge_metric, amount: int &default = 1%): bool %{ @@ -366,17 +333,6 @@ function Telemetry::__dbl_gauge_metric_get_or_add%(family: opaque of dbl_gauge_m } %} -function Telemetry::__dbl_gauge_singleton%(prefix: string, - name: string, - helptext: string &default = "Zeek Script Metric", - unit: string &default = "1", - is_sum: bool &default = F%): opaque of dbl_gauge_metric - %{ - auto hdl = telemetry_mgr->GaugeSingleton(sv(prefix), sv(name), - sv(helptext), sv(unit), is_sum); - return zeek::make_intrusive(hdl); - %} - function Telemetry::__dbl_gauge_inc%(val: opaque of dbl_gauge_metric, amount: double &default = 1.0%): bool %{ @@ -446,20 +402,6 @@ function Telemetry::__int_histogram_metric_get_or_add%(family: opaque of int_his } %} -function Telemetry::__int_histogram_singleton%(prefix: string, - name: string, - bounds: int_vec, - helptext: string &default = "Zeek Script Metric", - unit: string &default = "1", - is_sum: bool &default = F%): opaque of int_histogram_metric - %{ - auto std_bounds = to_std_vec(bounds); - auto hdl = telemetry_mgr->HistogramSingleton(sv(prefix), sv(name), - std_bounds, sv(helptext), - sv(unit), is_sum); - return zeek::make_intrusive(hdl); - %} - function Telemetry::__int_histogram_observe%(val: opaque of int_histogram_metric, measurement: int%): bool %{ @@ -524,20 +466,6 @@ function Telemetry::__dbl_histogram_metric_get_or_add%(family: opaque of dbl_his } %} -function Telemetry::__dbl_histogram_singleton%(prefix: string, - name: string, - bounds: double_vec, - helptext: string &default = "Zeek Script Metric", - unit: string &default = "1", - is_sum: bool &default = F%): opaque of dbl_histogram_metric - %{ - auto std_bounds = to_std_vec(bounds); - auto hdl = telemetry_mgr->HistogramSingleton(sv(prefix), sv(name), - std_bounds, sv(helptext), - sv(unit), is_sum); - return zeek::make_intrusive(hdl); - %} - function Telemetry::__dbl_histogram_observe%(val: opaque of dbl_histogram_metric, measurement: double%): bool %{ diff --git a/testing/btest/telemetry/counter.zeek b/testing/btest/telemetry/counter.zeek index efe062e46d..1aee71a813 100644 --- a/testing/btest/telemetry/counter.zeek +++ b/testing/btest/telemetry/counter.zeek @@ -4,20 +4,22 @@ # @TEST-EXEC: btest-diff output global cnt1 = Telemetry::__int_counter_family("cnt1", "bar", vector("dim1", "dim2")); -global cnt2_bar = Telemetry::__int_counter_singleton("cnt2", "bar"); +global cnt2 = Telemetry::__int_counter_family("cnt2", "bar", vector()); global cnt3 = Telemetry::__dbl_counter_family("cnt3", "bar", vector("dim1", "dim2")); -global cnt4_bar = Telemetry::__dbl_counter_singleton("cnt4", "bar"); +global cnt4 = Telemetry::__dbl_counter_family("cnt4", "bar", vector()); event zeek_init() { local cnt1_bar = Telemetry::__int_counter_metric_get_or_add(cnt1, table(["dim1"] = "val1", ["dim2"] = "val2")); Telemetry::__int_counter_inc(cnt1_bar); + local cnt2_bar = Telemetry::__int_counter_metric_get_or_add(cnt2, table()); Telemetry::__int_counter_inc(cnt2_bar); Telemetry::__int_counter_inc(cnt2_bar, 41); print fmt("cnt1_bar: %d", Telemetry::__int_counter_value(cnt1_bar)); print fmt("cnt2_bar: %d", Telemetry::__int_counter_value(cnt2_bar)); local cnt3_bar = Telemetry::__dbl_counter_metric_get_or_add(cnt3, table(["dim1"] = "val1", ["dim2"] = "val2")); Telemetry::__dbl_counter_inc(cnt3_bar); + local cnt4_bar = Telemetry::__dbl_counter_metric_get_or_add(cnt4, table()); Telemetry::__dbl_counter_inc(cnt4_bar); Telemetry::__dbl_counter_inc(cnt4_bar, 41.0); print fmt("cnt3_bar: %f", Telemetry::__dbl_counter_value(cnt3_bar)); diff --git a/testing/btest/telemetry/gauge.zeek b/testing/btest/telemetry/gauge.zeek index 5966571dbc..80369f8513 100644 --- a/testing/btest/telemetry/gauge.zeek +++ b/testing/btest/telemetry/gauge.zeek @@ -4,14 +4,15 @@ # @TEST-EXEC: btest-diff output global gg1 = Telemetry::__int_gauge_family("gg1", "bar", vector("dim1", "dim2")); -global gg2_bar = Telemetry::__int_gauge_singleton("gg2", "bar"); +global gg2 = Telemetry::__int_gauge_family("gg2", "bar", vector()); global gg3 = Telemetry::__dbl_gauge_family("gg3", "bar", vector("dim1", "dim2")); -global gg4_bar = Telemetry::__dbl_gauge_singleton("gg4", "bar"); +global gg4 = Telemetry::__dbl_gauge_family("gg4", "bar", vector()); event zeek_init() { local gg1_bar = Telemetry::__int_gauge_metric_get_or_add(gg1, table(["dim1"] = "val1", ["dim2"] = "val2")); Telemetry::__int_gauge_inc(gg1_bar); + local gg2_bar = Telemetry::__int_gauge_metric_get_or_add(gg2, table()); Telemetry::__int_gauge_inc(gg2_bar); Telemetry::__int_gauge_inc(gg2_bar, 41); Telemetry::__int_gauge_dec(gg2_bar); @@ -20,6 +21,7 @@ event zeek_init() print fmt("gg2_bar: %d", Telemetry::__int_gauge_value(gg2_bar)); local gg3_bar = Telemetry::__dbl_gauge_metric_get_or_add(gg3, table(["dim1"] = "val1", ["dim2"] = "val2")); Telemetry::__dbl_gauge_inc(gg3_bar); + local gg4_bar = Telemetry::__dbl_gauge_metric_get_or_add(gg4, table()); Telemetry::__dbl_gauge_inc(gg4_bar); Telemetry::__dbl_gauge_inc(gg4_bar, 41.0); Telemetry::__dbl_gauge_dec(gg4_bar); diff --git a/testing/btest/telemetry/histogram.zeek b/testing/btest/telemetry/histogram.zeek index de183872b0..aec7b2a2cd 100644 --- a/testing/btest/telemetry/histogram.zeek +++ b/testing/btest/telemetry/histogram.zeek @@ -7,21 +7,23 @@ const int_bounds = vector(+10, +20); const dbl_bounds = vector(10.0, 20.0); global hst1 = Telemetry::__int_histogram_family("hst1", "bar", vector("dim1", "dim2"), int_bounds); -global hst2_bar = Telemetry::__int_histogram_singleton("hst2", "bar", int_bounds); +global hst2 = Telemetry::__int_histogram_family("hst2", "bar", vector(), int_bounds); global hst3 = Telemetry::__dbl_histogram_family("hst3", "bar", vector("dim1", "dim2"), dbl_bounds); -global hst4_bar = Telemetry::__dbl_histogram_singleton("hst4", "bar", dbl_bounds); +global hst4 = Telemetry::__dbl_histogram_family("hst4", "bar", vector(), dbl_bounds); event zeek_init() { local hst1_bar = Telemetry::__int_histogram_metric_get_or_add(hst1, table(["dim1"] = "val1", ["dim2"] = "val2")); Telemetry::__int_histogram_observe(hst1_bar, 1); Telemetry::__int_histogram_observe(hst1_bar, 11); + local hst2_bar = Telemetry::__int_histogram_metric_get_or_add(hst2, table()); Telemetry::__int_histogram_observe(hst2_bar, 31337); print fmt("hst1_bar: %d", Telemetry::__int_histogram_sum(hst1_bar)); print fmt("hst2_bar: %d", Telemetry::__int_histogram_sum(hst2_bar)); local hst3_bar = Telemetry::__dbl_histogram_metric_get_or_add(hst3, table(["dim1"] = "val1", ["dim2"] = "val2")); Telemetry::__dbl_histogram_observe(hst3_bar, 2.0); Telemetry::__dbl_histogram_observe(hst3_bar, 4.0); + local hst4_bar = Telemetry::__dbl_histogram_metric_get_or_add(hst4, table()); Telemetry::__dbl_histogram_observe(hst4_bar, 64.0); print fmt("hst3_bar: %f", Telemetry::__dbl_histogram_sum(hst3_bar)); print fmt("hst4_bar: %f", Telemetry::__dbl_histogram_sum(hst4_bar));