http_client_requests_seconds Prometheus metrics missing with retry enabled

[gianvitom]

Looks like when retry is enabled for the Prometheus does not return the http_client_requests_seconds metric for the configured client

Description

The stack we are using is SpringBoot 2.6.8, Riptide 3.2.2 and Kotlin/target Java 17 (eclipse-temurin), GET operation on the client.
With this configuration:

factoring-service:
      base-url: https://xxx.zalan.do/
      tracing:
        tags:
          peer.service: factoring-service
      retry:
        enabled: true
        fixed-delay: 10 milliseconds
        max-retries: 1

The prometheus actuator doesn't return the http_client_requests_seconds metric (for other clients without retry everything is fine).
While disabling it, so with just:

factoring-service:
      base-url: https://xxx.zalan.do/
      tracing:
        tags:
          peer.service: factoring-service

Everything is fine.

Expected Behavior

http_client_requests_seconds is returned in any case, when the client operation is called.

Actual Behavior

http_client_requests_seconds is missing for the client configured with retry enabled, while it is returned for all the other clients

Steps to Reproduce

• Configure a project with the previous stack (defined at the beginning of the description)
• Configure 2 clients, one with retry and another without
• Verify, after calling the client operations, that only for one of them the metric http_client_requests_seconds is available.

Context

Upgrade SpringBoot/Riptide/JDK in the project

[gianvitom]

From an additional investigation, looks like with retry enabled there is an additional tag called retry_number that doesn't exist in the metrics for the clients without retry.

So, if you have multiple Riptide clients defined, some with retry.enabled=true and others without, depending on which one pushes the metric first, the ones that pushes after will not be visible.

[whiskeysierra]

That's a Prometheus issue, isn't it? Might be worth working around this by setting the retry tag to false, as a default. Can you try that using static tags?

…

On Thu, 23 Jun 2022, 08:13 gianvitom, ***@***.***> wrote: From an additional investigation, looks like with retry enabled there is an additional tag called retry_number that doesn't exist in the metrics for the clients without retry. So, if you have multiple Riptide clients defined, some with retry.enabled=true and others without, depending on which one pushes the metric first, the ones that pushes after will not be visible. — Reply to this email directly, view it on GitHub <#1294 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADI7HNQAYIE5LZOKANR22TVQP6CNANCNFSM5ZO7XJSA> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[gianvitom]

Do you mean by setting:

• metric tag retry_number = 0
• metric tag retry = false
  if the metrics do not contain those already?

In a MeterFilter, like this?

@Configuration
class RiptideMetricsConfiguration {
    @Bean
    fun meterFilter(): MeterFilter {
        return object : MeterFilter {
            override fun map(id: Meter.Id): Meter.Id {
                return if (id.name.startsWith("http.client") && id.tags.none { it.key.equals("retry_number") })
                    id.withTags(Tags.concat(Tags.of("retry_number", "0", "retry", "false"), id.tagsAsIterable))
                else
                    id
            }
        }
    }
}

[whiskeysierra]

That might work. Alternatively, you can use the application.yaml:

riptide:
  defaults:
    metrics:
      tags:
        retry: "false"
        retry_number: "0"
  clients:
    factoring-service:
      base-url: https://xxx.zalan.do/
      tracing:
        tags:
          peer.service: factoring-service
      retry:
        enabled: true
        fixed-delay: 10 milliseconds
        max-retries: 1

[gianvitom]

Nice, would that override only the ones without or also the one that already have it for example retry_number=1 and retry=true?

[whiskeysierra]

I believe it should.

[gianvitom]

Ok, you mean the ones with already the tags will not re-default to false/0?

[whiskeysierra]

Sorry, it should default it for every client, but the ones with retry enabled should then override it with proper values.

[gianvitom]

Ok, thank you. I will try to use your suggestion 👍

[whiskeysierra]

For reference, this is the issue for micrometer: micrometer-metrics/micrometer#877