Comment From: wilkinsona

Based on this review comment, I think the main thing that we need to consider here is how to name each target DataSource. There are a few scenarios to consider:

  1. A target DataSource is a target of a single AbstractRoutingDataSource.
  2. A target DataSource is a target of multiple AbstractRoutingDataSources.
  3. A DataSource is both a top-level bean and a target of a single AbstractRoutingDataSource.
  4. A DataSource is both a top-level bean and a target of multiple AbstractRoutingDataSources.

When a target DataSource is also a bean (scenarios 3 and 4), I think its name tag should continue to be its bean name. This will ensure backwards compatibility with 2.4 and earlier. This would then require a different tag to display its routing information. When the DataSource is a target of a single routing data source, the value of this new tag could be a combination of the router's bean name and the routing key. When the DataSource is a target of multiple routing data sources, the value would, ideally, convey that information. We're limited to a String so we'd have to delimit each value. In bean's name can, in theory, be anything you want so it's not immediately clear what a good delimiter would be.

When a target DataSource is only a target (scenarios 1 and 2), we need a unique name for it. The routing key is only guaranteed to be unique in the scope of the routing data source. We need something that will be unique across the whole application so we can't use the routing key alone. The bean name of the routing data source will be unique across the whole application so we could combine the bean name of the routing data source with the routing key. A downside of this would be that the name tag's value will have a different format depending on whether the target is or is not also exposed as a bean.

The four scenarios described above could be mapped onto the following beans:

  • dsThree
  • dsFour
  • routerOne
  • key1 -> dsOne
  • key2 -> dsTwo
  • key3 -> dsThree
  • key4 -> dsFour
  • routerTwo
  • key1 -> dsTwo
  • key2 -> dsFour

dsThree would be tagged with name -> dsThree and routing -> routerOne:key3. dsFour would be tagged with name -> dsFour and routing -> routerOne:key4,routerTwo:key2 dsOne would be tagged with name -> routerOne:key1 and routing -> routerOne:key1 dsTwo would be tagged with name -> routerOne:key2,routerTwo:key1 and routing -> routerOne:key2,routerTwo:key1

Of these, I'm only really happy with dsThree. dsFour could be fine too if we can identify a good delimiter to use in the routing value. dsOne and dsTwo feel a bit odd with identical values for name and routing. And dsTwo feels really odd as the name gets longer.

Comment From: onobc

@wilkinsona thanks for thinking deeply about the naming and also the "same DS in multiple routing DS" case.

Of these, I'm only really happy with dsThree. dsFour could be fine too if we can identify a good delimiter to use in the routing value. dsOne and dsTwo feel a bit odd with identical values for name and routing. And dsTwo feels really odd as the name gets longer.

I agree w/ all of the above. And yeh, In the case where the DS is not also a registered bean, the routing info is the name, so the two tags would be identical. And yes, it gets wonky as it gets longer. It's not the worst thing though, and I am curious how often this case occurs.

As for a good delimiter, there are really no bean name restrictions nor tag value restrictions in Micrometer. I dug a bit in Micrometer to see if there was any direction there. We could choose something sensible and then make it configurable for the "one" case that the delimiter does not cover :)

Comment From: wilkinsona

We discussed this today and were in agreement that the tag values described above are pretty gross and not something that we'd want to implement. We discussed an alternative where users have to implement a strategy interface to provide the tags for a target data source. We were concerned that this strategy interface may have to be quite complex and provide quite a bit of context, for example:

  1. The bean name of the routing data source
  2. The routing data source
  3. The target data source
  4. The routing key of the target data source

We're not yet convinced that such a strategy interface would offer much over the user binding some meters themselves for each target data source. We're going to take a step back and revisit this for a future release. It may become solely a documentation issue or we may make some enhancements either in the form of some auto-configuration and a strategy interface for the tags, or just some helper API that makes it easier for user code to bind some meters directly.

Comment From: dharezlak

@wilkinsona Is there already an issue for the same thing for org.springframework.r2dbc.connection.lookup.AbstractRoutingConnectionFactory? As soon as I start to use AbstractRoutingConnectionFactory in my project I am loosing all r2dbc.pool.* metrics exposed by the /actuator/metrics endpoint.

Comment From: wilkinsona

I don't believe we have an issue for the R2DBC side of things. There's no accessor for the target connection factories so any issue would be blocked on a Spring Framework change. As things stand, I don't think we could justify asking for such a change to be made as R2DBC will have the same tagging challenges as those described above for JDBC.

If you're interested in metrics for your target connection factories (or target data sources with JDBC), I would recommend exposing them as beans and then having a @Primary routing connection factory or data source that uses them.

Comment From: mgook

Even now, when using AbstractRoutingDatasource, can't I get metrics if I don't register it as a bean?

Comment From: wilkinsona

If it isn’t a bean, Actuator won’t know it exists. In that case, you can register metrics manually using the Micrometer API. Alternatively, you could follow the suggestion above.

Comment From: mgook

AbstractRoutingDatasource is bean.

I mean that how to register all target datasources to metrics. :)

Comment From: sahilkamboj334

@wilkinsona i am trying to get pool info for abstract-routing-datasource but hikari data source pool is coming as null. when i do getConnection on datasource then it becomes available. Could u please update how i can log metrics around pool for abstract-routing-datasource. i want to expose them to prometheus manually using a background running thread.

Comment From: wilkinsona

@sahilkamboj334 Please ask your question on Stack Overflow and provide a complete yet minimal example of what you're trying to do.

Comment From: philwebb

We're cleaning out the issue tracker and closing issues that we've not seen much demand to fix. Feel free to comment with additional justifications if you feel that this one should not have been closed.

Comment From: mathieufortin01

Not sure if commenting on a closed ticket is fine but since

Feel free to comment with additional justifications if you feel that this one should not have been closed.

I think some work has been done in the past to auto-configure the health endpoint with resolved targets of AbstractRoutingDataSource, and it really feel like its missing for the metrics. You can lose your metrics when switching to writer-reader infra, or 1 to multiple shards, etc. And you need some digging to understand why this is happening.

In our case we ended up manually registering for all resolved datasources, but having auto-configuration handle it as it is doing for health would simplify it.

For Hikari at least, could it just be to use getResolvedDataSources() if the passed-in datasource is a routing one ? (ref: https://github.com/spring-projects/spring-boot/blob/8fa318453f2e3f128557350eb975fd68d356375f/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/metrics/jdbc/DataSourcePoolMetricsAutoConfiguration.java#L140)

Comment From: mathieufortin01

Ok saw the closed pr https://github.com/spring-projects/spring-boot/pull/25749/files/778bdccb44b9367a01e35540b011cf6f1e1873a8#diff-0f67c2fc3d035ff0490429235411976ec6fc3e3635b94a9d80adb58b82552a7f. Hoping for this to be revived.

Comment From: wilkinsona

@mathieufortin01 Unfortunately, without a solution to the tagging problem described above we don't have a way to revive this. As things stand, binding each target manually is the best option.

Comment From: mathieufortin01

@wilkinsona Thanks for prompt response. In that case, if the metrics auto-configuration meets a routing datasource, could it log the fact that it wont register metrics for its targets (and that it should be manually done)? I think it could help drive people toward the right solution, but not sure if those kind of logs are typical in the project.

Comment From: wilkinsona

Thanks for the suggestion. Unfortunately, we try to avoid log messages like that as they can add quite a bit of complexity. For example, if we started logging such a message when we encounter a routing DataSource, a user that had already bound metrics for all of the targets would quite reasonably not want it to be logged. We would then either have to try to detect that is the case or offer a configuration property to disable the log message.