You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jiang Yan Xu <ya...@jxu.me> on 2017/09/01 18:02:49 UTC

Re: Review Request 60960: Added an enum to track Metric semantics.

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



As suggested in https://issues.apache.org/jira/browse/MESOS-6918, we should summarize the high level design first but I am asking questions here because it's closer to the context.


3rdparty/libprocess/include/process/metrics/metric.hpp
Lines 34-40 (patched)
<https://reviews.apache.org/r/60960/#comment260521>

    AFAIK these are not generally accepted meaning of these terms even though Prometheus defines them this way. Currently in Mesos the word *counter* is already used for an `int64_t` typed value, although the monotonically increasing part is common. I see this use of *counter* to describe a metric type elsewhere ([eg](http://metrics.dropwizard.io/3.1.0/manual/core/#man-core-counters)) as well. The case for gauges is similar too. 
    
    So if this is used to distinguish the metric types when rendering Prometheus output, we can probably find another way to retain the metric type as they are (i.e., Counter, Gauge, Timer, as we really just have a few of these types).


- Jiang Yan Xu


On July 19, 2017, 2:02 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60960/
> -----------------------------------------------------------
> 
> (Updated July 19, 2017, 2:02 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6918
>     https://issues.apache.org/jira/browse/MESOS-6918
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a Semantics enumeration to to process::Metrics to track the
> semantics content of a Metric. This is needed for Prometheus metrics
> output where we need to know what Prometheus data type to attribute
> to each Metric.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/metrics/counter.hpp 15aeeb5710636d4e11b862faee50fd6ea4d1cb07 
>   3rdparty/libprocess/include/process/metrics/gauge.hpp 474f8e80b0128ae8d742a022733019351710ef48 
>   3rdparty/libprocess/include/process/metrics/metric.hpp 21f162d5b7d9e56dc3289d65b6d86deb4c2fa721 
>   3rdparty/libprocess/include/process/metrics/timer.hpp 0a9c0227c457c6c81a59f65f901a5464ee00983d 
> 
> 
> Diff: https://reviews.apache.org/r/60960/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 60960: Added an enum to track Metric semantics.

Posted by James Peach <jp...@apache.org>.

> On Sept. 1, 2017, 6:02 p.m., Jiang Yan Xu wrote:
> > 3rdparty/libprocess/include/process/metrics/metric.hpp
> > Lines 34-40 (patched)
> > <https://reviews.apache.org/r/60960/diff/3/?file=1782397#file1782397line34>
> >
> >     AFAIK these are not generally accepted meaning of these terms even though Prometheus defines them this way. Currently in Mesos the word *counter* is already used for an `int64_t` typed value, although the monotonically increasing part is common. I see this use of *counter* to describe a metric type elsewhere ([eg](http://metrics.dropwizard.io/3.1.0/manual/core/#man-core-counters)) as well. The case for gauges is similar too. 
> >     
> >     So if this is used to distinguish the metric types when rendering Prometheus output, we can probably find another way to retain the metric type as they are (i.e., Counter, Gauge, Timer, as we really just have a few of these types).

You are confusing class/implementation type with abstract metrics semantics.


- James


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


On July 19, 2017, 9:02 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60960/
> -----------------------------------------------------------
> 
> (Updated July 19, 2017, 9:02 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6918
>     https://issues.apache.org/jira/browse/MESOS-6918
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added a Semantics enumeration to to process::Metrics to track the
> semantics content of a Metric. This is needed for Prometheus metrics
> output where we need to know what Prometheus data type to attribute
> to each Metric.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/metrics/counter.hpp 15aeeb5710636d4e11b862faee50fd6ea4d1cb07 
>   3rdparty/libprocess/include/process/metrics/gauge.hpp 474f8e80b0128ae8d742a022733019351710ef48 
>   3rdparty/libprocess/include/process/metrics/metric.hpp 21f162d5b7d9e56dc3289d65b6d86deb4c2fa721 
>   3rdparty/libprocess/include/process/metrics/timer.hpp 0a9c0227c457c6c81a59f65f901a5464ee00983d 
> 
> 
> Diff: https://reviews.apache.org/r/60960/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> James Peach
> 
>