Re: Review Request 38296: SAMZA-341: Support metrics report via Ganglia

2015-09-14 Thread Yan Fang

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



build.gradle (line 219)


if we only use it as isEmpty, do not think worth doing this.



docs/learn/documentation/versioned/container/metrics.md (line 25)


as mentioned in SAMZA-340's review, can we move this part to the end of 
this doc? Because Kafka's way is a little more recommeded. (no bias to Ganglia. 
:)



docs/learn/documentation/versioned/jobs/configuration-table.html (line 1387)


is this the default port for Ganglia or Graphite?



samza-ganglia/src/main/java/org/apache/samza/metrics/reporter/SamzaGangliaReporter.java
 (line 41)


add java doc



samza-ganglia/src/main/java/org/apache/samza/metrics/reporter/SamzaGangliaReporter.java
 (line 44)


should be SamzaGangliaReporter



samza-ganglia/src/main/java/org/apache/samza/metrics/reporter/SamzaGangliaReporter.java
 (line 62)


1. if there is only one port name, should it be UNICAST?
Maybe it is better to use getModeForAddress to determin it.

2. why is the ttl "1"? do we make it configurable?



samza-ganglia/src/main/java/org/apache/samza/metrics/reporter/SamzaGangliaReporter.java
 (line 66)


maybe throw the Samza Exception to kill the job if the Ganglia is not 
available?



samza-ganglia/src/main/java/org/apache/samza/metrics/reporter/SamzaGangliaReporter.java
 (line 72)


wrap it with SamzaException



samza-ganglia/src/main/java/org/apache/samza/metrics/reporter/SamzaGangliaReporter.java
 (line 101)


change to error level



samza-ganglia/src/main/java/org/apache/samza/metrics/reporter/SamzaGangliaReporter.java
 (line 112)


same, error level



samza-ganglia/src/main/java/org/apache/samza/metrics/reporter/SamzaGangliaReporter.java
 (line 123)


error level



samza-ganglia/src/main/java/org/apache/samza/metrics/reporter/SamzaGangliaReporter.java
 (line 143)


error



samza-ganglia/src/main/java/org/apache/samza/metrics/reporter/SamzaGangliaReporter.java
 (line 154)


error



samza-ganglia/src/main/java/org/apache/samza/metrics/reporter/SamzaGangliaReporter.java
 (line 165)


error



samza-ganglia/src/test/java/org/apache/samza/metrics/reporter/GangliaCounterTest.java
 (line 31)


when the test are not run in the order shown in the class, the asserts will 
be incorrect.


- Yan Fang


On Sept. 11, 2015, 1:22 p.m., Aleksandar Pejakovic wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38296/
> ---
> 
> (Updated Sept. 11, 2015, 1:22 p.m.)
> 
> 
> Review request for samza.
> 
> 
> Repository: samza
> 
> 
> Description
> ---
> 
> Added new moduo for Ganglia support.
> 
> Implemented come is based on 
> [SAMZA-340](https://issues.apache.org/jira/browse/SAMZA-340)
> 
> 
> Diffs
> -
> 
>   build.gradle 3a7fabc 
>   checkstyle/import-control.xml bc07ae8 
>   docs/learn/documentation/versioned/container/metrics.md 11a62f9 
>   docs/learn/documentation/versioned/jobs/configuration-table.html c23d7d3 
>   gradle/dependency-versions.gradle 36d564b 
>   
> samza-ganglia/src/main/java/org/apache/samza/metrics/reporter/GangliaCounter.java
>  PRE-CREATION 
>   
> samza-ganglia/src/main/java/org/apache/samza/metrics/reporter/GangliaGauge.java
>  PRE-CREATION 
>   
> samza-ganglia/src/main/java/org/apache/samza/metrics/reporter/GangliaReporterFactory.java
>  PRE-CREATION 
>   
> samza-ganglia/src/main/java/org/apache/samza/metrics/reporter/GangliaSnapshot.java
>  PRE-CREATION 
>   
> samza-ganglia/src/main/java/org/apache/samza/metrics/reporter/GangliaTimer.java
>  PRE-CREATION 
>   
> samza-ganglia/src/main/java/org/apache/samza/metrics/reporter/SamzaGangliaReporter.java
>  PRE-CREATION 
>   
> samza-ganglia/src/test/java/org/apache/samza/metrics/reporter/GangliaCounterTest.java
>  PRE-CREATION 
>   
> samza-ganglia/src/test/java/org/apache/samza/metrics/reporter/GangliaGaugeTest.java
>  PRE-CREATION 
>   
> 

Review Request 38296: SAMZA-341: Support metrics report via Ganglia

2015-09-11 Thread Aleksandar Pejakovic

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

Review request for samza.


Repository: samza


Description
---

Added new moduo for Ganglia support.

Implemented come is based on 
[SAMZA-340](https://issues.apache.org/jira/browse/SAMZA-340)


Diffs
-

  build.gradle 3a7fabc 
  checkstyle/import-control.xml bc07ae8 
  docs/learn/documentation/versioned/container/metrics.md 11a62f9 
  docs/learn/documentation/versioned/jobs/configuration-table.html c23d7d3 
  gradle/dependency-versions.gradle 36d564b 
  
samza-ganglia/src/main/java/org/apache/samza/metrics/reporter/GangliaCounter.java
 PRE-CREATION 
  
samza-ganglia/src/main/java/org/apache/samza/metrics/reporter/GangliaGauge.java 
PRE-CREATION 
  
samza-ganglia/src/main/java/org/apache/samza/metrics/reporter/GangliaReporterFactory.java
 PRE-CREATION 
  
samza-ganglia/src/main/java/org/apache/samza/metrics/reporter/GangliaSnapshot.java
 PRE-CREATION 
  
samza-ganglia/src/main/java/org/apache/samza/metrics/reporter/GangliaTimer.java 
PRE-CREATION 
  
samza-ganglia/src/main/java/org/apache/samza/metrics/reporter/SamzaGangliaReporter.java
 PRE-CREATION 
  
samza-ganglia/src/test/java/org/apache/samza/metrics/reporter/GangliaCounterTest.java
 PRE-CREATION 
  
samza-ganglia/src/test/java/org/apache/samza/metrics/reporter/GangliaGaugeTest.java
 PRE-CREATION 
  
samza-ganglia/src/test/java/org/apache/samza/metrics/reporter/GangliaReporterTest.java
 PRE-CREATION 
  
samza-ganglia/src/test/java/org/apache/samza/metrics/reporter/GangliaSnapshotTest.java
 PRE-CREATION 
  
samza-ganglia/src/test/java/org/apache/samza/metrics/reporter/GangliaTimerTest.java
 PRE-CREATION 
  settings.gradle 4c1aa10 

Diff: https://reviews.apache.org/r/38296/diff/


Testing
---


Thanks,

Aleksandar Pejakovic