[GitHub] [jena] afs edited a comment on issue #534: [WIP] Proof of concept for prometheus endpoint
afs edited a comment on issue #534: [WIP] Proof of concept for prometheus
endpoint
URL: https://github.com/apache/jena/pull/534#issuecomment-479922355
Licensing
@claudenw - would you be able to help out with the L 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)
```
[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)
```
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [jena] afs edited a comment on issue #534: [WIP] Proof of concept for prometheus endpoint
afs edited a comment 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 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)
```
[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)
```
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [jena] afs edited a comment on issue #534: [WIP] Proof of concept for prometheus endpoint
afs edited a comment on issue #534: [WIP] Proof of concept for prometheus
endpoint
URL: https://github.com/apache/jena/pull/534#issuecomment-475252662
This is looking good. I have it working in Fuseki Full (webapp form) and in
Fuseki Main (the more configurable servlet form) - details for running in
Fuseki Main below.
Configuration
Something we have to think about carefully is using `ServiceLoader`. Jena
already has an [init
mechanism](https://jena.apache.org/documentation/notes/system-initialization.html)
and it allows controlling the order of initialization. It is quite simple -
there is an `init()` method which might be a limitation.
`ActionPrometheus` could use this to wire in a function lambda to call it's
`perform()` into a registry.
I don't know at the moment which of a "different ServiceLoader" or "single
ServiceLoader" is better.
Naming
Rather than hide under `/$/extras/` I think I prefer `/$/metrics/` or
something else to cover just statistics reporting. This is a significant
functionality, not to be hidden under "extras".
Dispatch
The `ActionExtras.perform` could dispatch on the final component of the URI.
No fixed string!
It might even be better to look for the dispatch string in
`init(ServletConfig)` to find the prefix for this servlet instance.
HTTP MIME Type
The HTTP return is set as `application/json;charset=utf-8` but it isn't
JSON. If there isn't a specific MIME type, then `text/plain;charset=utf-8`
would be better (?).
jena-prometheus jar
The shaded JAR has got slf4j classes in it and io.micrometer. I don't think
they are needed. Can they be excluded? either in the POM or in the shader.
Maybe it is an effect of the `provided`.
[jena-fuseki2/jena-fuseki-server/pom.xml](https://github.com/apache/jena/blob/master/jena-fuseki2/jena-fuseki-server/pom.xml)
has an example of shading Jena including license control.
Licensing
`org.hdrhistogram` and `org.latencyutils` are 2-clause BSD license and we a
shipping them in shared artifacts so the acknowledge clause applies. No
problem, just a task to do. The right LICENSE and NOTICE need to end up in the
shaded jar.
Fuseki Main example
Illustration of running in `FusekiMain`:
public static void mainServer(String ... a) {
FusekiLogging.setLogging();
FusekiServer server = FusekiServer.create()
.add("/ds", DatasetGraphFactory.createTxnMem())
.port(3030)
.verbose(true)
.addServlet("/$/extras/*", new ActionExtras())
.build();
try { server.start().join(); }
finally { server.stop(); }
}
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [jena] afs edited a comment on issue #534: [WIP] Proof of concept for prometheus endpoint
afs edited a comment on issue #534: [WIP] Proof of concept for prometheus
endpoint
URL: https://github.com/apache/jena/pull/534#issuecomment-475252662
This is looking good. I have it working in Fuseki Full (webapp form) and in
Fuseki Main (the more configurable servlet form) - details for running in
Fuseki Main below.
Configuration
Something we have to think about carefully is using `ServiceLoader`. Jena
already has an [init
mechanism](https://jena.apache.org/documentation/notes/system-initialization.html)
and it allows controlling the order of initialization. It is quite simple -
there is an `init()` method which might be a limitation.
`ActionPrometheus` could use this to wire in a function lambda to call it's
`perform()` into a registry.
I don't know at the moment which of a "different ServiceLoader" or "single
ServiceLoader" is better.
Naming
Rather than hide under `/$/extras/` I think I prefer `/$/metrics/` or
something else to cover just statistics reporting. This is a significant
functionality, not to be hidden under "extras".
Dispatch
The `ActionExtras.perform` could dispatch on the final component of the URI.
No fixed string!
It might even be better to look for the dispatch string in
`init(ServletConfig)` to find the prefix for this servlet instance.
HTTP MIME Type
The HTTP return is set as `application/json;charset=utf-8` but it isn't
JSON. If there isn't a specific MIME type, then `text/plain;charset=utf-8`
would be better (?).
jena-prometheus jar
The shaded JAR has got slf4j classes in it. I don't think they are needed.
Can they be excluded
? either in the POM or in the shader. Maybe it is an effect of the
`provided`.
[jena-fuseki2/jena-fuseki-server/pom.xml](https://github.com/apache/jena/blob/master/jena-fuseki2/jena-fuseki-server/pom.xml)
has an example of shading Jena including license control.
Licensing
`org.hdrhistogram` and `org.latencyutils` are 2-clause BSD license and we a
shipping them in shared artifacts so the acknowledge clause applies. No
problem, just a task to do. The right LICENSE and NOTICE need to end up in the
shaded jar.
Fuseki Main example
Illustration of running in `FusekiMain`:
public static void mainServer(String ... a) {
FusekiLogging.setLogging();
FusekiServer server = FusekiServer.create()
.add("/ds", DatasetGraphFactory.createTxnMem())
.port(3030)
.verbose(true)
.addServlet("/$/extras/*", new ActionExtras())
.build();
try { server.start().join(); }
finally { server.stop(); }
}
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:
us...@infra.apache.org
With regards,
Apache Git Services
