Javadoc fix in simpleclient_servlet #417
Conversation
Signed-off-by: Raphael P. Barazzutti <raphael@barazzutti.net>
| /** | ||
| * The servlet exposes the metrics | ||
| * | ||
| * <p>Example config to export metrics on path {@code /metrics}: |
There was a problem hiding this comment.
This makes no sense to me as it lacks context. What is it that takes this configuration?
As this is not tied to the servlet itself, I don't think this should be here.
There was a problem hiding this comment.
thanks for the feedback. tried to address that
rbarazzutti
force-pushed
the
javadoc_fix
branch
from
2b80f03
to
de673b2
Compare
Signed-off-by: Raphael P. Barazzutti <raphael@barazzutti.net>
Signed-off-by: Raphael P. Barazzutti <raphael@barazzutti.net>
rbarazzutti
force-pushed
the
javadoc_fix
branch
from
de673b2
to
ce2df5a
Compare
| /** | ||
| * The MetricsServlet class exists to provide a simple way of exposing the metrics values. | ||
| * | ||
| * <p>This servlet might be used with a filter such the {@link io.prometheus.client.filter.MetricsFilter} |
There was a problem hiding this comment.
That's for something else.
There was a problem hiding this comment.
This line shouldn't be here, exposition and instrumentation are separate concerns.
| * | ||
| * <p>This servlet might be used with a filter such the {@link io.prometheus.client.filter.MetricsFilter} | ||
| * | ||
| * <p>Usage example (in webapp's {@code web.xml}): |
There was a problem hiding this comment.
What's a web.xml?
This seems specific to one particular framework, and only one way that this could be used. The changes to this file don't belong here.
There was a problem hiding this comment.
The web.xml file is the deployment descriptor for a Servlet-based Java web application. It's pretty common.
The XML snippet that is in MetricsFilter.java is also referring to that web.xml.
A user of the servlet may want to know how to use it, this way is the most straightforward.
There was a problem hiding this comment.
I personally have never heard of it, nor have I ever needed to use it to use servlets. This sounds like something specific to the framework you are using, and not generally applicable and thus doesn't belong here.
There was a problem hiding this comment.
All major Java servlet containers handle it (all containers that are compliant with JSR-154, or newer JSRs).
Currently, this module depends on Java Servlet 3.0, that implements JSR-315 (http://download.oracle.com/otn-pub/jcp/servlet-3.0-fr-eval-oth-JSpec/servlet-3_0-final-spec.pdf), that describes pretty well how to use it.
Could you please tell me where the XML snippet that is in MetricsFilter.java should be used then?
There was a problem hiding this comment.
I presumed it was something Spring related, as it came in in the same PR. Claiming that this applies to all web applications seems a bit broad.
There was a problem hiding this comment.
This PR is "Spring-free" ...
I don't claim it applies to all web application. But for sure, its the most common way of deploying a very simple servlet (especially when not using an extra framework). It's a simple example, that helps many.
But I'm still trying to figure out why this syntax is fine in MetricsFilter.java and not with the servlet. I added it for consistency.
There was a problem hiding this comment.
The current text claims it works for all web applications, which is not true.
There was a problem hiding this comment.
Hi @brian-brazil,
I'll see how to rephrase this.
But it is supposed to work on any web app container that supports the servlet 3.0 specs (which is referred by the pom.xml of this module).
Cheers
Signed-off-by: Raphael P. Barazzutti <raphael@barazzutti.net>
Signed-off-by: Raphael P. Barazzutti <raphael@barazzutti.net>
|
Thanks! |
hi @brian-brazil,
a small PR with fixes in the JavaDoc (displays properly the XML code)