> On Aug. 13, 2012, 11:09 a.m., Mubarak Seyed wrote:
> > lgtm.
> > 
> > Hi Hari,
> > One suggestion. There is a FLUME-967 for HTTP based monitoring. Can we 
> > combine both 1482 and 967 as single feature with passing type as query 
> > parameter? e.g. /metrics?type=json (or) /metrics?type=xxxx (table | plain).
> > If we have to implement 967 then it requires one more port (with Jetty 
> > server). We can refactor handle() to cover both cases. I am thinking of 
> > embedding HTML (as like how servlet generates) to represent metric values 
> > in tabular format, if type == plain (or) table.
> > 
> > Thoughts? Thanks.

Mubarak,

Sorry - I didn't see FLUME-967 before starting work on this. It should be 
fairly simple to integrate the 967 style metrics no? I'd like to leave JSON as 
default - but you could do something like this in the handle() method and 
resubmit the FLUME-967 patch:

Map<String, Map<String, String> metrics = JMXUtils.getAllMbeans();

if(request.getParameter().equalsIgnoreCase("plain")) {
//parse the table into FLUME-967 format
} else {
//Default JSON parsing code from this patch
}

I don't want to add this code to the current patch because there are no 
multiple options here right now - so adding it to FLUME-967 makes more sense.
I will rename the classes and type to HTTP to make it more extensible, so we 
can support more formats later if we want. So based on the params, we return 
the values - if there are no params or there are params we can't understand 
return json (that is upto you to decide - maybe not a valid param can return 
error, we shall look at it FLUME-967).

Does that sound good? I will submit a patch with the names changed.


- Hari


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6567/#review10200
-----------------------------------------------------------


On Aug. 13, 2012, 7:24 a.m., Hari Shreedharan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6567/
> -----------------------------------------------------------
> 
> (Updated Aug. 13, 2012, 7:24 a.m.)
> 
> 
> Review request for Flume.
> 
> 
> Description
> -------
> 
> Support for expose metrics by HTTP. Refactored Ganglia metrics to move the 
> JMX polling code to a util to be reused by JSON server as well.
> 
> 
> This addresses bug FLUME-1482.
>     https://issues.apache.org/jira/browse/FLUME-1482
> 
> 
> Diffs
> -----
> 
>   
> flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java
>  cc8f89a 
>   flume-ng-core/pom.xml 8dd0d3e 
>   flume-ng-core/src/main/java/org/apache/flume/channel/MemoryChannel.java 
> 65b0166 
>   
> flume-ng-core/src/main/java/org/apache/flume/channel/PseudoTxnMemoryChannel.java
>  489d3e5 
>   
> flume-ng-core/src/main/java/org/apache/flume/instrumentation/ChannelCounter.java
>  316384a 
>   
> flume-ng-core/src/main/java/org/apache/flume/instrumentation/ChannelCounterMBean.java
>  799dd5d 
>   
> flume-ng-core/src/main/java/org/apache/flume/instrumentation/GangliaServer.java
>  d93cd33 
>   
> flume-ng-core/src/main/java/org/apache/flume/instrumentation/MonitoredCounterGroup.java
>  a03d004 
>   
> flume-ng-core/src/main/java/org/apache/flume/instrumentation/MonitoringType.java
>  d132995 
>   
> flume-ng-core/src/main/java/org/apache/flume/instrumentation/SinkCounterMBean.java
>  6905d49 
>   
> flume-ng-core/src/main/java/org/apache/flume/instrumentation/SourceCounterMBean.java
>  e6612d5 
>   
> flume-ng-core/src/main/java/org/apache/flume/instrumentation/json/JSONMetricsServer.java
>  PRE-CREATION 
>   
> flume-ng-core/src/main/java/org/apache/flume/instrumentation/util/JMXPollUtil.java
>  PRE-CREATION 
>   
> flume-ng-core/src/test/java/org/apache/flume/instrumentation/json/TestJSONMetricsServer.java
>  PRE-CREATION 
>   
> flume-ng-core/src/test/java/org/apache/flume/instrumentation/util/JMXTestUtils.java
>  PRE-CREATION 
>   
> flume-ng-core/src/test/java/org/apache/flume/instrumentation/util/TestJMXPollUtil.java
>  PRE-CREATION 
>   flume-ng-doc/sphinx/FlumeUserGuide.rst 45dd7cc 
>   pom.xml 8c67610 
> 
> Diff: https://reviews.apache.org/r/6567/diff/
> 
> 
> Testing
> -------
> 
> Added unit tests for JSON server and JMX polling code.
> 
> 
> Thanks,
> 
> Hari Shreedharan
> 
>

Reply via email to