You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by yew1eb <gi...@git.apache.org> on 2017/09/08 18:43:54 UTC
[GitHub] flink pull request #4661: [FLINK-4831] Implement a slf4j metric reporter
GitHub user yew1eb opened a pull request:
https://github.com/apache/flink/pull/4661
[FLINK-4831] Implement a slf4j metric reporter
## What is the purpose of the change
Implement a slf4j metric reporter For debugging.
## Brief change log
## Verifying this change
- *Added unit test Slf4jReporterTest*
- *Build a local flink cluster test Slf4jReporter, it works well*
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): (no)
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no)
- The serializers: (no)
- The runtime per-record code paths (performance sensitive): (no)
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)
## Documentation
- Does this pull request introduce a new feature? (yes)
- If yes, how is the feature documented? (docs)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/yew1eb/flink FLINK-4831
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/4661.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #4661
----
commit 79804a6ae554b93d41e5c8748c6442c2aa330fa2
Author: yew1eb <ye...@gmail.com>
Date: 2017-09-06T15:09:13Z
Implement a slf4j metric reporter
----
---
[GitHub] flink pull request #4661: [FLINK-4831][metrics] Implement a slf4j metric rep...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/4661
---
[GitHub] flink issue #4661: [FLINK-4831][metrics] Implement a slf4j metric reporter
Posted by yew1eb <gi...@git.apache.org>.
Github user yew1eb commented on the issue:
https://github.com/apache/flink/pull/4661
@zentol thanks a lot for your review. I have updated the PR according to your comments.
---
[GitHub] flink pull request #4661: [FLINK-4831][metrics] Implement a slf4j metric rep...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/4661#discussion_r146533089
--- Diff: flink-metrics/flink-metrics-slf4j/pom.xml ---
@@ -0,0 +1,65 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+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.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+
+ <modelVersion>4.0.0</modelVersion>
+
+ <parent>
+ <groupId>org.apache.flink</groupId>
+ <artifactId>flink-metrics</artifactId>
+ <version>1.4-SNAPSHOT</version>
+ <relativePath>..</relativePath>
+ </parent>
+
+ <artifactId>flink-metrics-slf4j</artifactId>
+ <name>flink-metrics-slf4j</name>
+
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.flink</groupId>
+ <artifactId>flink-annotations</artifactId>
+ <version>${project.version}</version>
+ <scope>provided</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.flink</groupId>
+ <artifactId>flink-runtime_${scala.binary.version}</artifactId>
+ <version>${project.version}</version>
+ <scope>provided</scope>
--- End diff --
move to test dependency block and set scope to `test`
---
[GitHub] flink issue #4661: [FLINK-4831][metrics] Implement a slf4j metric reporter
Posted by yew1eb <gi...@git.apache.org>.
Github user yew1eb commented on the issue:
https://github.com/apache/flink/pull/4661
@zentol Please let me know if this is good :)
---
[GitHub] flink issue #4661: [FLINK-4831] Implement a slf4j metric reporter
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/4661
Really nice that you figured out to rename the package to slf4j :)
I'll check this out in more detail next week.
---
[GitHub] flink pull request #4661: [FLINK-4831][metrics] Implement a slf4j metric rep...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/4661#discussion_r146533152
--- Diff: flink-metrics/flink-metrics-slf4j/pom.xml ---
@@ -0,0 +1,65 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+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.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+
+ <modelVersion>4.0.0</modelVersion>
+
+ <parent>
+ <groupId>org.apache.flink</groupId>
+ <artifactId>flink-metrics</artifactId>
+ <version>1.4-SNAPSHOT</version>
+ <relativePath>..</relativePath>
+ </parent>
+
+ <artifactId>flink-metrics-slf4j</artifactId>
+ <name>flink-metrics-slf4j</name>
+
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.flink</groupId>
+ <artifactId>flink-annotations</artifactId>
+ <version>${project.version}</version>
+ <scope>provided</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.flink</groupId>
+ <artifactId>flink-runtime_${scala.binary.version}</artifactId>
+ <version>${project.version}</version>
+ <scope>provided</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.flink</groupId>
+ <artifactId>flink-metrics-core</artifactId>
+ <version>${project.version}</version>
+ <scope>provided</scope>
+ </dependency>
+
+ <!-- test dependencies -->
+
+ <dependency>
+ <groupId>org.apache.flink</groupId>
+ <artifactId>flink-test-utils-junit</artifactId>
--- End diff --
add `<scope>test</scope>
---
[GitHub] flink issue #4661: [FLINK-4831][metrics] Implement a slf4j metric reporter
Posted by yew1eb <gi...@git.apache.org>.
Github user yew1eb commented on the issue:
https://github.com/apache/flink/pull/4661
cc @zentol
---
[GitHub] flink issue #4661: [FLINK-4831][metrics] Implement a slf4j metric reporter
Posted by yew1eb <gi...@git.apache.org>.
Github user yew1eb commented on the issue:
https://github.com/apache/flink/pull/4661
CC @zentol
---
[GitHub] flink pull request #4661: [FLINK-4831][metrics] Implement a slf4j metric rep...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/4661#discussion_r146535744
--- Diff: flink-metrics/flink-metrics-slf4j/src/test/resources/log4j-test.properties ---
@@ -0,0 +1,24 @@
+#
+# 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.
+#
+
+log4j.rootLogger=INFO, testlogger
--- End diff --
This will cause a bit of noise in the logs that I would like to avoid.
Instead, can we, If the log level is configured to OFF, remove all other appenders and set it to INFO? This will make the test run without affecting log files etc.
---
[GitHub] flink pull request #4661: [FLINK-4831][metrics] Implement a slf4j metric rep...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/4661#discussion_r139702401
--- Diff: flink-metrics/flink-metrics-slf4j/src/main/java/org/apache/flink/metrics/slf4j/Slf4jReporter.java ---
@@ -0,0 +1,135 @@
+/*
+ * 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.flink.metrics.slf4j;
+
+import org.apache.flink.annotation.VisibleForTesting;
+import org.apache.flink.metrics.Counter;
+import org.apache.flink.metrics.Gauge;
+import org.apache.flink.metrics.Histogram;
+import org.apache.flink.metrics.HistogramStatistics;
+import org.apache.flink.metrics.Meter;
+import org.apache.flink.metrics.Metric;
+import org.apache.flink.metrics.MetricConfig;
+import org.apache.flink.metrics.reporter.AbstractReporter;
+import org.apache.flink.metrics.reporter.MetricReporter;
+import org.apache.flink.metrics.reporter.Scheduled;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Map;
+
+/**
+ * {@link MetricReporter} that exports {@link Metric Metrics} via SLF4J {@link Logger}.
+ */
+public class Slf4jReporter extends AbstractReporter implements Scheduled {
+ private static final Logger LOG = LoggerFactory.getLogger(Slf4jReporter.class);
+ private static final String lineSeparator = System.lineSeparator();
+
+ @VisibleForTesting
+ public Map<Gauge<?>, String> getGauges() {
+ return gauges;
+ }
+
+ @VisibleForTesting
+ public Map<Counter, String> getCounters() {
+ return counters;
+ }
+
+ @VisibleForTesting
+ public Map<Histogram, String> getHistograms() {
+ return histograms;
+ }
+
+ @VisibleForTesting
+ public Map<Meter, String> getMeters() {
+ return meters;
+ }
+
+ @Override
+ public void open(MetricConfig metricConfig) {
+
+ }
+
+ @Override
+ public void close() {
+
+ }
+
+ @Override
+ public void report() {
+ StringBuilder builder = new StringBuilder();
+ builder.append(lineSeparator).append("=========================== Starting metrics report ===========================").append(lineSeparator);
+ builder.append("-- Counters -------------------------------------------------------------------").append(lineSeparator);
+ for (Map.Entry<Counter, String> metric : counters.entrySet()) {
+ builder.append(metric.getValue()).append(": count=").append(metric.getKey().getCount()).append(lineSeparator);
+ }
+
+ builder.append(lineSeparator).append("-- Gauges ---------------------------------------------------------------------").append(lineSeparator);
+ for (Map.Entry<Gauge<?>, String> metric : gauges.entrySet()) {
+ builder.append(metric.getValue()).append(": value=").append(metric.getKey().getValue()).append(lineSeparator);
+ }
+
+ builder.append(lineSeparator).append("-- Histograms -----------------------------------------------------------------").append(lineSeparator);
+ for (Map.Entry<Histogram, String> metric : histograms.entrySet()) {
+ HistogramStatistics stats = metric.getKey().getStatistics();
+ builder.append(metric.getValue()).append(": count=").append(stats.size())
+ .append(", min=").append(stats.getMin()).append(", max=").append(stats.getMax())
+ .append(", mean=").append(stats.getMean()).append(", stddev=").append(stats.getStdDev())
+ .append(", p50=").append(stats.getQuantile(0.50)).append(", p75=").append(stats.getQuantile(0.75))
+ .append(", p95=").append(stats.getQuantile(0.95)).append(", p98=").append(stats.getQuantile(0.98))
+ .append(", p99=").append(stats.getQuantile(0.99)).append(", p999=").append(stats.getQuantile(0.999)).append(lineSeparator);
+ }
+
+ builder.append(lineSeparator).append("-- Meters ---------------------------------------------------------------------").append(lineSeparator);
+ for (Map.Entry<Meter, String> metric : meters.entrySet()) {
+ builder.append(metric.getValue()).append(": rate=").append(metric.getKey().getRate()).append(lineSeparator);
+ }
+
+ builder.append(lineSeparator).append("=========================== Finished metrics report ===========================").append(lineSeparator);
+ LOG.info(builder.toString());
+ }
+
+ @Override
+ public String filterCharacters(String input) {
+ char[] chars = null;
+ final int strLen = input.length();
+ int pos = 0;
+
+ for (int i = 0; i < strLen; i++) {
+ final char c = input.charAt(i);
+ switch (c) {
+ case ':':
--- End diff --
why are we filtering this character?
---
[GitHub] flink issue #4661: [FLINK-4831][metrics] Implement a slf4j metric reporter
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/4661
merging.
---
[GitHub] flink pull request #4661: [FLINK-4831][metrics] Implement a slf4j metric rep...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/4661#discussion_r139702445
--- Diff: flink-metrics/flink-metrics-slf4j/src/test/java/org/apache/flink/metrics/slf4j/Slf4jReporterTest.java ---
@@ -0,0 +1,128 @@
+/*
+ * 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.flink.metrics.slf4j;
+
+import org.apache.flink.api.common.JobID;
+import org.apache.flink.configuration.ConfigConstants;
+import org.apache.flink.configuration.Configuration;
+import org.apache.flink.configuration.MetricOptions;
+import org.apache.flink.metrics.Gauge;
+import org.apache.flink.metrics.SimpleCounter;
+import org.apache.flink.runtime.metrics.MetricRegistry;
+import org.apache.flink.runtime.metrics.MetricRegistryConfiguration;
+import org.apache.flink.runtime.metrics.groups.TaskManagerJobMetricGroup;
+import org.apache.flink.runtime.metrics.groups.TaskManagerMetricGroup;
+import org.apache.flink.runtime.metrics.groups.TaskMetricGroup;
+import org.apache.flink.util.AbstractID;
+import org.apache.flink.util.TestLogger;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import static org.hamcrest.Matchers.equalTo;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Test for {@link Slf4jReporter}.
+ */
+
--- End diff --
remove empty line
---