You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "lokeshj1703 (via GitHub)" <gi...@apache.org> on 2023/02/24 11:35:01 UTC
[GitHub] [hudi] lokeshj1703 opened a new pull request, #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
lokeshj1703 opened a new pull request, #8041:
URL: https://github.com/apache/hudi/pull/8041
### Change Logs
The PR aims to add support for multiple metric reporters within a Metric Registry. Further it also adds labels to metrics.
### Impact
NA
### Risk level (write none, low medium or high below)
low
### Documentation Update
_Describe any necessary documentation update if there is any new feature, config, or user-facing change_
- _The config description must be updated if new configs are added or the default value of the configs are changed_
- _Any new feature or user-facing change requires updating the Hudi website. Please create a Jira ticket, attach the
ticket number here and follow the [instruction](https://hudi.apache.org/contribute/developer-setup#website) to make
changes to the website._
### Contributor's checklist
- [ ] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute)
- [ ] Change Logs and Impact were stated clearly
- [ ] Adequate tests were added if applicable
- [ ] CI passed
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8041:
URL: https://github.com/apache/hudi/pull/8041#issuecomment-1450183567
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15386",
"triggerID" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"triggerType" : "PUSH"
}, {
"hash" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15475",
"triggerID" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"triggerType" : "PUSH"
}, {
"hash" : "a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15504",
"triggerID" : "a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530",
"triggerType" : "PUSH"
}, {
"hash" : "e453f39e2e3f48eabcb1470922573b07d4ed486d",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15507",
"triggerID" : "e453f39e2e3f48eabcb1470922573b07d4ed486d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* e453f39e2e3f48eabcb1470922573b07d4ed486d Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15507)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8041:
URL: https://github.com/apache/hudi/pull/8041#issuecomment-1449431904
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15386",
"triggerID" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"triggerType" : "PUSH"
}, {
"hash" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15475",
"triggerID" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"triggerType" : "PUSH"
}, {
"hash" : "a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15475)
* a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8041:
URL: https://github.com/apache/hudi/pull/8041#issuecomment-1449484598
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15386",
"triggerID" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"triggerType" : "PUSH"
}, {
"hash" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15475",
"triggerID" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"triggerType" : "PUSH"
}, {
"hash" : "a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530",
"status" : "CANCELED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15504",
"triggerID" : "a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530",
"triggerType" : "PUSH"
}, {
"hash" : "e453f39e2e3f48eabcb1470922573b07d4ed486d",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15507",
"triggerID" : "e453f39e2e3f48eabcb1470922573b07d4ed486d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530 Azure: [CANCELED](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15504)
* e453f39e2e3f48eabcb1470922573b07d4ed486d Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15507)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] lokeshj1703 commented on a diff in pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "lokeshj1703 (via GitHub)" <gi...@apache.org>.
lokeshj1703 commented on code in PR #8041:
URL: https://github.com/apache/hudi/pull/8041#discussion_r1119787547
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/metrics/HoodieMetricsConfig.java:
##########
@@ -95,6 +95,12 @@ public class HoodieMetricsConfig extends HoodieConfig {
.sinceVersion("0.13.0")
.withDocumentation("Enable metrics for locking infra. Useful when operating in multiwriter mode");
+ public static final ConfigProperty<String> METRICS_REPORTER_FILE_BASED_CONFIGS_PATH = ConfigProperty
+ .key(METRIC_PREFIX + ".configs.properties")
+ .defaultValue("")
+ .sinceVersion("0.12.2")
Review Comment:
Addressed
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/datadog/DatadogReporter.java:
##########
@@ -84,18 +87,22 @@ public void report(
SortedMap<String, Histogram> histograms,
SortedMap<String, Meter> meters,
SortedMap<String, Timer> timers) {
- final long now = clock.getTime() / 1000;
- final PayloadBuilder builder = new PayloadBuilder();
- builder.withMetricType(MetricType.gauge);
- gauges.forEach((metricName, metric) -> {
- builder.addGauge(prefix(metricName), now, (long) metric.getValue());
+ Map<List<String>, List<Pair<String, List<String>>>> labelsPair = gauges.keySet().stream().map(MetricUtils::getLabelsAndMetricList)
+ .collect(Collectors.groupingBy(Pair::getValue));
+ labelsPair.entrySet().forEach(labelsKeyValue -> {
+ final long now = clock.getTime() / 1000;
+ final PayloadBuilder builder = new PayloadBuilder();
+ builder.withMetricType(MetricType.gauge);
+ gauges.forEach((metricName, metric) -> {
+ builder.addGauge(prefix(MetricUtils.getMetricAndLabels(metricName).getKey()), now, (long) metric.getValue());
+ });
Review Comment:
Addressed
##########
hudi-client/hudi-client-common/src/test/resources/datadog.properties:
##########
@@ -0,0 +1,25 @@
+###
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+###
+hoodie.base.path="/tmp/base_path"
+hoodie.metrics.datadog.api.key=f4e1ca9f6e3f16ff5c0800dddfda6179
+hoodie.metrics.datadog.report.period.seconds=3600
+hoodie.metrics.datadog.job.name=test_job
+hoodie.metrics.datadog.metric.host=app.us5.datadoghq
+hoodie.metrics.datadog.metric.prefix=oh
+hoodie.metrics.datadog.api.site=US
+hoodie.metrics.reporter.type=DATADOG
Review Comment:
Addressed
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8041:
URL: https://github.com/apache/hudi/pull/8041#issuecomment-1443609262
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15386",
"triggerID" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2068af21c85e51254b849562f38cf6a2918f9bef Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15386)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8041:
URL: https://github.com/apache/hudi/pull/8041#issuecomment-1447879262
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15386",
"triggerID" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"triggerType" : "PUSH"
}, {
"hash" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15475",
"triggerID" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2068af21c85e51254b849562f38cf6a2918f9bef Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15386)
* 9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15475)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] codope commented on a diff in pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "codope (via GitHub)" <gi...@apache.org>.
codope commented on code in PR #8041:
URL: https://github.com/apache/hudi/pull/8041#discussion_r1122575366
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/Metrics.java:
##########
@@ -40,27 +48,51 @@ public class Metrics {
private static Metrics instance = null;
private final MetricRegistry registry;
- private MetricsReporter reporter;
+ private final List<MetricsReporter> reporters;
private final String commonMetricPrefix;
private Metrics(HoodieWriteConfig metricConfig) {
registry = new MetricRegistry();
commonMetricPrefix = metricConfig.getMetricReporterMetricsNamePrefix();
- reporter = MetricsReporterFactory.createReporter(metricConfig, registry);
- if (reporter == null) {
- throw new RuntimeException("Cannot initialize Reporter.");
+ reporters = new ArrayList<>();
+ MetricsReporter defaultReporter = MetricsReporterFactory.createReporter(metricConfig, registry);
+ if (defaultReporter != null) {
+ reporters.add(defaultReporter);
}
- reporter.start();
+ reporters.addAll(addAdditionalMetricsExporters(metricConfig));
+ if (reporters.size() == 0) {
+ throw new RuntimeException("Cannot initialize Reporters.");
+ }
+ reporters.forEach(MetricsReporter::start);
Runtime.getRuntime().addShutdownHook(new Thread(Metrics::shutdown));
}
+ private List<MetricsReporter> addAdditionalMetricsExporters(HoodieWriteConfig metricConfig) {
+ List<MetricsReporter> reporterList = new ArrayList<>();
+ if (!StringUtils.isNullOrEmpty(metricConfig.getMetricReporterFileBasedConfigs())) {
+ List<String> propPathList = StringUtils.split(metricConfig.getMetricReporterFileBasedConfigs(), ",");
+ try (FileSystem fs = FSUtils.getFs(propPathList.get(0), new Configuration())) {
+ for (String propPath: propPathList) {
+ HoodieWriteConfig secondarySourceConfig = HoodieWriteConfig.newBuilder().fromInputStream(
+ fs.open(new Path(propPath))).withPath(metricConfig.getBasePath()).build();
+ reporterList.add(MetricsReporterFactory.createReporter(secondarySourceConfig, registry));
+ }
+ } catch (IOException e) {
+ LOG.error("Failed to add MetricsExporters", e);
+ throw new HoodieException("failed to MetricsExporters", e);
Review Comment:
sounds good. even if we did throw an exception it should be handled in the caller.. continuous ingestion takes precedence over metrics.
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] codope merged pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "codope (via GitHub)" <gi...@apache.org>.
codope merged PR #8041:
URL: https://github.com/apache/hudi/pull/8041
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] nsivabalan commented on pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on PR #8041:
URL: https://github.com/apache/hudi/pull/8041#issuecomment-1453706032
CI is green
<img width="1195" alt="image" src="https://user-images.githubusercontent.com/513218/222760967-36a1b0e9-fb75-46cd-8e15-7ed373cc5b32.png">
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8041:
URL: https://github.com/apache/hudi/pull/8041#issuecomment-1453058343
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15386",
"triggerID" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"triggerType" : "PUSH"
}, {
"hash" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15475",
"triggerID" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"triggerType" : "PUSH"
}, {
"hash" : "a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15504",
"triggerID" : "a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530",
"triggerType" : "PUSH"
}, {
"hash" : "e453f39e2e3f48eabcb1470922573b07d4ed486d",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15507",
"triggerID" : "e453f39e2e3f48eabcb1470922573b07d4ed486d",
"triggerType" : "PUSH"
}, {
"hash" : "6995a948f49fadbec59748c4728a2beef6072b36",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "6995a948f49fadbec59748c4728a2beef6072b36",
"triggerType" : "PUSH"
}, {
"hash" : "d04edd5df0b50036aa9ef5175fa3f46f0b0f4c6f",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15533",
"triggerID" : "d04edd5df0b50036aa9ef5175fa3f46f0b0f4c6f",
"triggerType" : "PUSH"
}, {
"hash" : "b7c04d848ba10091669905ad1ebf02a528f80ace",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "b7c04d848ba10091669905ad1ebf02a528f80ace",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 6995a948f49fadbec59748c4728a2beef6072b36 UNKNOWN
* d04edd5df0b50036aa9ef5175fa3f46f0b0f4c6f Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15533)
* b7c04d848ba10091669905ad1ebf02a528f80ace UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8041:
URL: https://github.com/apache/hudi/pull/8041#issuecomment-1443600686
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2068af21c85e51254b849562f38cf6a2918f9bef UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] nsivabalan commented on pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on PR #8041:
URL: https://github.com/apache/hudi/pull/8041#issuecomment-1447530147
can you take a look at https://github.com/apache/hudi/pull/7934 and ensure there are no overlaps or do you need any coordination.
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] codope commented on a diff in pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "codope (via GitHub)" <gi...@apache.org>.
codope commented on code in PR #8041:
URL: https://github.com/apache/hudi/pull/8041#discussion_r1118356753
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/Metrics.java:
##########
@@ -40,27 +48,51 @@ public class Metrics {
private static Metrics instance = null;
private final MetricRegistry registry;
- private MetricsReporter reporter;
+ private final List<MetricsReporter> reporters;
private final String commonMetricPrefix;
private Metrics(HoodieWriteConfig metricConfig) {
registry = new MetricRegistry();
commonMetricPrefix = metricConfig.getMetricReporterMetricsNamePrefix();
- reporter = MetricsReporterFactory.createReporter(metricConfig, registry);
- if (reporter == null) {
- throw new RuntimeException("Cannot initialize Reporter.");
+ reporters = new ArrayList<>();
+ MetricsReporter defaultReporter = MetricsReporterFactory.createReporter(metricConfig, registry);
+ if (defaultReporter != null) {
+ reporters.add(defaultReporter);
}
- reporter.start();
+ reporters.addAll(addAdditionalMetricsExporters(metricConfig));
+ if (reporters.size() == 0) {
+ throw new RuntimeException("Cannot initialize Reporters.");
+ }
+ reporters.forEach(MetricsReporter::start);
Runtime.getRuntime().addShutdownHook(new Thread(Metrics::shutdown));
}
+ private List<MetricsReporter> addAdditionalMetricsExporters(HoodieWriteConfig metricConfig) {
+ List<MetricsReporter> reporterList = new ArrayList<>();
+ if (!StringUtils.isNullOrEmpty(metricConfig.getMetricReporterFileBasedConfigs())) {
Review Comment:
move this check to constructor. it's better to not add empty list
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/Metrics.java:
##########
@@ -40,27 +48,51 @@ public class Metrics {
private static Metrics instance = null;
private final MetricRegistry registry;
- private MetricsReporter reporter;
+ private final List<MetricsReporter> reporters;
private final String commonMetricPrefix;
private Metrics(HoodieWriteConfig metricConfig) {
registry = new MetricRegistry();
commonMetricPrefix = metricConfig.getMetricReporterMetricsNamePrefix();
- reporter = MetricsReporterFactory.createReporter(metricConfig, registry);
- if (reporter == null) {
- throw new RuntimeException("Cannot initialize Reporter.");
+ reporters = new ArrayList<>();
+ MetricsReporter defaultReporter = MetricsReporterFactory.createReporter(metricConfig, registry);
+ if (defaultReporter != null) {
+ reporters.add(defaultReporter);
}
- reporter.start();
+ reporters.addAll(addAdditionalMetricsExporters(metricConfig));
+ if (reporters.size() == 0) {
+ throw new RuntimeException("Cannot initialize Reporters.");
+ }
+ reporters.forEach(MetricsReporter::start);
Runtime.getRuntime().addShutdownHook(new Thread(Metrics::shutdown));
}
+ private List<MetricsReporter> addAdditionalMetricsExporters(HoodieWriteConfig metricConfig) {
+ List<MetricsReporter> reporterList = new ArrayList<>();
+ if (!StringUtils.isNullOrEmpty(metricConfig.getMetricReporterFileBasedConfigs())) {
+ List<String> propPathList = StringUtils.split(metricConfig.getMetricReporterFileBasedConfigs(), ",");
+ try (FileSystem fs = FSUtils.getFs(propPathList.get(0), new Configuration())) {
+ for (String propPath: propPathList) {
+ HoodieWriteConfig secondarySourceConfig = HoodieWriteConfig.newBuilder().fromInputStream(
+ fs.open(new Path(propPath))).withPath(metricConfig.getBasePath()).build();
+ reporterList.add(MetricsReporterFactory.createReporter(secondarySourceConfig, registry));
+ }
+ } catch (IOException e) {
+ LOG.error("Failed to add MetricsExporters", e);
Review Comment:
let's remove this redundant log? the exception below already includes it.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/MetricUtils.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.metrics;
+
+import org.apache.hudi.common.util.collection.Pair;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+public class MetricUtils {
+
+ private static Pair<String, String> splitToPair(String label) {
+ String[] keyValuess = label.split(":");
Review Comment:
split can return empty string.. does it make sense to have empty key and value in that case?
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/MetricUtils.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.metrics;
+
+import org.apache.hudi.common.util.collection.Pair;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+public class MetricUtils {
+
+ private static Pair<String, String> splitToPair(String label) {
+ String[] keyValuess = label.split(":");
Review Comment:
```suggestion
String[] keyValues = label.split(":");
```
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/datadog/DatadogReporter.java:
##########
@@ -84,18 +87,22 @@ public void report(
SortedMap<String, Histogram> histograms,
SortedMap<String, Meter> meters,
SortedMap<String, Timer> timers) {
- final long now = clock.getTime() / 1000;
- final PayloadBuilder builder = new PayloadBuilder();
- builder.withMetricType(MetricType.gauge);
- gauges.forEach((metricName, metric) -> {
- builder.addGauge(prefix(metricName), now, (long) metric.getValue());
+ Map<List<String>, List<Pair<String, List<String>>>> labelsPair = gauges.keySet().stream().map(MetricUtils::getLabelsAndMetricList)
+ .collect(Collectors.groupingBy(Pair::getValue));
+ labelsPair.entrySet().forEach(labelsKeyValue -> {
+ final long now = clock.getTime() / 1000;
+ final PayloadBuilder builder = new PayloadBuilder();
+ builder.withMetricType(MetricType.gauge);
+ gauges.forEach((metricName, metric) -> {
+ builder.addGauge(prefix(MetricUtils.getMetricAndLabels(metricName).getKey()), now, (long) metric.getValue());
+ });
Review Comment:
```suggestion
gauges.forEach((metricName, metric) -> builder.addGauge(prefix(MetricUtils.getMetricAndLabels(metricName).getKey()), now, (long) metric.getValue()));
```
##########
hudi-client/hudi-client-common/src/test/resources/datadog.properties:
##########
@@ -0,0 +1,25 @@
+###
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+###
+hoodie.base.path="/tmp/base_path"
+hoodie.metrics.datadog.api.key=f4e1ca9f6e3f16ff5c0800dddfda6179
+hoodie.metrics.datadog.report.period.seconds=3600
+hoodie.metrics.datadog.job.name=test_job
+hoodie.metrics.datadog.metric.host=app.us5.datadoghq
+hoodie.metrics.datadog.metric.prefix=oh
+hoodie.metrics.datadog.api.site=US
+hoodie.metrics.reporter.type=DATADOG
Review Comment:
nit: add a new line in both the properties files.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/Metrics.java:
##########
@@ -118,6 +150,11 @@ public static void registerGauges(Map<String, Long> metricsMap, Option<String> p
metricsMap.forEach((k, v) -> registerGauge(metricPrefix + k, v));
}
+ public static void registerGauges(Map<String, Long> metricsMap, Option<String> prefix, Map<String, String> labels) {
Review Comment:
There are two `registerGauges` and this one is not getting used anywhere other than the tests. Exposing a public method just for tests does is not a good practice. Is there a better way to refactor?
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/Metrics.java:
##########
@@ -40,27 +48,51 @@ public class Metrics {
private static Metrics instance = null;
private final MetricRegistry registry;
- private MetricsReporter reporter;
+ private final List<MetricsReporter> reporters;
private final String commonMetricPrefix;
private Metrics(HoodieWriteConfig metricConfig) {
registry = new MetricRegistry();
commonMetricPrefix = metricConfig.getMetricReporterMetricsNamePrefix();
- reporter = MetricsReporterFactory.createReporter(metricConfig, registry);
- if (reporter == null) {
- throw new RuntimeException("Cannot initialize Reporter.");
+ reporters = new ArrayList<>();
+ MetricsReporter defaultReporter = MetricsReporterFactory.createReporter(metricConfig, registry);
+ if (defaultReporter != null) {
+ reporters.add(defaultReporter);
}
- reporter.start();
+ reporters.addAll(addAdditionalMetricsExporters(metricConfig));
+ if (reporters.size() == 0) {
+ throw new RuntimeException("Cannot initialize Reporters.");
+ }
+ reporters.forEach(MetricsReporter::start);
Runtime.getRuntime().addShutdownHook(new Thread(Metrics::shutdown));
}
+ private List<MetricsReporter> addAdditionalMetricsExporters(HoodieWriteConfig metricConfig) {
+ List<MetricsReporter> reporterList = new ArrayList<>();
+ if (!StringUtils.isNullOrEmpty(metricConfig.getMetricReporterFileBasedConfigs())) {
Review Comment:
There is also a method `StringUtils#nonEmpty`
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/metrics/HoodieMetricsConfig.java:
##########
@@ -95,6 +95,12 @@ public class HoodieMetricsConfig extends HoodieConfig {
.sinceVersion("0.13.0")
.withDocumentation("Enable metrics for locking infra. Useful when operating in multiwriter mode");
+ public static final ConfigProperty<String> METRICS_REPORTER_FILE_BASED_CONFIGS_PATH = ConfigProperty
+ .key(METRIC_PREFIX + ".configs.properties")
+ .defaultValue("")
+ .sinceVersion("0.12.2")
Review Comment:
this should be 0.14.0 right?
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/Metrics.java:
##########
@@ -40,27 +48,51 @@ public class Metrics {
private static Metrics instance = null;
private final MetricRegistry registry;
- private MetricsReporter reporter;
+ private final List<MetricsReporter> reporters;
private final String commonMetricPrefix;
private Metrics(HoodieWriteConfig metricConfig) {
registry = new MetricRegistry();
commonMetricPrefix = metricConfig.getMetricReporterMetricsNamePrefix();
- reporter = MetricsReporterFactory.createReporter(metricConfig, registry);
- if (reporter == null) {
- throw new RuntimeException("Cannot initialize Reporter.");
+ reporters = new ArrayList<>();
+ MetricsReporter defaultReporter = MetricsReporterFactory.createReporter(metricConfig, registry);
+ if (defaultReporter != null) {
+ reporters.add(defaultReporter);
}
- reporter.start();
+ reporters.addAll(addAdditionalMetricsExporters(metricConfig));
+ if (reporters.size() == 0) {
+ throw new RuntimeException("Cannot initialize Reporters.");
+ }
+ reporters.forEach(MetricsReporter::start);
Runtime.getRuntime().addShutdownHook(new Thread(Metrics::shutdown));
}
+ private List<MetricsReporter> addAdditionalMetricsExporters(HoodieWriteConfig metricConfig) {
+ List<MetricsReporter> reporterList = new ArrayList<>();
+ if (!StringUtils.isNullOrEmpty(metricConfig.getMetricReporterFileBasedConfigs())) {
+ List<String> propPathList = StringUtils.split(metricConfig.getMetricReporterFileBasedConfigs(), ",");
+ try (FileSystem fs = FSUtils.getFs(propPathList.get(0), new Configuration())) {
+ for (String propPath: propPathList) {
+ HoodieWriteConfig secondarySourceConfig = HoodieWriteConfig.newBuilder().fromInputStream(
+ fs.open(new Path(propPath))).withPath(metricConfig.getBasePath()).build();
+ reporterList.add(MetricsReporterFactory.createReporter(secondarySourceConfig, registry));
+ }
+ } catch (IOException e) {
+ LOG.error("Failed to add MetricsExporters", e);
+ throw new HoodieException("failed to MetricsExporters", e);
Review Comment:
i'm wondering whether we should throw an exception of just log a warning. I think failing the deltastreamer because one of the metrics exporters could not be registered is a bit aggressive. Typically, upstream systems will have some monitor over missing metrics. So, can we let the deltasync continue with just a warning?
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/MetricsReporterFactory.java:
##########
@@ -56,6 +56,9 @@ public static MetricsReporter createReporter(HoodieWriteConfig config, MetricReg
MetricsReporterType type = config.getMetricsReporterType();
MetricsReporter reporter = null;
+ if (type == null) {
+ return reporter;
Review Comment:
this can return `null` reporter and we have to be extra careful at the caller, e.g. when it is called to add to the list in `addAdditionalMetricsExporters` it will throw and exception. It's better to handle null here itself.
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8041:
URL: https://github.com/apache/hudi/pull/8041#issuecomment-1453064225
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15386",
"triggerID" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"triggerType" : "PUSH"
}, {
"hash" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15475",
"triggerID" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"triggerType" : "PUSH"
}, {
"hash" : "a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15504",
"triggerID" : "a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530",
"triggerType" : "PUSH"
}, {
"hash" : "e453f39e2e3f48eabcb1470922573b07d4ed486d",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15507",
"triggerID" : "e453f39e2e3f48eabcb1470922573b07d4ed486d",
"triggerType" : "PUSH"
}, {
"hash" : "6995a948f49fadbec59748c4728a2beef6072b36",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "6995a948f49fadbec59748c4728a2beef6072b36",
"triggerType" : "PUSH"
}, {
"hash" : "d04edd5df0b50036aa9ef5175fa3f46f0b0f4c6f",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15533",
"triggerID" : "d04edd5df0b50036aa9ef5175fa3f46f0b0f4c6f",
"triggerType" : "PUSH"
}, {
"hash" : "b7c04d848ba10091669905ad1ebf02a528f80ace",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15547",
"triggerID" : "b7c04d848ba10091669905ad1ebf02a528f80ace",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 6995a948f49fadbec59748c4728a2beef6072b36 UNKNOWN
* d04edd5df0b50036aa9ef5175fa3f46f0b0f4c6f Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15533)
* b7c04d848ba10091669905ad1ebf02a528f80ace Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15547)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] lokeshj1703 commented on a diff in pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "lokeshj1703 (via GitHub)" <gi...@apache.org>.
lokeshj1703 commented on code in PR #8041:
URL: https://github.com/apache/hudi/pull/8041#discussion_r1119786931
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/Metrics.java:
##########
@@ -40,27 +48,51 @@ public class Metrics {
private static Metrics instance = null;
private final MetricRegistry registry;
- private MetricsReporter reporter;
+ private final List<MetricsReporter> reporters;
private final String commonMetricPrefix;
private Metrics(HoodieWriteConfig metricConfig) {
registry = new MetricRegistry();
commonMetricPrefix = metricConfig.getMetricReporterMetricsNamePrefix();
- reporter = MetricsReporterFactory.createReporter(metricConfig, registry);
- if (reporter == null) {
- throw new RuntimeException("Cannot initialize Reporter.");
+ reporters = new ArrayList<>();
+ MetricsReporter defaultReporter = MetricsReporterFactory.createReporter(metricConfig, registry);
+ if (defaultReporter != null) {
+ reporters.add(defaultReporter);
}
- reporter.start();
+ reporters.addAll(addAdditionalMetricsExporters(metricConfig));
+ if (reporters.size() == 0) {
+ throw new RuntimeException("Cannot initialize Reporters.");
+ }
+ reporters.forEach(MetricsReporter::start);
Runtime.getRuntime().addShutdownHook(new Thread(Metrics::shutdown));
}
+ private List<MetricsReporter> addAdditionalMetricsExporters(HoodieWriteConfig metricConfig) {
+ List<MetricsReporter> reporterList = new ArrayList<>();
+ if (!StringUtils.isNullOrEmpty(metricConfig.getMetricReporterFileBasedConfigs())) {
+ List<String> propPathList = StringUtils.split(metricConfig.getMetricReporterFileBasedConfigs(), ",");
+ try (FileSystem fs = FSUtils.getFs(propPathList.get(0), new Configuration())) {
+ for (String propPath: propPathList) {
+ HoodieWriteConfig secondarySourceConfig = HoodieWriteConfig.newBuilder().fromInputStream(
+ fs.open(new Path(propPath))).withPath(metricConfig.getBasePath()).build();
+ reporterList.add(MetricsReporterFactory.createReporter(secondarySourceConfig, registry));
+ }
+ } catch (IOException e) {
+ LOG.error("Failed to add MetricsExporters", e);
+ throw new HoodieException("failed to MetricsExporters", e);
Review Comment:
In the latest commit I have removed the exception.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/Metrics.java:
##########
@@ -118,6 +150,11 @@ public static void registerGauges(Map<String, Long> metricsMap, Option<String> p
metricsMap.forEach((k, v) -> registerGauge(metricPrefix + k, v));
}
+ public static void registerGauges(Map<String, Long> metricsMap, Option<String> prefix, Map<String, String> labels) {
Review Comment:
Removed it.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/MetricsReporterFactory.java:
##########
@@ -56,6 +56,9 @@ public static MetricsReporter createReporter(HoodieWriteConfig config, MetricReg
MetricsReporterType type = config.getMetricsReporterType();
MetricsReporter reporter = null;
+ if (type == null) {
+ return reporter;
Review Comment:
If type is null, there is no default reporter created. Type is used to determine whether reporter needs to be created or not today. So I am not throwing an exception here but I am converting return type to Option for this function.
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8041:
URL: https://github.com/apache/hudi/pull/8041#issuecomment-1448582523
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15386",
"triggerID" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"triggerType" : "PUSH"
}, {
"hash" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15475",
"triggerID" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15475)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8041:
URL: https://github.com/apache/hudi/pull/8041#issuecomment-1451658797
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15386",
"triggerID" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"triggerType" : "PUSH"
}, {
"hash" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15475",
"triggerID" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"triggerType" : "PUSH"
}, {
"hash" : "a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15504",
"triggerID" : "a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530",
"triggerType" : "PUSH"
}, {
"hash" : "e453f39e2e3f48eabcb1470922573b07d4ed486d",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15507",
"triggerID" : "e453f39e2e3f48eabcb1470922573b07d4ed486d",
"triggerType" : "PUSH"
}, {
"hash" : "6995a948f49fadbec59748c4728a2beef6072b36",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "6995a948f49fadbec59748c4728a2beef6072b36",
"triggerType" : "PUSH"
}, {
"hash" : "d04edd5df0b50036aa9ef5175fa3f46f0b0f4c6f",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15533",
"triggerID" : "d04edd5df0b50036aa9ef5175fa3f46f0b0f4c6f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* e453f39e2e3f48eabcb1470922573b07d4ed486d Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15507)
* 6995a948f49fadbec59748c4728a2beef6072b36 UNKNOWN
* d04edd5df0b50036aa9ef5175fa3f46f0b0f4c6f Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15533)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8041:
URL: https://github.com/apache/hudi/pull/8041#issuecomment-1451797447
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15386",
"triggerID" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"triggerType" : "PUSH"
}, {
"hash" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15475",
"triggerID" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"triggerType" : "PUSH"
}, {
"hash" : "a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15504",
"triggerID" : "a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530",
"triggerType" : "PUSH"
}, {
"hash" : "e453f39e2e3f48eabcb1470922573b07d4ed486d",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15507",
"triggerID" : "e453f39e2e3f48eabcb1470922573b07d4ed486d",
"triggerType" : "PUSH"
}, {
"hash" : "6995a948f49fadbec59748c4728a2beef6072b36",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "6995a948f49fadbec59748c4728a2beef6072b36",
"triggerType" : "PUSH"
}, {
"hash" : "d04edd5df0b50036aa9ef5175fa3f46f0b0f4c6f",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15533",
"triggerID" : "d04edd5df0b50036aa9ef5175fa3f46f0b0f4c6f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 6995a948f49fadbec59748c4728a2beef6072b36 UNKNOWN
* d04edd5df0b50036aa9ef5175fa3f46f0b0f4c6f Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15533)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] lokeshj1703 commented on a diff in pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "lokeshj1703 (via GitHub)" <gi...@apache.org>.
lokeshj1703 commented on code in PR #8041:
URL: https://github.com/apache/hudi/pull/8041#discussion_r1119632628
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/Metrics.java:
##########
@@ -40,27 +48,51 @@ public class Metrics {
private static Metrics instance = null;
private final MetricRegistry registry;
- private MetricsReporter reporter;
+ private final List<MetricsReporter> reporters;
private final String commonMetricPrefix;
private Metrics(HoodieWriteConfig metricConfig) {
registry = new MetricRegistry();
commonMetricPrefix = metricConfig.getMetricReporterMetricsNamePrefix();
- reporter = MetricsReporterFactory.createReporter(metricConfig, registry);
- if (reporter == null) {
- throw new RuntimeException("Cannot initialize Reporter.");
+ reporters = new ArrayList<>();
+ MetricsReporter defaultReporter = MetricsReporterFactory.createReporter(metricConfig, registry);
+ if (defaultReporter != null) {
+ reporters.add(defaultReporter);
}
- reporter.start();
+ reporters.addAll(addAdditionalMetricsExporters(metricConfig));
+ if (reporters.size() == 0) {
+ throw new RuntimeException("Cannot initialize Reporters.");
+ }
+ reporters.forEach(MetricsReporter::start);
Runtime.getRuntime().addShutdownHook(new Thread(Metrics::shutdown));
}
+ private List<MetricsReporter> addAdditionalMetricsExporters(HoodieWriteConfig metricConfig) {
+ List<MetricsReporter> reporterList = new ArrayList<>();
+ if (!StringUtils.isNullOrEmpty(metricConfig.getMetricReporterFileBasedConfigs())) {
+ List<String> propPathList = StringUtils.split(metricConfig.getMetricReporterFileBasedConfigs(), ",");
+ try (FileSystem fs = FSUtils.getFs(propPathList.get(0), new Configuration())) {
+ for (String propPath: propPathList) {
+ HoodieWriteConfig secondarySourceConfig = HoodieWriteConfig.newBuilder().fromInputStream(
+ fs.open(new Path(propPath))).withPath(metricConfig.getBasePath()).build();
+ reporterList.add(MetricsReporterFactory.createReporter(secondarySourceConfig, registry));
+ }
+ } catch (IOException e) {
+ LOG.error("Failed to add MetricsExporters", e);
+ throw new HoodieException("failed to MetricsExporters", e);
Review Comment:
I guess we could ignore a connection issue but this error would occur while reading the file. This would be more of a setup issue and could lead to a silent failure. It usually would take some time to debug these issues. WDYT?
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8041:
URL: https://github.com/apache/hudi/pull/8041#issuecomment-1449438799
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15386",
"triggerID" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"triggerType" : "PUSH"
}, {
"hash" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15475",
"triggerID" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"triggerType" : "PUSH"
}, {
"hash" : "a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15504",
"triggerID" : "a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15475)
* a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15504)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] lokeshj1703 commented on pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "lokeshj1703 (via GitHub)" <gi...@apache.org>.
lokeshj1703 commented on PR #8041:
URL: https://github.com/apache/hudi/pull/8041#issuecomment-1447564359
PR 7934 adds changes for supporting metric instances for different tables. This PR adds support for multiple reporters within a metric instance.
There would definitely be conflicts between these two PRs but otherwise PRs are tackling different issues.
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8041:
URL: https://github.com/apache/hudi/pull/8041#issuecomment-1443928774
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15386",
"triggerID" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2068af21c85e51254b849562f38cf6a2918f9bef Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15386)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] codope commented on a diff in pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "codope (via GitHub)" <gi...@apache.org>.
codope commented on code in PR #8041:
URL: https://github.com/apache/hudi/pull/8041#discussion_r1122576583
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/MetricUtils.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.metrics;
+
+import org.apache.hudi.common.util.StringUtils;
+import org.apache.hudi.common.util.ValidationUtils;
+import org.apache.hudi.common.util.collection.Pair;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+public class MetricUtils {
+
+ private static Pair<String, String> splitToPair(String label) {
+ String[] keyValues = label.split(":");
+ ValidationUtils.checkArgument(StringUtils.nonEmpty(keyValues[0]), String.format("Key is empty for label %s", label));
+ return Pair.of(keyValues[0], keyValues.length == 2 ? keyValues[1] : "");
+ }
+
+ public static Pair<String,Map<String, String>> getLabelsAndMetricMap(String metric) {
+ Pair<String, List<String>> labelsList = getLabelsAndMetricList(metric);
+ return Pair.of(labelsList.getLeft(),labelsList.getValue().stream().filter(s -> s.contains(":")).map(MetricUtils::splitToPair)
+ .collect(Collectors.toMap(Pair::getLeft, Pair::getRight))
+ );
+ }
+
+ public static Pair<String,String> getMetricAndLabels(String metric) {
+ String[] tokens = metric.split(";");
Review Comment:
should this be split on `:` instead of `;`? Can we declare these delimiters as constants for better readability and reusability?
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8041:
URL: https://github.com/apache/hudi/pull/8041#issuecomment-1453179733
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15386",
"triggerID" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"triggerType" : "PUSH"
}, {
"hash" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15475",
"triggerID" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"triggerType" : "PUSH"
}, {
"hash" : "a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15504",
"triggerID" : "a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530",
"triggerType" : "PUSH"
}, {
"hash" : "e453f39e2e3f48eabcb1470922573b07d4ed486d",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15507",
"triggerID" : "e453f39e2e3f48eabcb1470922573b07d4ed486d",
"triggerType" : "PUSH"
}, {
"hash" : "6995a948f49fadbec59748c4728a2beef6072b36",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "6995a948f49fadbec59748c4728a2beef6072b36",
"triggerType" : "PUSH"
}, {
"hash" : "d04edd5df0b50036aa9ef5175fa3f46f0b0f4c6f",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15533",
"triggerID" : "d04edd5df0b50036aa9ef5175fa3f46f0b0f4c6f",
"triggerType" : "PUSH"
}, {
"hash" : "b7c04d848ba10091669905ad1ebf02a528f80ace",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15547",
"triggerID" : "b7c04d848ba10091669905ad1ebf02a528f80ace",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 6995a948f49fadbec59748c4728a2beef6072b36 UNKNOWN
* b7c04d848ba10091669905ad1ebf02a528f80ace Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15547)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] nsivabalan commented on pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on PR #8041:
URL: https://github.com/apache/hudi/pull/8041#issuecomment-1447577236
sg
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8041:
URL: https://github.com/apache/hudi/pull/8041#issuecomment-1451648879
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15386",
"triggerID" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"triggerType" : "PUSH"
}, {
"hash" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15475",
"triggerID" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"triggerType" : "PUSH"
}, {
"hash" : "a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15504",
"triggerID" : "a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530",
"triggerType" : "PUSH"
}, {
"hash" : "e453f39e2e3f48eabcb1470922573b07d4ed486d",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15507",
"triggerID" : "e453f39e2e3f48eabcb1470922573b07d4ed486d",
"triggerType" : "PUSH"
}, {
"hash" : "6995a948f49fadbec59748c4728a2beef6072b36",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "6995a948f49fadbec59748c4728a2beef6072b36",
"triggerType" : "PUSH"
}, {
"hash" : "d04edd5df0b50036aa9ef5175fa3f46f0b0f4c6f",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "d04edd5df0b50036aa9ef5175fa3f46f0b0f4c6f",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* e453f39e2e3f48eabcb1470922573b07d4ed486d Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15507)
* 6995a948f49fadbec59748c4728a2beef6072b36 UNKNOWN
* d04edd5df0b50036aa9ef5175fa3f46f0b0f4c6f UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] lokeshj1703 commented on a diff in pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "lokeshj1703 (via GitHub)" <gi...@apache.org>.
lokeshj1703 commented on code in PR #8041:
URL: https://github.com/apache/hudi/pull/8041#discussion_r1119787291
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/MetricUtils.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.metrics;
+
+import org.apache.hudi.common.util.collection.Pair;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+public class MetricUtils {
+
+ private static Pair<String, String> splitToPair(String label) {
+ String[] keyValuess = label.split(":");
Review Comment:
Addressed
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/MetricUtils.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.metrics;
+
+import org.apache.hudi.common.util.collection.Pair;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+public class MetricUtils {
+
+ private static Pair<String, String> splitToPair(String label) {
+ String[] keyValuess = label.split(":");
Review Comment:
The caller of `splitToPair` is filtering strings which have `:` before passing to function. But in a weird case where input string is `:`, key and value can both be empty. Adding a validation check for this.
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] lokeshj1703 commented on a diff in pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "lokeshj1703 (via GitHub)" <gi...@apache.org>.
lokeshj1703 commented on code in PR #8041:
URL: https://github.com/apache/hudi/pull/8041#discussion_r1119786548
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/Metrics.java:
##########
@@ -40,27 +48,51 @@ public class Metrics {
private static Metrics instance = null;
private final MetricRegistry registry;
- private MetricsReporter reporter;
+ private final List<MetricsReporter> reporters;
private final String commonMetricPrefix;
private Metrics(HoodieWriteConfig metricConfig) {
registry = new MetricRegistry();
commonMetricPrefix = metricConfig.getMetricReporterMetricsNamePrefix();
- reporter = MetricsReporterFactory.createReporter(metricConfig, registry);
- if (reporter == null) {
- throw new RuntimeException("Cannot initialize Reporter.");
+ reporters = new ArrayList<>();
+ MetricsReporter defaultReporter = MetricsReporterFactory.createReporter(metricConfig, registry);
+ if (defaultReporter != null) {
+ reporters.add(defaultReporter);
}
- reporter.start();
+ reporters.addAll(addAdditionalMetricsExporters(metricConfig));
+ if (reporters.size() == 0) {
+ throw new RuntimeException("Cannot initialize Reporters.");
+ }
+ reporters.forEach(MetricsReporter::start);
Runtime.getRuntime().addShutdownHook(new Thread(Metrics::shutdown));
}
+ private List<MetricsReporter> addAdditionalMetricsExporters(HoodieWriteConfig metricConfig) {
+ List<MetricsReporter> reporterList = new ArrayList<>();
+ if (!StringUtils.isNullOrEmpty(metricConfig.getMetricReporterFileBasedConfigs())) {
Review Comment:
Addressed
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/Metrics.java:
##########
@@ -40,27 +48,51 @@ public class Metrics {
private static Metrics instance = null;
private final MetricRegistry registry;
- private MetricsReporter reporter;
+ private final List<MetricsReporter> reporters;
private final String commonMetricPrefix;
private Metrics(HoodieWriteConfig metricConfig) {
registry = new MetricRegistry();
commonMetricPrefix = metricConfig.getMetricReporterMetricsNamePrefix();
- reporter = MetricsReporterFactory.createReporter(metricConfig, registry);
- if (reporter == null) {
- throw new RuntimeException("Cannot initialize Reporter.");
+ reporters = new ArrayList<>();
+ MetricsReporter defaultReporter = MetricsReporterFactory.createReporter(metricConfig, registry);
+ if (defaultReporter != null) {
+ reporters.add(defaultReporter);
}
- reporter.start();
+ reporters.addAll(addAdditionalMetricsExporters(metricConfig));
+ if (reporters.size() == 0) {
+ throw new RuntimeException("Cannot initialize Reporters.");
+ }
+ reporters.forEach(MetricsReporter::start);
Runtime.getRuntime().addShutdownHook(new Thread(Metrics::shutdown));
}
+ private List<MetricsReporter> addAdditionalMetricsExporters(HoodieWriteConfig metricConfig) {
+ List<MetricsReporter> reporterList = new ArrayList<>();
+ if (!StringUtils.isNullOrEmpty(metricConfig.getMetricReporterFileBasedConfigs())) {
Review Comment:
Addressed
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8041:
URL: https://github.com/apache/hudi/pull/8041#issuecomment-1447865859
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15386",
"triggerID" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"triggerType" : "PUSH"
}, {
"hash" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 2068af21c85e51254b849562f38cf6a2918f9bef Azure: [SUCCESS](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15386)
* 9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8041:
URL: https://github.com/apache/hudi/pull/8041#issuecomment-1449446634
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15386",
"triggerID" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"triggerType" : "PUSH"
}, {
"hash" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15475",
"triggerID" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"triggerType" : "PUSH"
}, {
"hash" : "a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530",
"status" : "PENDING",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15504",
"triggerID" : "a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530",
"triggerType" : "PUSH"
}, {
"hash" : "e453f39e2e3f48eabcb1470922573b07d4ed486d",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "e453f39e2e3f48eabcb1470922573b07d4ed486d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15475)
* a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530 Azure: [PENDING](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15504)
* e453f39e2e3f48eabcb1470922573b07d4ed486d UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] lokeshj1703 commented on a diff in pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "lokeshj1703 (via GitHub)" <gi...@apache.org>.
lokeshj1703 commented on code in PR #8041:
URL: https://github.com/apache/hudi/pull/8041#discussion_r1122837033
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metrics/MetricUtils.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hudi.metrics;
+
+import org.apache.hudi.common.util.StringUtils;
+import org.apache.hudi.common.util.ValidationUtils;
+import org.apache.hudi.common.util.collection.Pair;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+public class MetricUtils {
+
+ private static Pair<String, String> splitToPair(String label) {
+ String[] keyValues = label.split(":");
+ ValidationUtils.checkArgument(StringUtils.nonEmpty(keyValues[0]), String.format("Key is empty for label %s", label));
+ return Pair.of(keyValues[0], keyValues.length == 2 ? keyValues[1] : "");
+ }
+
+ public static Pair<String,Map<String, String>> getLabelsAndMetricMap(String metric) {
+ Pair<String, List<String>> labelsList = getLabelsAndMetricList(metric);
+ return Pair.of(labelsList.getLeft(),labelsList.getValue().stream().filter(s -> s.contains(":")).map(MetricUtils::splitToPair)
+ .collect(Collectors.toMap(Pair::getLeft, Pair::getRight))
+ );
+ }
+
+ public static Pair<String,String> getMetricAndLabels(String metric) {
+ String[] tokens = metric.split(";");
Review Comment:
This is correct. I have added some docs and constants.
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hudi] hudi-bot commented on pull request #8041: [HUDI-5847] Add support for multiple metric reporters and metric labels
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #8041:
URL: https://github.com/apache/hudi/pull/8041#issuecomment-1451591128
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15386",
"triggerID" : "2068af21c85e51254b849562f38cf6a2918f9bef",
"triggerType" : "PUSH"
}, {
"hash" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15475",
"triggerID" : "9dca0ad4d4f8468f432044c3fd4b90434ecfe1fc",
"triggerType" : "PUSH"
}, {
"hash" : "a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530",
"status" : "DELETED",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15504",
"triggerID" : "a8cdea99cfbe90b9f07de9b63d7d5d9ca1c27530",
"triggerType" : "PUSH"
}, {
"hash" : "e453f39e2e3f48eabcb1470922573b07d4ed486d",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15507",
"triggerID" : "e453f39e2e3f48eabcb1470922573b07d4ed486d",
"triggerType" : "PUSH"
}, {
"hash" : "6995a948f49fadbec59748c4728a2beef6072b36",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "6995a948f49fadbec59748c4728a2beef6072b36",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* e453f39e2e3f48eabcb1470922573b07d4ed486d Azure: [FAILURE](https://dev.azure.com/apache-hudi-ci-org/785b6ef4-2f42-4a89-8f0e-5f0d7039a0cc/_build/results?buildId=15507)
* 6995a948f49fadbec59748c4728a2beef6072b36 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org