You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by zentol <gi...@git.apache.org> on 2016/06/20 09:19:28 UTC
[GitHub] flink pull request #2134: [FLINK-4093] Expose metric interfaces
GitHub user zentol opened a pull request:
https://github.com/apache/flink/pull/2134
[FLINK-4093] Expose metric interfaces
This PR allows users to create their own metrics and register them with Flink.
To that end, Counter was refactored into a SimpleCounter class implementing a new Counter interface. Gauge was changed to an interface as well.
Finally, the internal addMetric() method is now part of the MetricGroup interface.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zentol/flink metrics_interfaces
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/2134.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 #2134
----
commit aeefb8d681cbcea5d0a855a60b34848ea28e4bf5
Author: zentol <ch...@apache.org>
Date: 2016-06-20T09:13:53Z
[FLINK-4093] Expose metric interfaces
----
---
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.
---
[GitHub] flink issue #2134: [FLINK-4093] Expose metric interfaces
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/2134
The changes look good to me. +1 for merging. Good work @zentol. I only had a question concerning the use of generic parameters which is not really necessary, imo.
---
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.
---
[GitHub] flink pull request #2134: [FLINK-4093] Expose metric interfaces
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/2134#discussion_r68256978
--- Diff: flink-core/src/main/java/org/apache/flink/metrics/groups/AbstractMetricGroup.java ---
@@ -146,7 +147,16 @@ public Counter counter(int name) {
@Override
public Counter counter(String name) {
- Counter counter = new Counter();
+ return counter(name, new SimpleCounter());
+ }
+
+ @Override
+ public <C extends Counter> C counter(int name, C counter) {
--- End diff --
in other words, it allows the instantiation, registration and assignment to a variable or field in a single line without loosing the subclass type.
---
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.
---
[GitHub] flink pull request #2134: [FLINK-4093] Expose metric interfaces
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/2134#discussion_r68265180
--- Diff: flink-core/src/main/java/org/apache/flink/metrics/groups/AbstractMetricGroup.java ---
@@ -146,7 +147,16 @@ public Counter counter(int name) {
@Override
public Counter counter(String name) {
- Counter counter = new Counter();
+ return counter(name, new SimpleCounter());
+ }
+
+ @Override
+ public <C extends Counter> C counter(int name, C counter) {
--- End diff --
Ah true. Good idea :-) Maybe we should change it for the `gauge` and `histogram` functions as well.
---
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.
---
[GitHub] flink issue #2134: [FLINK-4093] Expose metric interfaces
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/2134
merging
---
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.
---
[GitHub] flink pull request #2134: [FLINK-4093] Expose metric interfaces
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/2134#discussion_r68268650
--- Diff: flink-core/src/main/java/org/apache/flink/metrics/groups/AbstractMetricGroup.java ---
@@ -146,7 +147,16 @@ public Counter counter(int name) {
@Override
public Counter counter(String name) {
- Counter counter = new Counter();
+ return counter(name, new SimpleCounter());
+ }
+
+ @Override
+ public <C extends Counter> C counter(int name, C counter) {
--- End diff --
we should change it for the other methods as well.
---
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.
---
[GitHub] flink pull request #2134: [FLINK-4093] Expose metric interfaces
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/2134#discussion_r68255328
--- Diff: flink-core/src/main/java/org/apache/flink/metrics/groups/AbstractMetricGroup.java ---
@@ -146,7 +147,16 @@ public Counter counter(int name) {
@Override
public Counter counter(String name) {
- Counter counter = new Counter();
+ return counter(name, new SimpleCounter());
+ }
+
+ @Override
+ public <C extends Counter> C counter(int name, C counter) {
--- End diff --
users can write ```MyCounter c = metricGroup.addCounter("myC", new MyCounter());```
---
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.
---
[GitHub] flink pull request #2134: [FLINK-4093] Expose metric interfaces
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/2134
---
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.
---
[GitHub] flink pull request #2134: [FLINK-4093] Expose metric interfaces
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:
https://github.com/apache/flink/pull/2134#discussion_r68248549
--- Diff: flink-core/src/main/java/org/apache/flink/metrics/groups/AbstractMetricGroup.java ---
@@ -146,7 +147,16 @@ public Counter counter(int name) {
@Override
public Counter counter(String name) {
- Counter counter = new Counter();
+ return counter(name, new SimpleCounter());
+ }
+
+ @Override
+ public <C extends Counter> C counter(int name, C counter) {
--- End diff --
What's the advantage of introducing the bounded generic parameter over simply having a `Counter` parameter?
---
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.
---