New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FLINK-4831][metrics] Implement a slf4j metric reporter #4661
Conversation
Really nice that you figured out to rename the package to slf4j :) I'll check this out in more detail next week. |
@zentol Please let me know if this is good :) |
CC @zentol |
for (int i = 0; i < strLen; i++) { | ||
final char c = input.charAt(i); | ||
switch (c) { | ||
case ':': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are we filtering this character?
/** | ||
* Test for {@link Slf4jReporter}. | ||
*/ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove empty line
<groupId>org.apache.flink</groupId> | ||
<artifactId>flink-runtime_${scala.binary.version}</artifactId> | ||
<version>${project.version}</version> | ||
<scope>provided</scope> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move to test dependency block and set scope to test
|
||
<dependency> | ||
<groupId>org.apache.flink</groupId> | ||
<artifactId>flink-test-utils-junit</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add <scope>test</scope>
# limitations under the License. | ||
# | ||
|
||
log4j.rootLogger=INFO, testlogger |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
@zentol thanks a lot for your review. I have updated the PR according to your comments. |
merging. |
What is the purpose of the change
Implement a slf4j metric reporter For debugging.
Brief change log
Verifying this change
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: (no)Documentation