samza-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Shanthoosh Venkataraman <santhoshvenkat1...@gmail.com>
Subject Re: Review Request 53297: Initial version of adding metrics into samza rest.
Date Thu, 10 Nov 2016 01:04:16 GMT


> On Nov. 9, 2016, 12:09 a.m., Jagadish Venkatraman wrote:
> > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestConfig.java, line 55
> > <https://reviews.apache.org/r/53297/diff/2/?file=1556964#file1556964line55>
> >
> >     How exactly is this used when metrics are reported? 
> >     
> >     Is there a reason for this to be configurable / adding a new config key? 
> >     
> >     If it's configurable, we should document it somewhere?
> 
> Shanthoosh Venkataraman wrote:
>     Container name here refers to the execution unit from which the metrics gets reported.
It's used when instantiating the MetricsReporter. If this configuration is undefined, it's
defaulted to hostName on which SamzaRest is running. Resources will report metrics alongside
monitors. Added relevent documentation for it.
> 
> Jagadish Venkatraman wrote:
>     Is there a use case where I would want to configure a different container name? If
not, this need not be a config? (we can default it to `samza-rest`)
> 
> Shanthoosh Venkataraman wrote:
>     For LI use case, using hostname(or SamzaRest) would suffice.
>     My understanding is that the containerName denotes either the logical host or the
physical host or some unique tag to identify the origin from which the metrics gets reported
from. 
>     This container name could be appended into metrics reported depending upon MetricsReporter
implementations. 
>     Hence, I think it will be useful to let users configure it. 
>     For instance, some implementations could generate a unique string as container name
based upon environment and populate it in the config to uniquely identify the environment
it's reporting the metrics from.

Discussed offline. Exposing containerName through samza-rest config might be unnecessary as
of now. Hardcoding it to hostname on which `samza-rest` is running.


- Shanthoosh


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


On Nov. 10, 2016, 1:04 a.m., Shanthoosh Venkataraman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53297/
> -----------------------------------------------------------
> 
> (Updated Nov. 10, 2016, 1:04 a.m.)
> 
> 
> Review request for samza and Jake Maes.
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> This patch aims at enabling users to define custom reporters to send metrics from the
monitors. Configurations required for the definition of the metrics reporters follows the
same convention as of the samza jobs.
> 
> 
> Diffs
> -----
> 
>   docs/learn/documentation/versioned/rest/monitors.md 46678bbe5fed99f767c3324dc9578ee1a64cec66

>   samza-core/src/main/java/org/apache/samza/util/MetricsReporterLoader.java PRE-CREATION

>   samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala e0468ee89c89fd720834461771ebb36475475bcb

>   samza-core/src/main/scala/org/apache/samza/metrics/ContainerProcessManagerMetrics.scala
f24beb1e099dd44b15b475e0a4a7f70560c6965e 
>   samza-rest/src/main/java/org/apache/samza/rest/SamzaRestConfig.java 47b0663637f6db187d86961377ee3ee203b73fdb

>   samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java 2a3e83a24a5343bb53b93fc9d0a647c1b253714b

>   samza-rest/src/test/java/org/apache/samza/rest/TestSamzaRestService.java PRE-CREATION

>   samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterMetrics.scala 8a5b4aaea6e11a5af999f12d50e5b6135dbc70ca

> 
> Diff: https://reviews.apache.org/r/53297/diff/
> 
> 
> Testing
> -------
> 
> Unit tests are done to verify the intended functionality.
> 
> 
> Thanks,
> 
> Shanthoosh Venkataraman
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message