[#33] Provide more health metrics #35
Conversation
plume-db/src/main/java/com/coreoz/plume/db/transaction/HikariDataSources.java
Show resolved
Hide resolved
|
Ok pour moi ! |
| @@ -16,12 +16,12 @@ | |||
| public class HikariDataSources { | |||
|
|
|||
| public static DataSource fromConfig(Config config, String prefix) { | |||
There was a problem hiding this comment.
Ce serait plus simple de retourner directement un HikariDataSource non ?
There was a problem hiding this comment.
En effet !
C'est corrigé.
plume-db/src/main/java/com/coreoz/plume/db/transaction/HikariDataSources.java
Show resolved
Hide resolved
| public MetricsCheckBuilder registerHikariMetrics(HikariDataSource hikariDataSource) { | ||
| hikariDataSource.setMetricRegistry(this.metricRegistry); | ||
| return this; | ||
| } |
There was a problem hiding this comment.
Il faudrait ajouter un test d'intégration avec ça pour vérifier qu'on récupère bien toutes les métriques et que c'est ok. Tu peux voir avec chatgpt pour t'aider à écrire le test :)
Wail-LT
force-pushed
the
more-metrics
branch
6 times, most recently
from
1ef1d28 to
3706925
Compare
| * Expose a {@link HikariDataSource} Object through dependency injection. | ||
| */ | ||
| @Singleton | ||
| public class HikariDataSourceProvider implements DataSourceProvider { |
There was a problem hiding this comment.
Ce serait pas plus simple de juste garder DataSourceProvider et mettre le code de cette classe dedans ?
There was a problem hiding this comment.
On ne veut pas laisser le choix d'avoir un autre Datasource que Hikari ?
There was a problem hiding this comment.
Si si on va le permettre, il suffira juste d'implémenter son propre Provider<DataSource>. Par contre en y repensant, peut-être qu'il faudrait tout de même garder le nom de HikariDataSourceProvider à la place de DataSourceProvider qui est moins clair sur le rôle du provider.
There was a problem hiding this comment.
Justement je ne suis pas sûr qu'en implémentant un Provider cela fonctionne.
public class CustomDataSourceProvider implements Provider<DataSource>;
bind(HikariDataSourceProvider.class).to(CustomDataSourceProvider.class);
Ça ne fonctionnera pas, HikariDataSources != Datasource.
| public HikariDataSourceProvider(Config config, String prefix) { | ||
| this.dataSource = HikariDataSources.fromConfig(config, prefix + ".hikari"); | ||
| } |
There was a problem hiding this comment.
Je pense qu'on peut supprimer ce constructeur, il n'a pas vraiment de sens en fait. Si quelqu'un veut créer une autre datasource, il appelera juste HikariDataSources.fromConfig(...)
}
| } | ||
| @Override | ||
| protected void configure() { | ||
| bind(DataSourceProvider.class).to(HikariDataSourceProvider.class); |
There was a problem hiding this comment.
Cette ligne ne sert pas vraiment. Par contre il manque la ligne: bind(HikariDataSource.class).toProvider(DataSourceProvider.class)
Wail-LT
force-pushed
the
more-metrics
branch
2 times, most recently
from
e5f273f to
03a7ae4
Compare
|
related to issue #33