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/11/30 15:46:17 UTC
[GitHub] [inlong] wangpeix opened a new pull request, #6692: [INLONG-6675][Sort] Sort does not report metrics because Flink Sql missing the metrics.audit.proxy.hosts parameter
wangpeix opened a new pull request, #6692:
URL: https://github.com/apache/inlong/pull/6692
### Prepare a Pull Request
- Title: [INLONG-6675][Sort] Sort does not report metrics because Flink Sql missing the `metrics.audit.proxy.hosts` parameter
- Fixes #6675
### Motivation
Fix `metrics.audit.proxy.hosts` parameter missing problem.
### Modifications
Pass the `metrics.audit.proxy.hosts` parameter to `org.apache.inlong.sort.Entrance`
--
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] wangpeix commented on pull request #6692: [INLONG-6675][Sort] Sort does not report metrics because Flink Sql missing the metrics.audit.proxy.hosts parameter
Posted by GitBox <gi...@apache.org>.
wangpeix commented on PR #6692:
URL: https://github.com/apache/inlong/pull/6692#issuecomment-1332407548
I created a new pr. Please help me review the code, thank you. @gong
--
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] healchow merged pull request #6692: [INLONG-6675][Sort] Add metrics.audit.proxy.hosts option to support report metrics
Posted by GitBox <gi...@apache.org>.
healchow merged PR #6692:
URL: https://github.com/apache/inlong/pull/6692
--
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] gong commented on a diff in pull request #6692: [INLONG-6675][Sort] Add metrics.audit.proxy.hosts option to support report metrics
Posted by GitBox <gi...@apache.org>.
gong commented on code in PR #6692:
URL: https://github.com/apache/inlong/pull/6692#discussion_r1036647858
##########
inlong-sort/sort-core/src/main/java/org/apache/inlong/sort/Entrance.java:
##########
@@ -58,6 +58,12 @@ public static void main(String[] args) throws Exception {
Parser parser;
if (StringUtils.isEmpty(sqlFile)) {
final GroupInfo groupInfo = getGroupInfoFromFile(config.getString(Constants.GROUP_INFO_FILE));
+ boolean hasAuditProxyHosts =
+ StringUtils.isNotEmpty(groupInfo.getProperties().get(Constants.METRICS_AUDIT_PROXY_HOSTS.key()));
+ if (!hasAuditProxyHosts) {
+ groupInfo.getProperties().put(Constants.METRICS_AUDIT_PROXY_HOSTS.key(),
+ config.getString(Constants.METRICS_AUDIT_PROXY_HOSTS));
+ }
Review Comment:
> The value may be an empty string. It may also need to be handled.
maybe judge config.getString(Constants.METRICS_AUDIT_PROXY_HOSTS) not empty
--
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] gong commented on a diff in pull request #6692: [INLONG-6675][Sort] Add metrics.audit.proxy.hosts option to support report metrics
Posted by GitBox <gi...@apache.org>.
gong commented on code in PR #6692:
URL: https://github.com/apache/inlong/pull/6692#discussion_r1036647858
##########
inlong-sort/sort-core/src/main/java/org/apache/inlong/sort/Entrance.java:
##########
@@ -58,6 +58,12 @@ public static void main(String[] args) throws Exception {
Parser parser;
if (StringUtils.isEmpty(sqlFile)) {
final GroupInfo groupInfo = getGroupInfoFromFile(config.getString(Constants.GROUP_INFO_FILE));
+ boolean hasAuditProxyHosts =
+ StringUtils.isNotEmpty(groupInfo.getProperties().get(Constants.METRICS_AUDIT_PROXY_HOSTS.key()));
+ if (!hasAuditProxyHosts) {
+ groupInfo.getProperties().put(Constants.METRICS_AUDIT_PROXY_HOSTS.key(),
+ config.getString(Constants.METRICS_AUDIT_PROXY_HOSTS));
+ }
Review Comment:
> The value may be an empty string. It may also need to be handled.
@wangpeix maybe judge config.getString(Constants.METRICS_AUDIT_PROXY_HOSTS) not empty
--
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] wangpeix commented on a diff in pull request #6692: [INLONG-6675][Sort] Add metrics.audit.proxy.hosts option to support report metrics
Posted by GitBox <gi...@apache.org>.
wangpeix commented on code in PR #6692:
URL: https://github.com/apache/inlong/pull/6692#discussion_r1036642524
##########
inlong-sort/sort-core/src/main/java/org/apache/inlong/sort/Entrance.java:
##########
@@ -58,6 +58,12 @@ public static void main(String[] args) throws Exception {
Parser parser;
if (StringUtils.isEmpty(sqlFile)) {
final GroupInfo groupInfo = getGroupInfoFromFile(config.getString(Constants.GROUP_INFO_FILE));
+ boolean hasAuditProxyHosts =
+ StringUtils.isNotEmpty(groupInfo.getProperties().get(Constants.METRICS_AUDIT_PROXY_HOSTS.key()));
+ if (!hasAuditProxyHosts) {
+ groupInfo.getProperties().put(Constants.METRICS_AUDIT_PROXY_HOSTS.key(),
+ config.getString(Constants.METRICS_AUDIT_PROXY_HOSTS));
+ }
Review Comment:
The value may be an empty string. It may also need to be handled.
--
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 a diff in pull request #6692: [INLONG-6675][Sort] Add metrics.audit.proxy.hosts option to support report metrics
Posted by GitBox <gi...@apache.org>.
dockerzhang commented on code in PR #6692:
URL: https://github.com/apache/inlong/pull/6692#discussion_r1036777413
##########
inlong-manager/manager-plugins/src/main/java/org/apache/inlong/manager/plugin/flink/FlinkService.java:
##########
@@ -285,6 +285,8 @@ private String[] genProgramArgsV2(FlinkInfo flinkInfo, FlinkConfig flinkConfig)
list.add(flinkInfo.getLocalConfPath());
list.add("-checkpoint.interval");
list.add("60000");
+ list.add("-metrics.audit.proxy.hosts");
+ list.add(flinkConfig.getAuditProxyHosts());
return list.toArray(new String[0]);
Review Comment:
please remove the `genProgramArgs` method and rename `genProgramArgsV2` to `genProgramArgs`, I think it will not be used again.
--
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] healchow commented on a diff in pull request #6692: [INLONG-6675][Sort] Add metrics.audit.proxy.hosts option to support report metrics
Posted by GitBox <gi...@apache.org>.
healchow commented on code in PR #6692:
URL: https://github.com/apache/inlong/pull/6692#discussion_r1036633101
##########
inlong-sort/sort-core/src/main/java/org/apache/inlong/sort/Entrance.java:
##########
@@ -58,6 +58,12 @@ public static void main(String[] args) throws Exception {
Parser parser;
if (StringUtils.isEmpty(sqlFile)) {
final GroupInfo groupInfo = getGroupInfoFromFile(config.getString(Constants.GROUP_INFO_FILE));
+ boolean hasAuditProxyHosts =
+ StringUtils.isNotEmpty(groupInfo.getProperties().get(Constants.METRICS_AUDIT_PROXY_HOSTS.key()));
+ if (!hasAuditProxyHosts) {
+ groupInfo.getProperties().put(Constants.METRICS_AUDIT_PROXY_HOSTS.key(),
+ config.getString(Constants.METRICS_AUDIT_PROXY_HOSTS));
+ }
Review Comment:
Is the `putIfAbsent` method better?
```suggestion
groupInfo.getProperties().putIfAbsent(Constants.METRICS_AUDIT_PROXY_HOSTS.key(),
config.getString(Constants.METRICS_AUDIT_PROXY_HOSTS));
```
--
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