From b1eb76801cb16f3c685e23ea361fb081d07c73da Mon Sep 17 00:00:00 2001 From: Attila Szakacs Date: Thu, 2 Jan 2025 15:15:57 +0100 Subject: [PATCH] metrics-probe/filterx/func-update-metric: use optimize() method Signed-off-by: Attila Szakacs --- lib/filterx/filterx-metrics-labels.c | 78 +++++++++++-------- lib/filterx/filterx-metrics-labels.h | 1 + lib/filterx/filterx-metrics.c | 73 +++++++++-------- lib/filterx/filterx-metrics.h | 1 + lib/filterx/tests/test_metrics_labels.c | 10 +++ .../filterx/func-update-metric.c | 55 +++++++------ 6 files changed, 127 insertions(+), 91 deletions(-) diff --git a/lib/filterx/filterx-metrics-labels.c b/lib/filterx/filterx-metrics-labels.c index e373f5c38..689055fea 100644 --- a/lib/filterx/filterx-metrics-labels.c +++ b/lib/filterx/filterx-metrics-labels.c @@ -99,6 +99,23 @@ _label_cmp(gconstpointer a, gconstpointer b) return strcmp(lhs->name, rhs->name); } +static void +_label_optimize(FilterXMetricsLabel *self) +{ + self->value.expr = filterx_expr_optimize(self->value.expr); + + if (!filterx_expr_is_literal(self->value.expr)) + return; + + ScratchBuffersMarker marker; + scratch_buffers_mark(&marker); + self->value.str = g_strdup(_format_value_expr(self->value.expr)); + scratch_buffers_reclaim_marked(marker); + + filterx_expr_unref(self->value.expr); + self->value.expr = NULL; +} + static gboolean _label_init(FilterXMetricsLabel *self, GlobalConfig *cfg) { @@ -140,23 +157,6 @@ _init_label_name(FilterXExpr *name) return str; } -static gboolean -_init_label_value(FilterXMetricsLabel *self, FilterXExpr *value) -{ - if (!filterx_expr_is_literal(value)) - { - self->value.expr = filterx_expr_ref(value); - return TRUE; - } - - ScratchBuffersMarker marker; - scratch_buffers_mark(&marker); - self->value.str = g_strdup(_format_value_expr(value)); - scratch_buffers_reclaim_marked(marker); - - return !!self->value.str; -} - static FilterXMetricsLabel * _label_new(FilterXExpr *name, FilterXExpr *value) { @@ -166,8 +166,7 @@ _label_new(FilterXExpr *name, FilterXExpr *value) if (!self->name) goto error; - if (!_init_label_value(self, value)) - goto error; + self->value.expr = filterx_expr_ref(value); return self; @@ -317,6 +316,34 @@ filterx_metrics_labels_format(FilterXMetricsLabels *self, DynMetricsStore *store return TRUE; } +static gboolean +_calculate_constness(FilterXMetricsLabels *self) +{ + if (self->expr) + return FALSE; + + gboolean is_const = TRUE; + g_ptr_array_foreach(self->literal_labels, _check_label_is_const, &is_const); + return is_const; +} + +void +filterx_metrics_labels_optimize(FilterXMetricsLabels *self) +{ + self->expr = filterx_expr_optimize(self->expr); + + if (self->literal_labels) + { + for (guint i = 0; i < self->literal_labels->len; i++) + { + FilterXMetricsLabel *label = g_ptr_array_index(self->literal_labels, i); + _label_optimize(label); + } + } + + self->is_const = _calculate_constness(self); +} + gboolean filterx_metrics_labels_init(FilterXMetricsLabels *self, GlobalConfig *cfg) { @@ -419,17 +446,6 @@ _init_labels(FilterXMetricsLabels *self, FilterXExpr *labels) return TRUE; } -static gboolean -_calculate_constness(FilterXMetricsLabels *self) -{ - if (self->expr) - return FALSE; - - gboolean is_const = TRUE; - g_ptr_array_foreach(self->literal_labels, _check_label_is_const, &is_const); - return is_const; -} - FilterXMetricsLabels * filterx_metrics_labels_new(FilterXExpr *labels) { @@ -441,7 +457,7 @@ filterx_metrics_labels_new(FilterXExpr *labels) return NULL; } - self->is_const = _calculate_constness(self); + self->is_const = FALSE; return self; } diff --git a/lib/filterx/filterx-metrics-labels.h b/lib/filterx/filterx-metrics-labels.h index faf3317fd..1792f8278 100644 --- a/lib/filterx/filterx-metrics-labels.h +++ b/lib/filterx/filterx-metrics-labels.h @@ -31,6 +31,7 @@ typedef struct _FilterXMetricsLabels FilterXMetricsLabels; FilterXMetricsLabels *filterx_metrics_labels_new(FilterXExpr *labels); +void filterx_metrics_labels_optimize(FilterXMetricsLabels *self); gboolean filterx_metrics_labels_init(FilterXMetricsLabels *self, GlobalConfig *cfg); void filterx_metrics_labels_deinit(FilterXMetricsLabels *self, GlobalConfig *cfg); void filterx_metrics_labels_free(FilterXMetricsLabels *self); diff --git a/lib/filterx/filterx-metrics.c b/lib/filterx/filterx-metrics.c index bcce4c6bf..17c8ffc08 100644 --- a/lib/filterx/filterx-metrics.c +++ b/lib/filterx/filterx-metrics.c @@ -104,7 +104,31 @@ _is_const(FilterXMetrics *self) } static void -_optimize(FilterXMetrics *self) +_optimize_key(FilterXMetrics *self) +{ + self->key.expr = filterx_expr_optimize(self->key.expr); + + if (!filterx_expr_is_literal(self->key.expr)) + return; + + FilterXObject *key_obj = filterx_expr_eval_typed(self->key.expr); + if (!filterx_object_is_type(key_obj, &FILTERX_TYPE_NAME(string))) + { + filterx_object_unref(key_obj); + return; + } + + /* There are no literal message values, so we don't need to call extract_string() here. */ + self->key.str = g_strdup(filterx_string_get_value_ref(key_obj, NULL)); + + filterx_expr_unref(self->key.expr); + self->key.expr = NULL; + + filterx_object_unref(key_obj); +} + +static void +_optimize_counter(FilterXMetrics *self) { DynMetricsStore *store = dyn_metrics_cache(); @@ -136,6 +160,14 @@ _optimize(FilterXMetrics *self) stats_unlock(); } +void +filterx_metrics_optimize(FilterXMetrics *self) +{ + _optimize_key(self); + filterx_metrics_labels_optimize(self->labels); + _optimize_counter(self); +} + gboolean filterx_metrics_get_stats_counter(FilterXMetrics *self, StatsCounterItem **counter) { @@ -182,8 +214,6 @@ filterx_metrics_init(FilterXMetrics *self, GlobalConfig *cfg) return FALSE; } - _optimize(self); - return TRUE; } @@ -213,36 +243,6 @@ filterx_metrics_free(FilterXMetrics *self) g_free(self); } -static gboolean -_init_key(FilterXMetrics *self, FilterXExpr *key) -{ - if (!filterx_expr_is_literal(key)) - { - self->key.expr = filterx_expr_ref(key); - return TRUE; - } - - FilterXObject *key_obj = filterx_expr_eval_typed(key); - if (!filterx_object_is_type(key_obj, &FILTERX_TYPE_NAME(string))) - { - filterx_eval_push_error_info("failed to init metrics key, key must be a string", key, - g_strdup_printf("got %s instread", key_obj->type->name), TRUE); - filterx_object_unref(key_obj); - return FALSE; - } - - /* There are no literal message values, so we don't need to call extract_string() here. */ - self->key.str = g_strdup(filterx_string_get_value_ref(key_obj, NULL)); - return TRUE; -} - -static gboolean -_init_labels(FilterXMetrics *self, FilterXExpr *labels) -{ - self->labels = filterx_metrics_labels_new(labels); - return !!self->labels; -} - FilterXMetrics * filterx_metrics_new(gint level, FilterXExpr *key, FilterXExpr *labels) { @@ -251,11 +251,10 @@ filterx_metrics_new(gint level, FilterXExpr *key, FilterXExpr *labels) g_assert(key); self->level = level; + self->key.expr = filterx_expr_ref(key); - if (!_init_key(self, key)) - goto error; - - if (!_init_labels(self, labels)) + self->labels = filterx_metrics_labels_new(labels); + if (!self->labels) goto error; return self; diff --git a/lib/filterx/filterx-metrics.h b/lib/filterx/filterx-metrics.h index d850d0f22..38c270428 100644 --- a/lib/filterx/filterx-metrics.h +++ b/lib/filterx/filterx-metrics.h @@ -34,6 +34,7 @@ gboolean filterx_metrics_is_enabled(FilterXMetrics *self); gboolean filterx_metrics_get_stats_counter(FilterXMetrics *self, StatsCounterItem **counter); FilterXMetrics *filterx_metrics_new(gint level, FilterXExpr *key, FilterXExpr *labels); +void filterx_metrics_optimize(FilterXMetrics *self); gboolean filterx_metrics_init(FilterXMetrics *self, GlobalConfig *cfg); void filterx_metrics_deinit(FilterXMetrics *self, GlobalConfig *cfg); void filterx_metrics_free(FilterXMetrics *self); diff --git a/lib/filterx/tests/test_metrics_labels.c b/lib/filterx/tests/test_metrics_labels.c index 85fdf1950..5f4e5d232 100644 --- a/lib/filterx/tests/test_metrics_labels.c +++ b/lib/filterx/tests/test_metrics_labels.c @@ -45,6 +45,8 @@ Test(filterx_metrics_labels, null_labels) FilterXMetricsLabels *metrics_labels = filterx_metrics_labels_new(NULL); cr_assert(metrics_labels); + filterx_metrics_labels_optimize(metrics_labels); + cr_assert(filterx_metrics_labels_is_const(metrics_labels)); DynMetricsStore *store = dyn_metrics_cache(); @@ -64,6 +66,8 @@ Test(filterx_metrics_labels, const_literal_generator_empty_labels) filterx_expr_unref(labels_expr); cr_assert(metrics_labels); + filterx_metrics_labels_optimize(metrics_labels); + cr_assert(filterx_metrics_labels_is_const(metrics_labels)); DynMetricsStore *store = dyn_metrics_cache(); @@ -83,6 +87,8 @@ Test(filterx_metrics_labels, non_literal_empty_labels) filterx_expr_unref(labels_expr); cr_assert(metrics_labels); + filterx_metrics_labels_optimize(metrics_labels); + cr_assert_not(filterx_metrics_labels_is_const(metrics_labels)); DynMetricsStore *store = dyn_metrics_cache(); @@ -112,6 +118,8 @@ Test(filterx_metrics_labels, const_literal_generator_labels) filterx_expr_unref(labels_expr); cr_assert(metrics_labels); + filterx_metrics_labels_optimize(metrics_labels); + cr_assert(filterx_metrics_labels_is_const(metrics_labels)); DynMetricsStore *store = dyn_metrics_cache(); @@ -146,6 +154,8 @@ Test(filterx_metrics_labels, non_const_literal_generator_labels) filterx_expr_unref(labels_expr); cr_assert(metrics_labels); + filterx_metrics_labels_optimize(metrics_labels); + cr_assert_not(filterx_metrics_labels_is_const(metrics_labels)); DynMetricsStore *store = dyn_metrics_cache(); diff --git a/modules/metrics-probe/filterx/func-update-metric.c b/modules/metrics-probe/filterx/func-update-metric.c index bcdc5e0e8..be04ce914 100644 --- a/modules/metrics-probe/filterx/func-update-metric.c +++ b/modules/metrics-probe/filterx/func-update-metric.c @@ -94,6 +94,37 @@ _eval(FilterXExpr *s) return filterx_boolean_new(TRUE); } +static void +_optimize_increment(FilterXFunctionUpdateMetric *self) +{ + self->increment.expr = filterx_expr_optimize(self->increment.expr); + if (!self->increment.expr || !filterx_expr_is_literal(self->increment.expr)) + return; + + FilterXObject *increment_obj = filterx_expr_eval(self->increment.expr); + if (!increment_obj) + return; + + gboolean success = filterx_object_extract_integer(increment_obj, &self->increment.value); + filterx_object_unref(increment_obj); + if (!success) + return; + + filterx_expr_unref(self->increment.expr); + self->increment.expr = NULL; +} + +static FilterXExpr * +_optimize(FilterXExpr *s) +{ + FilterXFunctionUpdateMetric *self = (FilterXFunctionUpdateMetric *) s; + + _optimize_increment(self); + filterx_metrics_optimize(self->metrics); + + return filterx_function_optimize_method(&self->super); +} + static gboolean _init(FilterXExpr *s, GlobalConfig *cfg) { @@ -138,29 +169,6 @@ _extract_increment_arg(FilterXFunctionUpdateMetric *self, FilterXFunctionArgs *a self->increment.value = 1; self->increment.expr = filterx_function_args_get_named_expr(args, "increment"); - if (self->increment.expr && filterx_expr_is_literal(self->increment.expr)) - { - FilterXObject *increment_obj = filterx_expr_eval(self->increment.expr); - if (!increment_obj) - { - g_set_error(error, FILTERX_FUNCTION_ERROR, FILTERX_FUNCTION_ERROR_CTOR_FAIL, - "failed to evaluate increment. " FILTERX_FUNC_UPDATE_METRIC_USAGE); - return FALSE; - } - - gboolean success = filterx_object_extract_integer(increment_obj, &self->increment.value); - filterx_object_unref(increment_obj); - if (!success) - { - g_set_error(error, FILTERX_FUNCTION_ERROR, FILTERX_FUNCTION_ERROR_CTOR_FAIL, - "failed to set increment, increment must be integer. " FILTERX_FUNC_UPDATE_METRIC_USAGE); - return FALSE; - } - - filterx_expr_unref(self->increment.expr); - self->increment.expr = NULL; - } - return TRUE; } @@ -227,6 +235,7 @@ filterx_function_update_metric_new(FilterXFunctionArgs *args, GError **error) filterx_function_init_instance(&self->super, "update_metric"); self->super.super.eval = _eval; + self->super.super.optimize = _optimize; self->super.super.init = _init; self->super.super.deinit = _deinit; self->super.super.free_fn = _free;