[SPARK-14754][SPARK CORE] Metrics as logs are not coming through slf4j #12697
Conversation
mihir6692
changed the title
|
Can one of the admins verify this patch? |
| MetricsSystem.checkMinimalPollingPeriod(pollUnit, pollPeriod) | ||
|
|
||
| val reporter: Slf4jReporter = Slf4jReporter.forRegistry(registry) | ||
| .outputTo(LoggerFactory.getLogger("org.apache.spark.metrics")) |
There was a problem hiding this comment.
Why not the class name for the logger name here, per your JIRA?
Updated import style as per conventions
Update class name for logger
|
I have added 2 more commits and updated the pull request. Please have a look. |
| MetricsSystem.checkMinimalPollingPeriod(pollUnit, pollPeriod) | ||
|
|
||
| val reporter: Slf4jReporter = Slf4jReporter.forRegistry(registry) | ||
| .outputTo(LoggerFactory.getLogger("org.apache.spark.metrics.sink.Slf4jSink")) |
There was a problem hiding this comment.
Sure, it can be classOf[Slf4jSink].getName for simplicity and to avoid forgetting to update it if for some reason this moves.
There was a problem hiding this comment.
For Slf4jSink.scala :-
Its not about class path or class level in package. It is just a name. Ex.
If you keep name like Spark.log4j, it would still work. ( and use the
same Spark.log4j in log4j.properties). So it won't matter even if we
move class to some other folder or package.
EDIT :-
I understood what you are trying to say. I will update it with new commit. Thanks for guidance.
|
gentle ping @mihir6692 |
1 similar comment
|
gentle ping @mihir6692 |
4 participants
What changes were proposed in this pull request?
Two changes need to be done :
Slf4jsink.scala
Added class name for logging. (Reference : https://dropwizard.github.io/metrics/3.1.0/manual/core/#man-core-reporters-slf4j )
log4j.properties.template
Added log configuration in log4j.properties.template for in support of above changes.
How was this patch tested?
It is tested with manual testing. I build spark with make-distribution.sh and then tried few example job to print Metrics in log files.