Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vertx-dropwizard-metrics module isn't osgi ready #178

Closed
ANierbeck opened this issue Aug 21, 2016 · 8 comments
Closed

vertx-dropwizard-metrics module isn't osgi ready #178

ANierbeck opened this issue Aug 21, 2016 · 8 comments

Comments

@ANierbeck
Copy link

ANierbeck commented Aug 21, 2016

Opposed to the other jars of the vertx project this module isn't generated with dependency headers, therefore it's not possible to use vertx with metrics and jmx enabled.

@cescoffier
Copy link
Member

Hi @ANierbeck

The main reason why the OSGi metadata is not present in this bundle is because it's a bit more complex than just adding the metadata. Computing the metadata is simple, however using the resulting bundle is a bit more complex. This is because the MetricFactory is instantiated with a SPI (ServiceLoader), and unlike the cluster manager there is no way to inject an existing this factory in the VertxOptions.

Is it impossible to use ? No, but it would require some classloader magic to find the SPI file and load the MetricFactory using the same classloader as Vert.x.

Here is a Gist showing how you can do that:
https://gist.github.com/cescoffier/ae0f17a217691097849eb9615bb08d98

I think we can add the OSGi metadata and people wanting to use this metrics implementation can refer to the gist.

@cescoffier
Copy link
Member

cescoffier commented Aug 24, 2016

I've also tried (successfully) another approach using VertxMetricOptions.setFactory, here is the code:

https://gist.github.com/cescoffier/e8aec18cc86581923f1cc639c2a71c4d

It contains a VertxSpiHelper class that can load any SPI required by Vert.x.

@ANierbeck
Copy link
Author

Ahh the last one is definitely more appealing. Though have you considered to use Apache Aries SPI-Fly, that should do the trick you did already ... haven't had the time to try it myself yet.
http://aries.apache.org/modules/spi-fly.html

@cescoffier
Copy link
Member

cescoffier commented Aug 24, 2016

I never used SPI-Fly, but it is probably very similar (aren't they exposing services ?).

Ideally, Vert.x SPI (factories) should be exposed as services, and so the bundle creating the Vert.x instance can pick (Select, react to absence) the services it needs. It would be a static link with these services as it is not possible to change on the fly the metrics factory or cluster manager.

My second attempt has 2 purposes:

  1. simplify how dropwizard metrics can be used in OSGi
  2. show that vert.x core does not need any extension to be used in OSGi

@ANierbeck
Copy link
Author

I will give SPI Fly a try later (not sure when ;) ). It'll expose the SPI as service, therefore yes that might be a good way of re-using it. I'll keep you posted, but thanks for creating the metadata already that should help already in just "starting" the bundle :)

@ANierbeck
Copy link
Author

It works quite well with SPI-Fly:
It just requires two additional headers in the manifest:
Require-Capability: osgi.extender; filter:="(osgi.extender=osgi.serviceloader.registrar)"
Provide-Capability: osgi.serviceloader; osgi.serviceloader=io.vertx.core.spi.VertxMetricsFactory

maybe for the command too ... but I didn't need that one yet.
After that the service was easily used within the activation of vertx with the options you provide. Just referenced the VertxMetricsFactory as Service (sample will follow soon)

@ANierbeck
Copy link
Author

added a pull request for those settings:
vert-x3/vertx-dropwizard-metrics#45

@ANierbeck
Copy link
Author

Here's an example on how to use with SPI-Fly (or any other implementation of that osgi-service) in case of pull-request 45 is merged:

https://github.com/ANierbeck/Karaf-Vertx/blob/master/Vertx-System/src/main/java/de/nierbeck/example/vertx/VertxService.java#L47-L61

@pmlopes pmlopes mentioned this issue Aug 31, 2016
60 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants