----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70116/#review214068 -----------------------------------------------------------
Fix it, then Ship it! LGTM. Two small-ish comments to tweak before commit. docs/monitoring.md Lines 684-685 (patched) <https://reviews.apache.org/r/70116/#comment300239> You can mention that terminal states, like FINISHED, FAILED, ERROR, DROPPED, or GONE are counters. Non-terminal states are gauges. Here and a couple lines below too. The distinction is (sometimes) important when consuming these metrics because Counters are usually consumed as differences over time (e.g. how many more finished operations are there since the last minute?) whereas gauges are consumed as-is. src/master/metrics.hpp Lines 82-84 (patched) <https://reviews.apache.org/r/70116/#comment300240> The framework-wide metrics are not implemented in this review, so you could remove mention of them for now. - Joseph Wu On March 26, 2019, 10:56 a.m., Benno Evers wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70116/ > ----------------------------------------------------------- > > (Updated March 26, 2019, 10:56 a.m.) > > > Review request for mesos, Gastón Kleiman, Greg Mann, and Joseph Wu. > > > Bugs: MESOS-8241 > https://issues.apache.org/jira/browse/MESOS-8241 > > > Repository: mesos > > > Description > ------- > > This commit adds additional metrics counting the > number of operations in each state. > > Unit tests are added in the subsequent commit. > > > Diffs > ----- > > docs/monitoring.md 54b872f579dbc68ca5f67f4cc1ba34065a09aee2 > src/master/master.cpp 9c4a9e83da94535873d72c902835f229c4f96320 > src/master/metrics.hpp 4495e65b6bb11f7236335a702c4f61e7c3f9b0aa > src/master/metrics.cpp 4dd73fb18a06ce8f75c4c1435dba84ade123bee9 > > > Diff: https://reviews.apache.org/r/70116/diff/5/ > > > Testing > ------- > > > Thanks, > > Benno Evers > >
