You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@inlong.apache.org by GitBox <gi...@apache.org> on 2022/07/27 09:14:18 UTC
[GitHub] [inlong] liangyepianzhou opened a new pull request, #5239: [INLONG-2538] [dataproxy] Error metric params in PulsarZoneSinkContext::addSendFailMetric
liangyepianzhou opened a new pull request, #5239:
URL: https://github.com/apache/inlong/pull/5239
### Prepare a Pull Request
- Fixes https://github.com/apache/inlong/issues/5238
### Motivation
Fix error metric params in PulsarZoneSinkContext::addSendFailMetric.
### Modifications
Fix error metric params in PulsarZoneSinkContext::addSendFailMetric.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] liangyepianzhou commented on a diff in pull request #5239: [INLONG-5238][DataProxy] Error metric params in addSendFailMetric
Posted by GitBox <gi...@apache.org>.
liangyepianzhou commented on code in PR #5239:
URL: https://github.com/apache/inlong/pull/5239#discussion_r930896472
##########
inlong-dataproxy/dataproxy-source/src/main/java/org/apache/inlong/dataproxy/sink/pulsarzone/PulsarZoneSinkContext.java:
##########
@@ -192,7 +192,7 @@ public void addSendMetric(DispatchProfile currentRecord, String bid) {
*/
public void addSendFailMetric() {
Map<String, String> dimensions = new HashMap<>();
- dimensions.put(DataProxyMetricItem.KEY_CLUSTER_ID, this.getProxyClusterId());
+ dimensions.put(DataProxyMetricItem.KEY_CLUSTER_ID, this.getClusterId());
Review Comment:
The method `getClusterId()` is exist in `SinkContext`.
The `KafkaZoneSinkContext` and `TubeZoneSinkContext` are also using the the method `getClusterId()` existing in `SinkContext`.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] pocozh commented on a diff in pull request #5239: [INLONG-5238][DataProxy] Error metric params in addSendFailMetric
Posted by GitBox <gi...@apache.org>.
pocozh commented on code in PR #5239:
URL: https://github.com/apache/inlong/pull/5239#discussion_r930886272
##########
inlong-dataproxy/dataproxy-source/src/main/java/org/apache/inlong/dataproxy/sink/pulsarzone/PulsarZoneSinkContext.java:
##########
@@ -192,7 +192,7 @@ public void addSendMetric(DispatchProfile currentRecord, String bid) {
*/
public void addSendFailMetric() {
Map<String, String> dimensions = new HashMap<>();
- dimensions.put(DataProxyMetricItem.KEY_CLUSTER_ID, this.getProxyClusterId());
+ dimensions.put(DataProxyMetricItem.KEY_CLUSTER_ID, this.getClusterId());
Review Comment:
There is no getClusterId() method in PulsarZoneSinkContext.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] liangyepianzhou commented on a diff in pull request #5239: [INLONG-5238][DataProxy] Error metric params in addSendFailMetric
Posted by GitBox <gi...@apache.org>.
liangyepianzhou commented on code in PR #5239:
URL: https://github.com/apache/inlong/pull/5239#discussion_r931757571
##########
inlong-dataproxy/dataproxy-source/src/main/java/org/apache/inlong/dataproxy/sink/pulsarzone/PulsarZoneSinkContext.java:
##########
@@ -192,7 +192,7 @@ public void addSendMetric(DispatchProfile currentRecord, String bid) {
*/
public void addSendFailMetric() {
Map<String, String> dimensions = new HashMap<>();
- dimensions.put(DataProxyMetricItem.KEY_CLUSTER_ID, this.getProxyClusterId());
+ dimensions.put(DataProxyMetricItem.KEY_CLUSTER_ID, this.getClusterId());
Review Comment:
There are a lot of variables that can be unified. It might be a little weird to optimize this one alone.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] dockerzhang merged pull request #5239: [INLONG-5238][DataProxy] Error metric params in addSendFailMetric
Posted by GitBox <gi...@apache.org>.
dockerzhang merged PR #5239:
URL: https://github.com/apache/inlong/pull/5239
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] liangyepianzhou commented on a diff in pull request #5239: [INLONG-5238][DataProxy] Error metric params in addSendFailMetric
Posted by GitBox <gi...@apache.org>.
liangyepianzhou commented on code in PR #5239:
URL: https://github.com/apache/inlong/pull/5239#discussion_r930896472
##########
inlong-dataproxy/dataproxy-source/src/main/java/org/apache/inlong/dataproxy/sink/pulsarzone/PulsarZoneSinkContext.java:
##########
@@ -192,7 +192,7 @@ public void addSendMetric(DispatchProfile currentRecord, String bid) {
*/
public void addSendFailMetric() {
Map<String, String> dimensions = new HashMap<>();
- dimensions.put(DataProxyMetricItem.KEY_CLUSTER_ID, this.getProxyClusterId());
+ dimensions.put(DataProxyMetricItem.KEY_CLUSTER_ID, this.getClusterId());
Review Comment:
The method `getClusterId() ` is exist in `SinkContext`.
The `KafkaZoneSinkContext` and TubeZoneSinkContext are also using the the method `getClusterId() ` existing in `SinkContext`.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] liangyepianzhou commented on pull request #5239: [INLONG-5238][DataProxy] Error metric params in addSendFailMetric
Posted by GitBox <gi...@apache.org>.
liangyepianzhou commented on PR #5239:
URL: https://github.com/apache/inlong/pull/5239#issuecomment-1197626634
@dockerzhang @healchow Could you please help review this PR when you have time, THX.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] pocozh commented on a diff in pull request #5239: [INLONG-5238][DataProxy] Error metric params in addSendFailMetric
Posted by GitBox <gi...@apache.org>.
pocozh commented on code in PR #5239:
URL: https://github.com/apache/inlong/pull/5239#discussion_r931727471
##########
inlong-dataproxy/dataproxy-source/src/main/java/org/apache/inlong/dataproxy/sink/pulsarzone/PulsarZoneSinkContext.java:
##########
@@ -192,7 +192,7 @@ public void addSendMetric(DispatchProfile currentRecord, String bid) {
*/
public void addSendFailMetric() {
Map<String, String> dimensions = new HashMap<>();
- dimensions.put(DataProxyMetricItem.KEY_CLUSTER_ID, this.getProxyClusterId());
+ dimensions.put(DataProxyMetricItem.KEY_CLUSTER_ID, this.getClusterId());
Review Comment:
> Now, the SinkContext has not `proxyClusterId`, it only has `ClusterId`. The unify will not be done in this PR.
Yep, SinkContext has `ClusterId`, so those child classes don't need `proxyClusterId`. Maybe, could you unify in this pr?
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] liangyepianzhou commented on a diff in pull request #5239: [INLONG-5238][DataProxy] Error metric params in addSendFailMetric
Posted by GitBox <gi...@apache.org>.
liangyepianzhou commented on code in PR #5239:
URL: https://github.com/apache/inlong/pull/5239#discussion_r931724629
##########
inlong-dataproxy/dataproxy-source/src/main/java/org/apache/inlong/dataproxy/sink/pulsarzone/PulsarZoneSinkContext.java:
##########
@@ -192,7 +192,7 @@ public void addSendMetric(DispatchProfile currentRecord, String bid) {
*/
public void addSendFailMetric() {
Map<String, String> dimensions = new HashMap<>();
- dimensions.put(DataProxyMetricItem.KEY_CLUSTER_ID, this.getProxyClusterId());
+ dimensions.put(DataProxyMetricItem.KEY_CLUSTER_ID, this.getClusterId());
Review Comment:
Now, the SinkContext has not `proxyClusterId`, it only has `ClusterId`.
The unify will not be done in this PR.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] liangyepianzhou commented on a diff in pull request #5239: [INLONG-5238][DataProxy] Error metric params in addSendFailMetric
Posted by GitBox <gi...@apache.org>.
liangyepianzhou commented on code in PR #5239:
URL: https://github.com/apache/inlong/pull/5239#discussion_r931757433
##########
inlong-dataproxy/dataproxy-source/src/main/java/org/apache/inlong/dataproxy/sink/pulsarzone/PulsarZoneSinkContext.java:
##########
@@ -192,7 +192,7 @@ public void addSendMetric(DispatchProfile currentRecord, String bid) {
*/
public void addSendFailMetric() {
Map<String, String> dimensions = new HashMap<>();
- dimensions.put(DataProxyMetricItem.KEY_CLUSTER_ID, this.getProxyClusterId());
+ dimensions.put(DataProxyMetricItem.KEY_CLUSTER_ID, this.getClusterId());
Review Comment:
There are a lot of variables that can be unified. It might be a little weird to optimize this one alone.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] pocozh commented on a diff in pull request #5239: [INLONG-5238][DataProxy] Error metric params in addSendFailMetric
Posted by GitBox <gi...@apache.org>.
pocozh commented on code in PR #5239:
URL: https://github.com/apache/inlong/pull/5239#discussion_r931721734
##########
inlong-dataproxy/dataproxy-source/src/main/java/org/apache/inlong/dataproxy/sink/pulsarzone/PulsarZoneSinkContext.java:
##########
@@ -192,7 +192,7 @@ public void addSendMetric(DispatchProfile currentRecord, String bid) {
*/
public void addSendFailMetric() {
Map<String, String> dimensions = new HashMap<>();
- dimensions.put(DataProxyMetricItem.KEY_CLUSTER_ID, this.getProxyClusterId());
+ dimensions.put(DataProxyMetricItem.KEY_CLUSTER_ID, this.getClusterId());
Review Comment:
> The method `getClusterId()` is exist in `SinkContext`. The `KafkaZoneSinkContext` and `TubeZoneSinkContext` are also using the the method `getClusterId()` existing in `SinkContext`.
Please remove `proxyClusterId` in all child classes, which has been covered by SinkContext.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] liangyepianzhou commented on a diff in pull request #5239: [INLONG-5238][DataProxy] Error metric params in addSendFailMetric
Posted by GitBox <gi...@apache.org>.
liangyepianzhou commented on code in PR #5239:
URL: https://github.com/apache/inlong/pull/5239#discussion_r931758142
##########
inlong-dataproxy/dataproxy-source/src/main/java/org/apache/inlong/dataproxy/sink/pulsarzone/PulsarZoneSinkContext.java:
##########
@@ -192,7 +192,7 @@ public void addSendMetric(DispatchProfile currentRecord, String bid) {
*/
public void addSendFailMetric() {
Map<String, String> dimensions = new HashMap<>();
- dimensions.put(DataProxyMetricItem.KEY_CLUSTER_ID, this.getProxyClusterId());
+ dimensions.put(DataProxyMetricItem.KEY_CLUSTER_ID, this.getClusterId());
Review Comment:
The unify will be done in this [issue](https://github.com/apache/inlong/issues/4962).
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [inlong] dockerzhang commented on pull request #5239: [INLONG-5238][DataProxy] Error metric params in addSendFailMetric
Posted by GitBox <gi...@apache.org>.
dockerzhang commented on PR #5239:
URL: https://github.com/apache/inlong/pull/5239#issuecomment-1198205636
@pocozh @luchunliang PTAL, thanks.
--
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: commits-unsubscribe@inlong.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org