Added metrics generator #46
Conversation
vnandwana
commented
Apr 11, 2022
vnandwana
commented
- Replaced javax.servlet with Jakarta servlet.
* Replaced javax.servlet with Jakarta servlet.
* Replaced javax.servlet with Jakarta servlet.
| @@ -114,8 +114,7 @@ | |||
| <dependency> | |||
| <groupId>org.jooq</groupId> | |||
| <artifactId>jooq</artifactId> | |||
| <!-- 3.14.0 broke returning() on MySQL :( --> | |||
There was a problem hiding this comment.
Could you confirm this is now fixed, by running the integration tests?
export STRIPE_API_KEY=...
export STRIPE_PUBLIC_KEY=...
mvn test -Pintegration-mysql
This has been a long standing bug in jooq that has prevented us from upgrading. It would be very good news if it's now addressed, many plugins could be upgraded.
| @@ -19,4 +19,7 @@ | |||
| <Match> | |||
| <Class name="~org\.killbill\.billing\.plugin\.stripe\.dao\.gen.*"/> | |||
| </Match> | |||
| <Bug pattern="EI_EXPOSE_REP"/> | |||
There was a problem hiding this comment.
Ignoring EI_EXPOSE_REP and EI_EXPOSE_REP2 is probably fine, however NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE seems a bit suspicious. Can you investigate what's triggering this?
| } | ||
|
|
||
| try { | ||
| metricRegistry.getMetricRegistry().counter("stripe_plugin_counter").inc(1); |
There was a problem hiding this comment.
This code snippet from the hello world plugin is really just an example. We probably don't want it in plugins that are used in production code 😉
You could add something more useful here for instance:
try {
return intent.capture(paymentIntentParams, requestOptions);
} finally {
metricRegistry.getMetricRegistry().counter("stripe_capture").inc(1);
}
Same for other Stripe client API calls.
| @@ -60,6 +62,10 @@ public void start(final BundleContext context) throws Exception { | |||
| final StripeHealthcheck stripeHealthcheck = new StripeHealthcheck(stripeConfigPropertiesConfigurationHandler); | |||
| registerHealthcheck(context, stripeHealthcheck); | |||
|
|
|||
| // Expose metrics (optional) | |||
