You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "mbalassi (via GitHub)" <gi...@apache.org> on 2024/04/22 09:16:20 UTC
[PR] [FLINK-35183] MinorVersion metric for tracking applications [flink-kubernetes-operator]
mbalassi opened a new pull request, #819:
URL: https://github.com/apache/flink-kubernetes-operator/pull/819
## What is the purpose of the change
* Adds new convenience metric for tracking minor version of running applications per namespace
* Changes current Flink version metric to report version as a dimension (both metrics are now dimensions)
## Verifying this change
* Updated existing test to cover this too
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35183] MinorVersion metric for tracking applications [flink-kubernetes-operator]
Posted by "mbalassi (via GitHub)" <gi...@apache.org>.
mbalassi commented on code in PR #819:
URL: https://github.com/apache/flink-kubernetes-operator/pull/819#discussion_r1584812594
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/metrics/FlinkDeploymentMetrics.java:
##########
@@ -94,6 +101,22 @@ public void onUpdate(FlinkDeployment flinkApp) {
})
.add(deploymentName);
+ // Minor version computed from the above
+ var subVersions = flinkVersion.split("\\.");
+ String minorVersion = MALFORMED_MINOR_VERSION;
Review Comment:
Fair, I am open to simply using `UNKNOWN`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35183] MinorVersion metric for tracking applications [flink-kubernetes-operator]
Posted by "mbalassi (via GitHub)" <gi...@apache.org>.
mbalassi merged PR #819:
URL: https://github.com/apache/flink-kubernetes-operator/pull/819
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35183] MinorVersion metric for tracking applications [flink-kubernetes-operator]
Posted by "gaborgsomogyi (via GitHub)" <gi...@apache.org>.
gaborgsomogyi commented on code in PR #819:
URL: https://github.com/apache/flink-kubernetes-operator/pull/819#discussion_r1584790463
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/metrics/FlinkDeploymentMetrics.java:
##########
@@ -165,13 +194,21 @@ private void initNamespaceStatusCounts(String ns) {
private void initFlinkVersions(String ns, String flinkVersion) {
parentMetricGroup
.createResourceNamespaceGroup(configuration, FlinkDeployment.class, ns)
- .addGroup(FLINK_VERSION_GROUP_NAME)
- .addGroup(flinkVersion)
+ .addGroup(FLINK_VERSION_GROUP_NAME, flinkVersion)
Review Comment:
This is not related, just merging the 2 statements to one, right?
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/metrics/FlinkDeploymentMetrics.java:
##########
@@ -94,6 +101,22 @@ public void onUpdate(FlinkDeployment flinkApp) {
})
.add(deploymentName);
+ // Minor version computed from the above
+ var subVersions = flinkVersion.split("\\.");
+ String minorVersion = MALFORMED_MINOR_VERSION;
Review Comment:
Not sure why not use `UNKNOWN` just like the other case but I'm fine with that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35183] MinorVersion metric for tracking applications [flink-kubernetes-operator]
Posted by "mbalassi (via GitHub)" <gi...@apache.org>.
mbalassi commented on code in PR #819:
URL: https://github.com/apache/flink-kubernetes-operator/pull/819#discussion_r1584810870
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/metrics/FlinkDeploymentMetrics.java:
##########
@@ -165,13 +194,21 @@ private void initNamespaceStatusCounts(String ns) {
private void initFlinkVersions(String ns, String flinkVersion) {
parentMetricGroup
.createResourceNamespaceGroup(configuration, FlinkDeployment.class, ns)
- .addGroup(FLINK_VERSION_GROUP_NAME)
- .addGroup(flinkVersion)
+ .addGroup(FLINK_VERSION_GROUP_NAME, flinkVersion)
Review Comment:
Good question. This is actually semantically different. We can consider a previous behavior a bug, this approach works well with modern metrics systems like Prometheus/Graphana.
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/metrics/FlinkDeploymentMetrics.java:
##########
@@ -165,13 +194,21 @@ private void initNamespaceStatusCounts(String ns) {
private void initFlinkVersions(String ns, String flinkVersion) {
parentMetricGroup
.createResourceNamespaceGroup(configuration, FlinkDeployment.class, ns)
- .addGroup(FLINK_VERSION_GROUP_NAME)
- .addGroup(flinkVersion)
+ .addGroup(FLINK_VERSION_GROUP_NAME, flinkVersion)
Review Comment:
Good question. This is actually semantically different. We can consider the previous behavior a bug, this approach works well with modern metrics systems like Prometheus/Graphana.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org