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

Metrics cleanup #40

Closed stanlemon closed 6 years ago

stanlemon commented 6 years ago

FWIW @Crim I do think this is an improvement over the metrics handling in VirtualSpout today.

Crim commented 6 years ago

I need to get a chance re-review this. I think there's some good ideas here, just need to massage it a bit.

Crim commented 6 years ago

@stanlemon give this a look over and let me know what you think. I went back over it and realized I made a few stupid mistakes (re-implemented counter() essentially) and fixed it up.

Its pretty verbose, and at some point we should probably remove these metrics, but for now they're kind of handy to diagnose perf issues in the critical logic loops.

stanlemon commented 6 years ago

Looks good @Crim - I can't approve since I opened this PR. I'd say update the README/CHANGELOG and lets get this merged.

Crim commented 6 years ago

Not sure these metrics warrant being documented since they're measuring internal performance metrics really only relevant to developers, and I expect them to be removed at some point in the future. I did document the change to timers now publishing a total time metric however.

One thing this has convinced me of is that we should probably go ahead with your idea of enumerating these metrics so we can auto-generate documentation.