[GitHub] [jena] afs commented on issue #534: [WIP] Proof of concept for prometheus endpoint

[GitBox <gi...@apache.org>]

afs commented on issue #534: [WIP] Proof of concept for prometheus endpoint
URL: https://github.com/apache/jena/pull/534#issuecomment-479922355
 
 
   #### Licensing
   
   @claudew - would you be able to help out with the L&N issues here?
   
   The LICENSE and NOTICE for the combined binary artifacts (3 of them) wil need to include BSD-related text.
   At the moment, they get autogenerated but we need to put specific text in.
   
   jena-fuseki-server
   jena-fuseki-war
   jena-fuseki-fulljar
   
   See jena-fuseki-webapp/src/resources/META-INF/ which puts in LICENSE and NOTICE into a jar but I think we need to do it in the shading step for external resources like BSD-licnesed binaries. 
   
   #### Packaging
   
   It is easier to add maven modules later than retire them.
   Prometheus is sufficiently common I think including in Fuseki core for now is easier for users. 
   
   Proposal: moving it to
   jena-fuseki-core::org.apache.jena.fuseki.metrics.prometheus (and the ServiceLoader file similarly)
   and now have the jena-prometheus (which is out of sequence in the build currently).
   
   #### Testing
   
   If `jena-prometheus` is not available I get a `500 server error` and a stacktrace, not 501 (running Fuseki main+Prometheus)
   
   {noformat}
   [12:47:13] INFO  Server               :: Start Fuseki (port=3030)
   [12:47:18] WARN  HttpChannel          :: /$/metrics/ds
   org.apache.jena.fuseki.servlets.ActionErrorException: SimpleMeterRegistry isn't scrapeable
   	at org.apache.jena.fuseki.servlets.ServletOps.error(ServletOps.java:143)
   	at org.apache.jena.fuseki.servlets.ServletOps.errorNotImplemented(ServletOps.java:116)
   	at org.apache.jena.fuseki.metrics.SimpleMetricsProvider.scrape(SimpleMetricsProvider.java:36)
   	at org.apache.jena.fuseki.ctl.ActionMetrics.perform(ActionMetrics.java:51)
   	at org.apache.jena.fuseki.ctl.ActionMetrics.doGet(ActionMetrics.java:42)
   	at javax.servlet.http.HttpServlet.service(HttpServlet.java:687)
   {noformat}
   
   Some tests of the metrics code would be good.  Testing the server is tricky because you have to have a server and `jena-fuseki-core` isn't a whole server. test can go into jena-fuseki-main for functional testing and ideally  jena-fuseki-webapp (just to test web.xml/shiro.ini setup).
   
   #### Code comment
   
   The direct adding of `MeterRegistry` to everywhere `DataAccessPointRegistry` is created feels wrong :-) but this does not have to be looked at now. The current way works.
   
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services