Skip to content

cat: fix skipped +Inf in Histograms #238

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
edsiper merged 4 commits into from
Dec 23, 2025
Merged

cat: fix skipped +Inf in Histograms #238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
caad5c1
cat: fix histogram +Inf prometheus text formatting
edsiper Jul 24, 2025
518a0c5
histogram: ensure math is ok for allocation for +Inf
edsiper Jul 24, 2025
5050dd7
cat: fix general logic for histograms concatenation
edsiper Dec 23, 2025
eec55a8
tests: cat: extend histograms concatenation tests
edsiper Dec 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 31 additions & 9 deletions src/cmt_cat.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,19 +96,38 @@ static int copy_label_values(struct cmt_metric *metric, char **out)
return i;
}

static inline int cat_histogram_values(struct cmt_metric *metric_dst, struct cmt_histogram *histogram,
struct cmt_metric *metric_src)
static inline int cat_histogram_values(struct cmt_metric *metric_dst, struct cmt_histogram *histogram_src,
struct cmt_metric *metric_src, struct cmt_histogram *histogram_dst)
{
int i;
size_t bucket_count_src;
size_t bucket_count_dst;

/* Validate source histogram buckets exist */
if (!metric_src->hist_buckets) {
/* Source has no bucket data, nothing to concatenate */
return 0;
}

bucket_count_src = histogram_src->buckets->count;
bucket_count_dst = histogram_dst->buckets->count;

/* Validate that source and destination have matching bucket structures */
if (bucket_count_src != bucket_count_dst) {
/* Histogram bucket structures don't match - cannot concatenate */
return -1;
}

/* Allocate destination buckets if needed */
if (!metric_dst->hist_buckets) {
metric_dst->hist_buckets = calloc(1, sizeof(uint64_t) * (histogram->buckets->count + 1));
metric_dst->hist_buckets = calloc(1, sizeof(uint64_t) * (bucket_count_dst + 1));
if (!metric_dst->hist_buckets) {
return -1;
}
}

for (i = 0; i < histogram->buckets->count; i++) {
/* Concatenate bucket values including +Inf bucket at index bucket_count_dst */
for (i = 0; i <= bucket_count_dst; i++) {
/* histogram buckets are always integers, no need to convert them */
metric_dst->hist_buckets[i] += metric_src->hist_buckets[i];
}
Expand Down Expand Up @@ -165,7 +184,8 @@ int cmt_cat_copy_map(struct cmt_opts *opts, struct cmt_map *dst, struct cmt_map
struct cmt_metric *metric_dst;
struct cmt_metric *metric_src;
struct cmt_summary *summary;
struct cmt_histogram *histogram;
struct cmt_histogram *histogram_src;
struct cmt_histogram *histogram_dst;

/* Handle static metric (no labels case) */
if (src->metric_static_set) {
Expand All @@ -176,8 +196,9 @@ int cmt_cat_copy_map(struct cmt_opts *opts, struct cmt_map *dst, struct cmt_map
metric_src = &src->metric;

if (src->type == CMT_HISTOGRAM) {
histogram = (struct cmt_histogram *) src->parent;
ret = cat_histogram_values(metric_dst, histogram, metric_src);
histogram_src = (struct cmt_histogram *) src->parent;
histogram_dst = (struct cmt_histogram *) dst->parent;
ret = cat_histogram_values(metric_dst, histogram_src, metric_src, histogram_dst);
if (ret == -1) {
return -1;
}
Expand Down Expand Up @@ -214,8 +235,9 @@ int cmt_cat_copy_map(struct cmt_opts *opts, struct cmt_map *dst, struct cmt_map
}

if (src->type == CMT_HISTOGRAM) {
histogram = (struct cmt_histogram *) src->parent;
ret = cat_histogram_values(metric_dst, histogram, metric_src);
histogram_src = (struct cmt_histogram *) src->parent;
histogram_dst = (struct cmt_histogram *) dst->parent;
ret = cat_histogram_values(metric_dst, histogram_src, metric_src, histogram_dst);
if (ret == -1) {
return -1;
}
Expand Down
2 changes: 1 addition & 1 deletion src/cmt_histogram.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ struct cmt_histogram_buckets *cmt_histogram_buckets_create_size(double *bkts, si
}

/* besides buckets set by the user, we add an implicit bucket for +inf */
upper_bounds = calloc(1, sizeof(double) * count + 1);
upper_bounds = calloc(1, sizeof(double) * (count + 1));
if (!upper_bounds) {
cmt_errno();
return NULL;
Expand Down
224 changes: 224 additions & 0 deletions tests/cat.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,8 +391,232 @@ void test_duplicate_metrics()

}

void test_histogram_empty_concatenation()
{
int ret;
struct cmt *cmt1;
struct cmt *cmt2;
struct cmt_histogram *h;
struct cmt_histogram_buckets *buckets;

/* Test concatenating an empty histogram (no observations, NULL hist_buckets) */
cmt1 = cmt_create();
TEST_CHECK(cmt1 != NULL);

buckets = cmt_histogram_buckets_create(11,
0.005, 0.01, 0.025, 0.05,
0.1, 0.25, 0.5, 1.0, 2.5,
5.0, 10.0);
TEST_CHECK(buckets != NULL);

/* Create histogram but never observe - hist_buckets will be NULL */
h = cmt_histogram_create(cmt1,
"test", "histogram", "empty", "Empty histogram test",
buckets,
0, NULL);
TEST_CHECK(h != NULL);

/* Create destination context */
cmt2 = cmt_create();
TEST_CHECK(cmt2 != NULL);

/* Concatenate empty histogram - should handle NULL hist_buckets gracefully */
ret = cmt_cat(cmt2, cmt1);
TEST_CHECK(ret == 0);

cmt_destroy(cmt1);
cmt_destroy(cmt2);
}

void test_histogram_mismatched_buckets()
{
int ret;
int i;
double val;
uint64_t ts;
struct cmt *cmt1;
struct cmt *cmt2;
struct cmt_histogram *h1;
struct cmt_histogram *h2;
struct cmt_histogram_buckets *buckets1;
struct cmt_histogram_buckets *buckets2;

/* Test concatenating histograms with different bucket structures */
cmt1 = cmt_create();
TEST_CHECK(cmt1 != NULL);

/* Create histogram with 11 buckets */
buckets1 = cmt_histogram_buckets_create(11,
0.005, 0.01, 0.025, 0.05,
0.1, 0.25, 0.5, 1.0, 2.5,
5.0, 10.0);
TEST_CHECK(buckets1 != NULL);

h1 = cmt_histogram_create(cmt1,
"test", "histogram", "mismatch", "Mismatched buckets test",
buckets1,
0, NULL);
TEST_CHECK(h1 != NULL);

ts = cfl_time_now();
for (i = 0; i < sizeof(hist_observe_values)/(sizeof(double)); i++) {
val = hist_observe_values[i];
cmt_histogram_observe(h1, ts, val, 0, NULL);
}

/* Create second context with different bucket structure */
cmt2 = cmt_create();
TEST_CHECK(cmt2 != NULL);

/* Create histogram with 5 buckets (different structure) */
buckets2 = cmt_histogram_buckets_create(5,
0.1, 0.5, 1.0, 5.0, 10.0);
TEST_CHECK(buckets2 != NULL);

h2 = cmt_histogram_create(cmt2,
"test", "histogram", "mismatch", "Mismatched buckets test",
buckets2,
0, NULL);
TEST_CHECK(h2 != NULL);

ts = cfl_time_now();
for (i = 0; i < sizeof(hist_observe_values)/(sizeof(double)); i++) {
val = hist_observe_values[i];
cmt_histogram_observe(h2, ts, val, 0, NULL);
}

/* Try to concatenate - should fail due to bucket mismatch */
ret = cmt_cat(cmt1, cmt2);
TEST_CHECK(ret == -1);

cmt_destroy(cmt1);
cmt_destroy(cmt2);
}

void test_histogram_empty_to_populated()
{
int ret;
int i;
double val;
uint64_t ts;
struct cmt *cmt1;
struct cmt *cmt2;
struct cmt_histogram *h;
struct cmt_histogram_buckets *buckets1;
struct cmt_histogram_buckets *buckets2;

/* Test concatenating empty histogram to one with data */
cmt1 = cmt_create();
TEST_CHECK(cmt1 != NULL);

buckets1 = cmt_histogram_buckets_create(11,
0.005, 0.01, 0.025, 0.05,
0.1, 0.25, 0.5, 1.0, 2.5,
5.0, 10.0);
TEST_CHECK(buckets1 != NULL);

/* Create empty histogram (no observations) */
h = cmt_histogram_create(cmt1,
"test", "histogram", "empty_to_full", "Empty to populated test",
buckets1,
0, NULL);
TEST_CHECK(h != NULL);

/* Create second context with populated histogram */
cmt2 = cmt_create();
TEST_CHECK(cmt2 != NULL);

buckets2 = cmt_histogram_buckets_create(11,
0.005, 0.01, 0.025, 0.05,
0.1, 0.25, 0.5, 1.0, 2.5,
5.0, 10.0);
TEST_CHECK(buckets2 != NULL);

h = cmt_histogram_create(cmt2,
"test", "histogram", "empty_to_full", "Empty to populated test",
buckets2,
0, NULL);
TEST_CHECK(h != NULL);

ts = cfl_time_now();
for (i = 0; i < sizeof(hist_observe_values)/(sizeof(double)); i++) {
val = hist_observe_values[i];
cmt_histogram_observe(h, ts, val, 0, NULL);
}

/* Concatenate empty to populated - should succeed */
ret = cmt_cat(cmt1, cmt2);
TEST_CHECK(ret == 0);

cmt_destroy(cmt1);
cmt_destroy(cmt2);
}

void test_histogram_populated_to_empty()
{
int ret;
int i;
double val;
uint64_t ts;
struct cmt *cmt1;
struct cmt *cmt2;
struct cmt_histogram *h;
struct cmt_histogram_buckets *buckets1;
struct cmt_histogram_buckets *buckets2;

/* Test concatenating populated histogram to empty one */
cmt1 = cmt_create();
TEST_CHECK(cmt1 != NULL);

buckets1 = cmt_histogram_buckets_create(11,
0.005, 0.01, 0.025, 0.05,
0.1, 0.25, 0.5, 1.0, 2.5,
5.0, 10.0);
TEST_CHECK(buckets1 != NULL);

h = cmt_histogram_create(cmt1,
"test", "histogram", "full_to_empty", "Populated to empty test",
buckets1,
0, NULL);
TEST_CHECK(h != NULL);

ts = cfl_time_now();
for (i = 0; i < sizeof(hist_observe_values)/(sizeof(double)); i++) {
val = hist_observe_values[i];
cmt_histogram_observe(h, ts, val, 0, NULL);
}

/* Create second context with empty histogram */
cmt2 = cmt_create();
TEST_CHECK(cmt2 != NULL);

buckets2 = cmt_histogram_buckets_create(11,
0.005, 0.01, 0.025, 0.05,
0.1, 0.25, 0.5, 1.0, 2.5,
5.0, 10.0);
TEST_CHECK(buckets2 != NULL);

/* Create empty histogram (no observations) */
h = cmt_histogram_create(cmt2,
"test", "histogram", "full_to_empty", "Populated to empty test",
buckets2,
0, NULL);
TEST_CHECK(h != NULL);

/* Concatenate populated to empty - should succeed */
ret = cmt_cat(cmt1, cmt2);
TEST_CHECK(ret == 0);

cmt_destroy(cmt1);
cmt_destroy(cmt2);
}

TEST_LIST = {
{"cat", test_cat},
{"duplicate_metrics", test_duplicate_metrics},
{"histogram_empty_concatenation", test_histogram_empty_concatenation},
{"histogram_mismatched_buckets", test_histogram_mismatched_buckets},
{"histogram_empty_to_populated", test_histogram_empty_to_populated},
{"histogram_populated_to_empty", test_histogram_populated_to_empty},
{ 0 }
};
Loading