Histogram with empty family labels creates unparseable metrics output

[tylerlevine]

I'm loosely following the actix example to add prom metrics for my simple webapp which doesn't need additional family labels on its metrics for now. However, when I tried removing the family labels, I get unparseable output from the metrics endpoint:

$ curl localhost:8080/metrics
# HELP requests Count of requests.
# TYPE requests counter
requests_total{} 1
# HELP elapsed Histogram of elapsed.
# TYPE elapsed histogram
elapsed_sum{} 10.0
elapsed_count{} 1
elapsed_bucket{le="0.0",} 0
elapsed_bucket{le="+Inf",} 1
# EOF

The extra comma in the label list for each bucket causes this to be unparseable by prometheus, resulting in the following error:

expected label name, got "BCLOSE"

Is this intended behavior? Or is there a better way to create metrics without family labels? I boiled my issue down to a small app which I'll include below, along with a test case and patch that I've found fixes the issue for me locally. I'd be happy to submit a PR with the fix if desired.

Test app source:

use std::sync::Mutex;

use actix_web::{web, App, HttpResponse, HttpServer, Responder, Result};
use prometheus_client::encoding::text::encode;
use prometheus_client::metrics::counter::Counter;
use prometheus_client::metrics::family::Family;
use prometheus_client::metrics::histogram::{Histogram, linear_buckets};
use prometheus_client::registry::Registry;

pub struct Metrics {
    requests: Family<(), Counter>,
    elapsed: Family<(), Histogram>,
}

pub struct AppState {
    pub registry: Registry,
}

pub async fn metrics_handler(state: web::Data<Mutex<AppState>>) -> Result<HttpResponse> {
    let state = state.lock().unwrap();
    let mut body = String::new();
    encode(&mut body, &state.registry).unwrap();
    Ok(HttpResponse::Ok()
        .content_type("application/openmetrics-text; version=1.0.0; charset=utf-8")
        .body(body))
}

pub async fn some_handler(metrics: web::Data<Metrics>) -> impl Responder {
    metrics.requests.get_or_create(&()).inc();
    metrics.elapsed.get_or_create(&()).observe(10f64);
    "okay".to_string()
}

#[actix_web::main]
async fn main() -> std::io::Result<()> {
    let elapsed_hist: Family<(), Histogram> = Family::new_with_constructor(||
        Histogram::new(linear_buckets(0f64, 1f64, 1)));

    let metrics = web::Data::new(Metrics {
        requests: Family::default(),
        elapsed: elapsed_hist,
    });

    let mut state = AppState {
        registry: Registry::default(),
    };

    state
        .registry
        .register("requests", "Count of requests", metrics.requests.clone());
    state
        .registry
        .register("elapsed", "Histogram of elapsed", metrics.elapsed.clone());
    let state = web::Data::new(Mutex::new(state));

    HttpServer::new(move || {
        App::new()
            .app_data(metrics.clone())
            .app_data(state.clone())
            .service(web::resource("/metrics").route(web::get().to(metrics_handler)))
            .service(web::resource("/handler").route(web::get().to(some_handler)))
    })
        .bind(("127.0.0.1", 8080))?
        .run()
        .await
}

Output:

$ curl localhost:8080/handler; curl localhost:8080/metrics
okay# HELP requests Count of requests.
# TYPE requests counter
requests_total{} 1
# HELP elapsed Histogram of elapsed.
# TYPE elapsed histogram
elapsed_sum{} 10.0
elapsed_count{} 1
elapsed_bucket{le="0.0",} 0
elapsed_bucket{le="+Inf",} 1
# EOF

Fix patch with unit test: master...tylerlevine:client_rust:histogram-empty-family-labels-fix

The idea behind the fix is that the trailing comma behind the additional labels is only added if the encoding of the family labels is non-empty. I'm not very familiar with this library so maybe there's a better approach here.

With this change the metrics output appears as expected, with no extraneous commas in the bucket label lists:

$ curl localhost:8080/handler; curl localhost:8080/metrics
okay# HELP requests Count of requests.
# TYPE requests counter
requests_total{} 1
# HELP elapsed Histogram of elapsed.
# TYPE elapsed histogram
elapsed_sum{} 10.0
elapsed_count{} 1
elapsed_bucket{le="0.0"} 0
elapsed_bucket{le="+Inf"} 1
# EOF

[mxinden]

Thank you for digging into this. Let's discuss on #175.

[kevincox]

It seems that the fix was merged months ago but not released. Is it possible to cut a new release with that fix?