flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From tillrohrmann <...@git.apache.org>
Subject [GitHub] flink issue #2112: [FLINK-3951] Add histogram metric type
Date Thu, 16 Jun 2016 12:43:03 GMT
Github user tillrohrmann commented on the issue:

    https://github.com/apache/flink/pull/2112
  
    We decided to add the histogram metric, because users requested this feature. The important
thing is that the histograms are not used by the runtime per default, because they indeed
don't come for free. But if the user decides to use such a metric, then he should be able
to explicitly register a histogram.
    
    Additionally, we don't want to add a strict dependency on dropwizard from flink-core.
Therefore, I introduced the `Histogram` interface which effectively hides the concrete implementation.
So later we can easily swap the implementation out.
    
    One of these implementations is the `DropwizardHistogramWrapper` which allows you to use
Dropwizard histograms in Flink. The reason for this is that Dropwizard's `Histogram` has already
a lot of functionality (different reservoirs for sampling streams, for example) and it would
be a lot of duplicate work to reimplement it.
    
    Since the assumption is that `Histograms` are a user metric, it seems to be ok for me
to not ship an implementation of the interface with flink-core but to require the user to
include an additional module such as flink-metrics-dropwizard. This module would add the dropwizard
dependency anyway. If necessary, then we can also add our own histogram implementation later.
    
    The PR does not introduce a Dropwizard compatibility layer. Dropwizard is only one of
many possible implementation for our metrics (counter, gauge and histogram so far). It is
up to the community to decide which other metrics to add.
    
    I agree that the naming of `HistogramWrapper` is not so distinctive. I actually sticked
to the naming scheme for `Counter` and `Gauge` which are called `CounterWrapper` and `GaugeWrapper`.
This can be  easily changed. Do you have a proposal?
    
    I'm sorry if you feel angry because you already did some of this work which was later
removed. But please keep in mind that not everyone was involved in the metrics PR.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message