Re: Review Request 53297: Initial version of adding metrics into samza rest.

[Jake Maes]

> On Nov. 5, 2016, 6:47 p.m., Jake Maes wrote:
> > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java, line 
> > 75
> > 
> >
> > I don't think the MetricsConfig constructure takes a subset. 
> > 
> > I think it takes the root and expects to find the "metrics" prefix

s/constructure/constructor

phonetic brain fail


- Jake


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


On Oct. 31, 2016, 9:18 p.m., Shanthoosh Venkataraman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53297/
> ---
> 
> (Updated Oct. 31, 2016, 9:18 p.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/scala/org/apache/samza/container/SamzaContainer.scala 
> e0468ee89c89fd720834461771ebb36475475bcb 
>   
> samza-core/src/main/scala/org/apache/samza/metrics/ContainerProcessManagerMetrics.scala
>  f24beb1e099dd44b15b475e0a4a7f70560c6965e 
>   samza-core/src/main/scala/org/apache/samza/util/MetricsReporterLoader.scala 
> PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/util/Util.scala 
> c4836f202f7eda1d4e71eac94fd48e46207b0316 
>   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/resources/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
> 
>

Re: Review Request 53297: Initial version of adding metrics into samza rest.

[Jake Maes]

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




samza-core/src/main/scala/org/apache/samza/util/MetricsReporterLoader.scala 
(line 40)


We're moving away from Scala. All new files should be Java.



samza-core/src/main/scala/org/apache/samza/util/Util.scala (line 335)


This edit looks like a mistake.

Did this file need to be modified at all?



samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java (line 60)


The Resources will eventually emit metrics too, so I think this value is 
too specific.



samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java (line 73)


What's the purpose of this static factory?

The typical reason is to construct the object differently (e.g. a different 
subclass) based on some parameter. 

I don't see any value of the approach here.

If there is some value, then the constructor should be made private.



samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java (line 75)


I don't think the MetricsConfig constructure takes a subset. 

I think it takes the root and expects to find the "metrics" prefix


- Jake Maes


On Oct. 31, 2016, 9:18 p.m., Shanthoosh Venkataraman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53297/
> ---
> 
> (Updated Oct. 31, 2016, 9:18 p.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/scala/org/apache/samza/container/SamzaContainer.scala 
> e0468ee89c89fd720834461771ebb36475475bcb 
>   
> samza-core/src/main/scala/org/apache/samza/metrics/ContainerProcessManagerMetrics.scala
>  f24beb1e099dd44b15b475e0a4a7f70560c6965e 
>   samza-core/src/main/scala/org/apache/samza/util/MetricsReporterLoader.scala 
> PRE-CREATION 
>   samza-core/src/main/scala/org/apache/samza/util/Util.scala 
> c4836f202f7eda1d4e71eac94fd48e46207b0316 
>   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/resources/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
> 
>

Re: Using json serde with KeyValueStore

[Jagadish Venkatraman]

You can write a small test job and verify this! I believe the serde should
probably be able to serialize it correctly.

You can define the KeyValueStore to be typed as .

- You can put the LHM directly into the store.
-  When you do a get on the store, you should probably be able re-cast it
back to an LHM.

String/json serialization is not necessarily performant for all
cases. There maybe other efficient serialization formats like protobuffs/
avro depending on the use case. Should you want to write your own
serialization have a look at the SerdeFactory interface.

Thanks
Jag

On Saturday, November 5, 2016, Raj raj  wrote:

> Hi,
>
> I am starting out with Samza and my test program is able to read a
> message from Kafka. My next task is to write the message to KeyValue
> store. The message is formatted as Json in Kafka and I am using json
> serde as my msg serde. The incoming message is available to me as a
> LinkedHashMap when I call envelope.getMessage(). i.e. serde
> automatically converted the json string to a LinkedHashMap.
>
> Now I want to store this to KeyValueStore. Will using a json serde for
> stores.test-container.msg.serde automatically convert the
> LinkedHashMap to a format suitable for storing in the KeyValueStore?
> i.e. the reverse process of what it did when I called
> envelope.getMessage().
>
> If it is not possible what is the best practice of storing a
> LinkedHashMap in KeyStoreValue?
>
> I am pasting the current code I am working with in pastebin for
> reference, there isn't much in the code, just the boilerplate stuff.
>
> test-container.properties : http://pastebin.com/dkbnjtNh
> TestContainer.java : http://pastebin.com/93ZBgeWa
>
> Thanks and Regards,
>
> Raj
>


-- 
Sent from my iphone.

Using json serde with KeyValueStore

[Raj raj]

Hi,

I am starting out with Samza and my test program is able to read a
message from Kafka. My next task is to write the message to KeyValue
store. The message is formatted as Json in Kafka and I am using json
serde as my msg serde. The incoming message is available to me as a
LinkedHashMap when I call envelope.getMessage(). i.e. serde
automatically converted the json string to a LinkedHashMap.

Now I want to store this to KeyValueStore. Will using a json serde for
stores.test-container.msg.serde automatically convert the
LinkedHashMap to a format suitable for storing in the KeyValueStore?
i.e. the reverse process of what it did when I called
envelope.getMessage().

If it is not possible what is the best practice of storing a
LinkedHashMap in KeyStoreValue?

I am pasting the current code I am working with in pastebin for
reference, there isn't much in the code, just the boilerplate stuff.

test-container.properties : http://pastebin.com/dkbnjtNh
TestContainer.java : http://pastebin.com/93ZBgeWa

Thanks and Regards,

Raj