diff --git a/src/cmt_decode_prometheus.c b/src/cmt_decode_prometheus.c index a6215a25..39dc413e 100644 --- a/src/cmt_decode_prometheus.c +++ b/src/cmt_decode_prometheus.c @@ -433,6 +433,7 @@ static int add_metric_histogram(struct cmt_decode_prometheus_context *context) int ret = 0; int i; int has_le = CMT_FALSE; + int has_count = CMT_FALSE; size_t bucket_count; size_t bucket_index; size_t label_count = 0; @@ -458,11 +459,23 @@ static int add_metric_histogram(struct cmt_decode_prometheus_context *context) "not enough samples for histogram"); } - /* bucket_count = sample count - 3: - * - "Inf" bucket - * - sum - * - count */ - bucket_count = cfl_list_size(&context->metric.samples) - 3; + /* bucket_count = number of bucket samples without the "+Inf" one. Count + * them instead of assuming that sum and count are present, _sum is + * optional */ + bucket_count = 0; + cfl_list_foreach(head, &context->metric.samples) { + sample = cfl_list_entry(head, struct cmt_decode_prometheus_context_sample, _head); + if (sample->type == CMT_DECODE_PROMETHEUS_CONTEXT_SAMPLE_TYPE_BUCKET) { + bucket_count++; + } + } + if (bucket_count < 2) { + return report_error(context, + CMT_DECODE_PROMETHEUS_SYNTAX_ERROR, + "not enough buckets for histogram"); + } + bucket_count--; + if (context->opts.override_timestamp) { timestamp = context->opts.override_timestamp; } @@ -530,23 +543,26 @@ static int add_metric_histogram(struct cmt_decode_prometheus_context *context) sample = cfl_list_entry(head, struct cmt_decode_prometheus_context_sample, _head); switch (sample->type) { case CMT_DECODE_PROMETHEUS_CONTEXT_SAMPLE_TYPE_BUCKET: - if (bucket_index == bucket_count) { - /* probably last bucket, which has "Inf" */ + if (bucket_index > bucket_count) { break; } - if (!sample->label_values[le_label_index] || - sample->label_values[le_label_index][0] == '\0') { - ret = report_error(context, - CMT_DECODE_PROMETHEUS_SYNTAX_ERROR, - "missing histogram bucket \"le\" value"); - goto end; - } - if (parse_double(sample->label_values[le_label_index], - buckets + bucket_index)) { - ret = report_error(context, - CMT_DECODE_PROMETHEUS_CMT_CREATE_ERROR, - "failed to parse bucket"); - goto end; + /* the last bucket is "+Inf": it has no boundary to parse and + * its value goes to the extra slot of bucket_defaults */ + if (bucket_index < bucket_count) { + if (!sample->label_values[le_label_index] || + sample->label_values[le_label_index][0] == '\0') { + ret = report_error(context, + CMT_DECODE_PROMETHEUS_SYNTAX_ERROR, + "missing histogram bucket \"le\" value"); + goto end; + } + if (parse_double(sample->label_values[le_label_index], + buckets + bucket_index)) { + ret = report_error(context, + CMT_DECODE_PROMETHEUS_CMT_CREATE_ERROR, + "failed to parse bucket"); + goto end; + } } if (parse_uint64(sample->value1, bucket_defaults + bucket_index)) { @@ -604,7 +620,7 @@ static int add_metric_histogram(struct cmt_decode_prometheus_context *context) count = (uint64_t)count_dbl; } } - bucket_defaults[bucket_index] = count; + has_count = CMT_TRUE; if (!timestamp) { ret = parse_timestamp(context, sample->value2, ×tamp); @@ -618,6 +634,13 @@ static int add_metric_histogram(struct cmt_decode_prometheus_context *context) } } + if (!has_count) { + ret = report_error(context, + CMT_DECODE_PROMETHEUS_SYNTAX_ERROR, + "missing histogram _count"); + goto end; + } + if (!timestamp) { /* No timestamp was specified, use default value */ timestamp = context->opts.default_timestamp; @@ -1043,9 +1066,11 @@ static int parse_histogram_summary_name( } if (name_matched) { - if (sum_found && count_found) { + if ((sum_found && count_found) || is_previous_sum_or_count) { /* finish instance of the summary/histogram, the samples of the - * next instance using the base name are quantiles */ + * next instance using the base name are quantiles. _sum is + * optional (e.g. summaries of Dropwizard timers), so a quantile + * following a _sum or _count also starts a new instance */ return finish_duplicate_histogram_summary_sum_count( context, metric_name, @@ -1060,8 +1085,9 @@ static int parse_histogram_summary_name( /* invalid histogram/summary suffix, treat it as a different metric */ if (!strcmp(metric_name + parsed_name_len, "_bucket")) { - if (sum_found && count_found && has_buckets && is_previous_sum_or_count) { - /* already found both sum and count, so this is a new metric */ + if (has_buckets && is_previous_sum_or_count) { + /* a bucket following the _sum or _count of an instance with + * buckets starts a new instance, _sum being optional */ return finish_duplicate_histogram_summary_sum_count( context, metric_name, diff --git a/tests/prometheus_parser.c b/tests/prometheus_parser.c index bb3881dd..0548dddb 100644 --- a/tests/prometheus_parser.c +++ b/tests/prometheus_parser.c @@ -1661,7 +1661,7 @@ void test_issue_fluent_bit_6534() "dotnet_threadpool_queue_length_bucket{le=\"10.0\"} 321733 0\n" "dotnet_threadpool_queue_length_bucket{le=\"100.0\"} 321733 0\n" "dotnet_threadpool_queue_length_bucket{le=\"1000.0\"} 321733 0\n" - "dotnet_threadpool_queue_length_bucket{le=\"+Inf\"} 0 0\n" + "dotnet_threadpool_queue_length_bucket{le=\"+Inf\"} 321733 0\n" "dotnet_threadpool_queue_length_sum 5 0\n" "dotnet_threadpool_queue_length_count 321733 0\n" "# HELP dotnet_gc_pause_seconds The amount of time execution was paused for garbage collection\n" @@ -1673,7 +1673,7 @@ void test_issue_fluent_bit_6534() "dotnet_gc_pause_seconds_bucket{le=\"0.5\"} 759 0\n" "dotnet_gc_pause_seconds_bucket{le=\"1.0\"} 759 0\n" "dotnet_gc_pause_seconds_bucket{le=\"10.0\"} 759 0\n" - "dotnet_gc_pause_seconds_bucket{le=\"+Inf\"} 0 0\n" + "dotnet_gc_pause_seconds_bucket{le=\"+Inf\"} 759 0\n" "dotnet_gc_pause_seconds_sum 1.3192573999999997 0\n" "dotnet_gc_pause_seconds_count 759 0\n" "# HELP dotnet_gc_collection_seconds The amount of time spent running garbage collections\n" @@ -1685,7 +1685,7 @@ void test_issue_fluent_bit_6534() "dotnet_gc_collection_seconds_bucket{le=\"0.5\",gc_generation=\"1\",gc_type=\"non_concurrent_gc\"} 133 0\n" "dotnet_gc_collection_seconds_bucket{le=\"1.0\",gc_generation=\"1\",gc_type=\"non_concurrent_gc\"} 133 0\n" "dotnet_gc_collection_seconds_bucket{le=\"10.0\",gc_generation=\"1\",gc_type=\"non_concurrent_gc\"} 133 0\n" - "dotnet_gc_collection_seconds_bucket{le=\"+Inf\",gc_generation=\"1\",gc_type=\"non_concurrent_gc\"} 0 0\n" + "dotnet_gc_collection_seconds_bucket{le=\"+Inf\",gc_generation=\"1\",gc_type=\"non_concurrent_gc\"} 133 0\n" "dotnet_gc_collection_seconds_sum{gc_generation=\"1\",gc_type=\"non_concurrent_gc\"} 0.20421500000000006 0\n" "dotnet_gc_collection_seconds_count{gc_generation=\"1\",gc_type=\"non_concurrent_gc\"} 133 0\n" "# HELP dotnet_gc_collection_seconds The amount of time spent running garbage collections\n" @@ -1709,7 +1709,7 @@ void test_issue_fluent_bit_6534() "dotnet_gc_collection_seconds_bucket{le=\"0.5\",gc_generation=\"2\",gc_type=\"non_concurrent_gc\"} 8 0\n" "dotnet_gc_collection_seconds_bucket{le=\"1.0\",gc_generation=\"2\",gc_type=\"non_concurrent_gc\"} 8 0\n" "dotnet_gc_collection_seconds_bucket{le=\"10.0\",gc_generation=\"2\",gc_type=\"non_concurrent_gc\"} 8 0\n" - "dotnet_gc_collection_seconds_bucket{le=\"+Inf\",gc_generation=\"2\",gc_type=\"non_concurrent_gc\"} 0 0\n" + "dotnet_gc_collection_seconds_bucket{le=\"+Inf\",gc_generation=\"2\",gc_type=\"non_concurrent_gc\"} 8 0\n" "dotnet_gc_collection_seconds_sum{gc_generation=\"2\",gc_type=\"non_concurrent_gc\"} 0.093447800000000011 0\n" "dotnet_gc_collection_seconds_count{gc_generation=\"2\",gc_type=\"non_concurrent_gc\"} 8 0\n" "dotnet_gc_collection_seconds_bucket{le=\"0.001\",gc_generation=\"0\",gc_type=\"non_concurrent_gc\"} 127 0\n" @@ -1719,7 +1719,7 @@ void test_issue_fluent_bit_6534() "dotnet_gc_collection_seconds_bucket{le=\"0.5\",gc_generation=\"0\",gc_type=\"non_concurrent_gc\"} 618 0\n" "dotnet_gc_collection_seconds_bucket{le=\"1.0\",gc_generation=\"0\",gc_type=\"non_concurrent_gc\"} 618 0\n" "dotnet_gc_collection_seconds_bucket{le=\"10.0\",gc_generation=\"0\",gc_type=\"non_concurrent_gc\"} 618 0\n" - "dotnet_gc_collection_seconds_bucket{le=\"+Inf\",gc_generation=\"0\",gc_type=\"non_concurrent_gc\"} 0 0\n" + "dotnet_gc_collection_seconds_bucket{le=\"+Inf\",gc_generation=\"0\",gc_type=\"non_concurrent_gc\"} 618 0\n" "dotnet_gc_collection_seconds_sum{gc_generation=\"0\",gc_type=\"non_concurrent_gc\"} 0.85545190000000104 0\n" "dotnet_gc_collection_seconds_count{gc_generation=\"0\",gc_type=\"non_concurrent_gc\"} 618 0\n" ; @@ -2000,6 +2000,169 @@ void test_duplicate_label_and_help() } } +/* + * Summaries exported from Dropwizard timers, e.g. by the Jenkins Prometheus + * plugin, have quantiles and a _count but no _sum. When a family has more than + * one instance, the quantiles of the next instance must start a new instance + * instead of being added to the previous one. + */ +void test_summary_instances_without_sum() +{ + int status; + char errbuf[256]; + struct cmt *cmt; + struct cmt_decode_prometheus_parse_opts opts; + cfl_sds_t result; + + memset(&opts, 0, sizeof(opts)); + errbuf[0] = '\0'; + opts.errbuf = errbuf; + opts.errbuf_size = sizeof(errbuf); + + cmt = NULL; + status = cmt_decode_prometheus_create(&cmt, + "# HELP builds Generated from Dropwizard metric import\n" + "# TYPE builds summary\n" + "builds{node=\"a\",quantile=\"0.5\",} 11\n" + "builds{node=\"a\",quantile=\"0.999\",} 12\n" + "builds_count{node=\"a\",} 1\n" + "builds{node=\"b\",quantile=\"0.5\",} 21\n" + "builds{node=\"b\",quantile=\"0.999\",} 22\n" + "builds_count{node=\"b\",} 2\n" + "builds{node=\"c\",quantile=\"0.5\",} 31\n" + "builds{node=\"c\",quantile=\"0.999\",} 32\n" + "builds_count{node=\"c\",} 3\n" + "# HELP other a gauge after the summary\n" + "# TYPE other gauge\n" + "other 5\n", 0, &opts); + TEST_CHECK(status == 0); + TEST_MSG("%s", errbuf); + if (status == 0) { + result = cmt_encode_prometheus_create(cmt, CMT_FALSE); + TEST_CHECK(strstr(result, + "builds{quantile=\"0.5\",node=\"a\"} 11\n") != NULL); + TEST_CHECK(strstr(result, + "builds{quantile=\"0.999\",node=\"a\"} 12\n") != NULL); + TEST_CHECK(strstr(result, "builds_count{node=\"a\"} 1\n") != NULL); + TEST_CHECK(strstr(result, + "builds{quantile=\"0.5\",node=\"b\"} 21\n") != NULL); + TEST_CHECK(strstr(result, + "builds{quantile=\"0.999\",node=\"b\"} 22\n") != NULL); + TEST_CHECK(strstr(result, "builds_count{node=\"b\"} 2\n") != NULL); + TEST_CHECK(strstr(result, + "builds{quantile=\"0.999\",node=\"c\"} 32\n") != NULL); + TEST_CHECK(strstr(result, "builds_count{node=\"c\"} 3\n") != NULL); + TEST_CHECK(strstr(result, "other 5\n") != NULL); + TEST_MSG("%s", result); + cfl_sds_destroy(result); + cmt_decode_prometheus_destroy(cmt); + } +} + +/* same as above for histograms, where OpenMetrics also makes _sum optional */ +void test_histogram_instances_without_sum() +{ + int status; + char errbuf[256]; + struct cmt *cmt; + struct cmt_decode_prometheus_parse_opts opts; + cfl_sds_t result; + + memset(&opts, 0, sizeof(opts)); + errbuf[0] = '\0'; + opts.errbuf = errbuf; + opts.errbuf_size = sizeof(errbuf); + + cmt = NULL; + status = cmt_decode_prometheus_create(&cmt, + "# TYPE h histogram\n" + "h_bucket{code=\"200\",le=\"1\"} 1\n" + "h_bucket{code=\"200\",le=\"+Inf\"} 2\n" + "h_count{code=\"200\"} 2\n" + "h_bucket{code=\"500\",le=\"1\"} 3\n" + "h_bucket{code=\"500\",le=\"+Inf\"} 4\n" + "h_count{code=\"500\"} 4\n", 0, &opts); + TEST_CHECK(status == 0); + TEST_MSG("%s", errbuf); + if (status == 0) { + result = cmt_encode_prometheus_create(cmt, CMT_FALSE); + TEST_CHECK(strstr(result, + "h_bucket{le=\"1.0\",code=\"200\"} 1\n") != NULL); + TEST_CHECK(strstr(result, "h_count{code=\"200\"} 2\n") != NULL); + TEST_CHECK(strstr(result, + "h_bucket{le=\"1.0\",code=\"500\"} 3\n") != NULL); + TEST_CHECK(strstr(result, + "h_bucket{le=\"+Inf\",code=\"500\"} 4\n") != NULL); + TEST_CHECK(strstr(result, "h_count{code=\"500\"} 4\n") != NULL); + TEST_MSG("%s", result); + cfl_sds_destroy(result); + cmt_decode_prometheus_destroy(cmt); + } + + /* _count is still required, decoding it as zero would change the total */ + cmt = NULL; + status = cmt_decode_prometheus_create(&cmt, + "# TYPE h histogram\n" + "h_bucket{le=\"1\"} 1\n" + "h_bucket{le=\"+Inf\"} 2\n" + "h_sum 5\n", 0, &opts); + TEST_CHECK(status == CMT_DECODE_PROMETHEUS_SYNTAX_ERROR); + TEST_MSG("%s", errbuf); +} + +/* _count before the buckets must not overwrite or lose the "+Inf" bucket */ +void test_histogram_count_before_buckets() +{ + int status; + char errbuf[256]; + struct cmt *cmt; + struct cmt_decode_prometheus_parse_opts opts; + cfl_sds_t result; + + memset(&opts, 0, sizeof(opts)); + errbuf[0] = '\0'; + opts.errbuf = errbuf; + opts.errbuf_size = sizeof(errbuf); + + cmt = NULL; + status = cmt_decode_prometheus_create(&cmt, + "# TYPE a histogram\n" + "a_sum 5\n" + "a_count 3\n" + "a_bucket{le=\"1\"} 1\n" + "a_bucket{le=\"2\"} 2\n" + "a_bucket{le=\"+Inf\"} 3\n" + "# TYPE b histogram\n" + "b_count{code=\"200\"} 2\n" + "b_bucket{code=\"200\",le=\"1\"} 1\n" + "b_bucket{code=\"200\",le=\"+Inf\"} 2\n" + "b_count{code=\"500\"} 4\n" + "b_bucket{code=\"500\",le=\"1\"} 3\n" + "b_bucket{code=\"500\",le=\"+Inf\"} 4\n", 0, &opts); + TEST_CHECK(status == 0); + TEST_MSG("%s", errbuf); + if (status == 0) { + result = cmt_encode_prometheus_create(cmt, CMT_FALSE); + TEST_CHECK(strstr(result, "a_bucket{le=\"1.0\"} 1\n") != NULL); + TEST_CHECK(strstr(result, "a_bucket{le=\"2.0\"} 2\n") != NULL); + TEST_CHECK(strstr(result, "a_bucket{le=\"+Inf\"} 3\n") != NULL); + TEST_CHECK(strstr(result, "a_count 3\n") != NULL); + TEST_CHECK(strstr(result, + "b_bucket{le=\"1.0\",code=\"200\"} 1\n") != NULL); + TEST_CHECK(strstr(result, + "b_bucket{le=\"+Inf\",code=\"200\"} 2\n") != NULL); + TEST_CHECK(strstr(result, "b_count{code=\"200\"} 2\n") != NULL); + TEST_CHECK(strstr(result, + "b_bucket{le=\"1.0\",code=\"500\"} 3\n") != NULL); + TEST_CHECK(strstr(result, + "b_bucket{le=\"+Inf\",code=\"500\"} 4\n") != NULL); + TEST_CHECK(strstr(result, "b_count{code=\"500\"} 4\n") != NULL); + TEST_MSG("%s", result); + cfl_sds_destroy(result); + cmt_decode_prometheus_destroy(cmt); + } +} + TEST_LIST = { {"header_help", test_header_help}, {"header_type", test_header_type}, @@ -2043,5 +2206,8 @@ TEST_LIST = { {"lexer_unmatched_input", test_lexer_unmatched_input}, {"histogram_summary_shorter_instance", test_histogram_summary_shorter_instance}, {"duplicate_label_and_help", test_duplicate_label_and_help}, + {"summary_instances_without_sum", test_summary_instances_without_sum}, + {"histogram_instances_without_sum", test_histogram_instances_without_sum}, + {"histogram_count_before_buckets", test_histogram_count_before_buckets}, { 0 } };