search

salesforce / storm-dynamic-spout

A framework for building spouts for Apache Storm and a Kafka based spout for dynamically skipping messages to be processed later.
BSD 3-Clause "New" or "Revised" License
40 stars 13 forks source link

Provide mechanism for defining metrics #59

Closed Crim closed 6 years ago

Crim commented 6 years ago

Don't really like the API that this defines, and I'm totally OK will throwing away this code. At the very least we can take the enum defined here w/ the names, descriptions etc.. and re-work everything into something better.

The main issue is a lot of our metrics use dynamic keys, which makes pre-defining them in an ENUM more difficult. The approach I went with is you can define your keys with {} place holders, and then pass arguments to replace the placeholders. Perhaps @stanlemon knows of a better way to deal w/ this?

stanlemon commented 6 years ago

@Crim I rebased this and sorted through all of the merge issues. Please make sure you delete this branch locally and pull down a fresh version.

stanlemon commented 6 years ago

@Crim I think the enums make this more challenging that it needs to be. I've switched that to a final class that holds constants, similar to what we do for the config. I've also added ClassMetric and simplified CustomMetric, the former of which uses an actual Class for stricter typing and the latter that just holds a String. I'm going to stew on this some more, but I think I prefer this. I will say I am reconsidering the use of enum's for the Category property on the Documentation annotation (same goes for the config too), because I feel like we have to couple in a way I don't want to do.

stanlemon commented 6 years ago

@Crim I think I'm done here... what do you think?