Rogue label le="" showing up in one generic rule

[mtthwcmpbll]

I've been exploring Pyrra over the last few days and had some trouble getting the UI to work for me, so I've also been looking at the generic rules generation. This works well for us because we're already using Grafana for all of our observability stuff today.

I've deployed the following SLO:

apiVersion: pyrra.dev/v1alpha1
kind: ServiceLevelObjective
metadata:
  name: test-api-playground-latency
  namespace: monitoring
  labels:
    prometheus: k8s
    role: alert-rules
spec:
  target: '95'
  window: 4w
  description: Service responded to 95% of requests within half a second
  indicator:
    latency:
      success:
        metric: http_server_requests_seconds_bucket{application="test-api", namespace="playground", le="0.5"}
      total:
        metric: http_server_requests_seconds_bucket{application="test-api", namespace="playground", le="+Inf"}

The metric I'm using for this SLO is a Spring Boot histogram for request times. The generated PrometheusRule looks correct to me and almost all of the recording rules record data as expected.

However, the pyrra_availability generic metric recording rule gets generated with an extra le="" label, despite there already being an le label in the original query. Here's the generated availability recording rule:

- expr: sum(http_server_requests_seconds:increase4w{application="test-api",le="0.5",namespace="playground",slo="test-api-playground-latency"}
      or vector(0)) / sum(http_server_requests_seconds:increase4w{application="test-api",le="",le="+Inf",namespace="playground",slo="test-api-playground-latency"})
  labels:
    slo: test-api-playground-latency
  record: pyrra_availability

If I run the query in expression manually in Grafana I get no data because of that le="" in the denominator. If I remove that label and run the following query, it returns data fine.

sum(http_server_requests_seconds:increase4w{application="test-api",le="0.5",namespace="playground",slo="test-api-playground-latency"}
      or vector(0)) / sum(http_server_requests_seconds:increase4w{application="test-api",le="+Inf",namespace="playground",slo="test-api-playground-latency"})

I see there was some work related to native histograms that seems to have introduced the empty le filter, so maybe related.

[mtthwcmpbll]

This bit of code looks like it's responsible for building up the pyrra_availability generic rule for a latency SLO like mine, and I see where it's adding the le="" matcher. Unfortunately I don't understand why it's expecting there to be an empty le label. In the above PR for native histograms, there's a comment in that code that isn't in this one:

// Add the matcher {le=""} to select the recording rule that summed up all requests

I'm not sure I'm following the code enough to understand why it's expecting the recording rule to have an empty le label. Here are the generated increase recording rules for my SLO:

- expr: sum(increase(http_server_requests_seconds_bucket{application="test-api",le="+Inf",namespace="playground"}[4w]))
  labels:
    application: test-api
    le: +Inf
    namespace: playground
    slo: test-api-playground-latency
  record: http_server_requests_seconds:increase4w
- expr: sum(increase(http_server_requests_seconds_bucket{application="test-api",le="0.5",namespace="playground"}[4w]))
  labels:
    application: test-api
    le: +Inf
    namespace: playground
    slo: test-api-playground-latency
  record: http_server_requests_seconds:increase4w

[gdidok]

I ran into the same issue when using http_server_requests_seconds_bucket{le="+Inf"} for total metrics. While the root problem still exists in the code, switching to http_server_requests_seconds_count worked as a temporary workaround.

[metalmatze]

Yes, I think using the _count is generally how this works.
_bucket{le="+Inf"} and _count are the same counter.

I looked at the initial report a couple of days ago and didn't catch that part, so thanks for pointing out the le="+Inf" part.