[
https://issues.apache.org/jira/browse/FLINK-6053?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16366710#comment-16366710
]
ASF GitHub Bot commented on FLINK-6053:
---------------------------------------
GitHub user zentol opened a pull request:
https://github.com/apache/flink/pull/5501
[FLINK-6053][metrics] Add new Number-/StringGauge metric types
## What is the purpose of the change
This PR deprecates the `Gauge` metric type and introduces 2 new metric
types: `StringGauge` and `NumberGauge`. The generic nature of `Gauges` is a big
headache as it encourages exposing arbitrary objects, which not a single
reporter actually supports. Furthermore, making the type distinction also
proved to be difficult, as gauges may return null which fails `instanceof`
checks.
This change is for the most part backwards compatible:
* `StringGauges` are wrapped such that they are registered as both
`Gauge<String>` and `StringGauge`
* `NumberGauges` are wrapped such that they are registered as both
`Gauge<Number>` and `NumberGauge`
* `Gauges<?>` are wrapper such that they are registered as both `Gauge<?>`
and `StringGauge`.
Thus, legacy reporters still work with the new metric types, and old
metrics still work with new reporters, provided the reporter supports strings.
We _could_ improve the compatibility for old metrics by checking the return
type of the gauge for whether it is a number and register a `NumberGauge`
instead.
A new overloaded method `void register` was added to `MetricGroup`. We
could not overload `gauge()` as this would forbid lambdas from being used
without explicit class declaration.
The signature of `register` is different than other registration methods.
Neither do they return the given metric nor do they have generic parameters.
This is done to support lambdas, as they have inherent problems with generic
methods (ambiguity). This change has no effect on our code-base as we nowhere
made use of this feature (which i think is still neat).
The first commit introduces the new types, deprecates Gauges and sets up
the compatibility layer.
The second commit updates all reporters to work against the new metric
types.
The third commit replaces all existing usages of `Gauges` to instead use
the new metric types.
## Verifying this change
Existing tests were updated.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): (no)
- The public API, i.e., is any changed class annotated with
`@Public(Evolving)`: (yes)
- The serializers: (no)
- The runtime per-record code paths (performance sensitive): (no)
- Anything that affects deployment or recovery: JobManager (and its
components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)
- The S3 file system connector: (no)
## Documentation
The documentation was not updated yet.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zentol/flink 6053
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/5501.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #5501
----
commit 7bc13f9f8dcb814fa88b746c1839511a639b234e
Author: zentol <chesnay@...>
Date: 2018-02-07T10:11:56Z
[FLINK-6053][metrics] Add new Number-/StringGauge metric types
commit 85a48c02fbd58edc822949cf9f9177c9c36661cc
Author: zentol <chesnay@...>
Date: 2018-02-07T10:34:50Z
update reporters
commit 2046d3ee79adeb0528169686c3fddf980232614d
Author: zentol <chesnay@...>
Date: 2018-02-14T14:26:04Z
replace all usages of now deprecated Gauge
----
> Gauge<T> should only take subclasses of Number, rather than everything
> ----------------------------------------------------------------------
>
> Key: FLINK-6053
> URL: https://issues.apache.org/jira/browse/FLINK-6053
> Project: Flink
> Issue Type: Improvement
> Components: Metrics
> Affects Versions: 1.2.0
> Reporter: Bowen Li
> Assignee: Chesnay Schepler
> Priority: Major
> Fix For: 1.5.0
>
>
> Currently, Flink's Gauge is defined as
> ```java
> public interface Gauge<T> extends Metric {
> T getValue();
> }
> ```
> But it doesn't make sense to have Gauge take generic types other than Number.
> And it blocks I from finishing FLINK-6013, because I cannot assume Gauge is
> only about Number. So the class should be like
> ```java
> public interface Gauge<T extends Number> extends Metric {
> T getValue();
> }
> ```
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
