[
https://issues.apache.org/jira/browse/KNOX-940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16031869#comment-16031869
]
Sandeep More commented on KNOX-940:
-----------------------------------
[~kamrul] I went through the patch and it looks good ! few more observations.
Can you also briefly describe the overall architecture and the url pattern that
can be used to access the metrics, it will be helpful for the review and for
future reference.
Also,
# In class DefaultMetricsService, any specific reason to make 'MetricRegistry
metrics' static.
# HealthServiceMessages class, the messages are too generic like "Exception
occurred" it would be good to have messages which indicate the cause of failure.
## Method "basicInfo(String original)" boes not appear to be used.
# MetricsResource class:
## Path has hard coded version (/v0/metrics), if the version is hard coded do
we really need it ?
## Using instanceof is probably a bad idea as pointed by [~lmccay] recently, it
breaks the [Open Close
Principle|http://www.oodesign.com/open-close-principle.html] and [Liskov's
Substitution Principle|
http://www.oodesign.com/liskov-s-substitution-principle.html]
## Why do we have a doPost method when it does not handle POST (or may be I
read it wrong)
## doGet() method produces JSON as well as XML, as per the method annotations
but the response content type is only JSON
## in the method getMetrics(), on error the response is
Response.ok().entity(String.format("Failed to reply correctly due to : %s ",
ioe)).build(), shouldn't it be something like a Response.serverError()..
# PingResource class
## Can you explain the purpose of this class
## Path has hard coded version (/v0/ping), if the version is hard coded do we
really need it ?
## Why do we have a doPost method when it does not handle POST (or may be I
read it wrong)
## in the method getPingResponse(), on error the response is
Response.ok().entity(String.format("Failed to reply correctly due to : %s ",
ioe)).build(), shouldn't it be something like a Response.serverError()..
Great work, thanks !
> Support REST access exposing metrics
> ------------------------------------
>
> Key: KNOX-940
> URL: https://issues.apache.org/jira/browse/KNOX-940
> Project: Apache Knox
> Issue Type: Task
> Affects Versions: 0.11.0
> Reporter: Mohammad Kamrul Islam
> Assignee: Mohammad Kamrul Islam
> Fix For: 0.13.0
>
> Attachments: KNOX-940.patch
>
>
> KNOX-643 added the support to publish metrics. However, the metrics are not
> accessible through REST. This JIRA is to add REST support.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
