You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by mihir6692 <gi...@git.apache.org> on 2016/04/26 10:32:06 UTC

[GitHub] spark pull request: [SPARK-14754][CORE] Metrics as logs are not co...

GitHub user mihir6692 opened a pull request:

    https://github.com/apache/spark/pull/12697

    [SPARK-14754][CORE] Metrics as logs are not coming through slf4j

    ## What changes were proposed in this pull request?
    
    Two changes need to be done : 
    1) Slf4jsink.scala 
            Added class name for logging. (Reference : https://dropwizard.github.io/metrics/3.1.0/manual/core/#man-core-reporters-slf4j )
    
    2) 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.
    


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/mihir6692/spark master

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/12697.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 #12697
    
----
commit 12d896c9d9b326c8539d11376c1d28187a343057
Author: mihir6692 <mo...@gmail.com>
Date:   2016-04-26T08:25:45Z

    [SPARK-14754][CORE]Metrics as logs are not coming through slf4j

commit 5a437a12fb0c6cfdd7119d63bebc3f3f2c935d5f
Author: mihir6692 <mo...@gmail.com>
Date:   2016-04-26T08:27:26Z

    [SPARK-14754][CORE]Metrics as logs are not coming through slf4j

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14754][SPARK CORE] Metrics as logs are ...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12697#discussion_r61051439
  
    --- Diff: core/src/main/scala/org/apache/spark/metrics/sink/Slf4jSink.scala ---
    @@ -25,6 +25,9 @@ import com.codahale.metrics.{MetricRegistry, Slf4jReporter}
     import org.apache.spark.SecurityManager
     import org.apache.spark.metrics.MetricsSystem
     
    +import org.slf4j.Logger
    +import org.slf4j.LoggerFactory
    --- End diff --
    
    (It would be great if you run `sbt scalastyle` before adding some more commits which shows up such things)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12697: [SPARK-14754][SPARK CORE] Metrics as logs are not coming...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/12697
  
    gentle ping @mihir6692 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14754][SPARK CORE] Metrics as logs are ...

Posted by mihir6692 <gi...@git.apache.org>.
Github user mihir6692 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12697#discussion_r61053479
  
    --- Diff: core/src/main/scala/org/apache/spark/metrics/sink/Slf4jSink.scala ---
    @@ -25,6 +25,9 @@ import com.codahale.metrics.{MetricRegistry, Slf4jReporter}
     import org.apache.spark.SecurityManager
     import org.apache.spark.metrics.MetricsSystem
     
    +import org.slf4j.Logger
    +import org.slf4j.LoggerFactory
    --- End diff --
    
    Ok Sean, I understood your point. I will recreate pull request with more
    precise changes and will also run "sbt scalastyle" for it.
    
    Should i close this pull request??
    
    On Tue, Apr 26, 2016 at 2:48 PM, Sean Owen <no...@github.com> wrote:
    
    > In core/src/main/scala/org/apache/spark/metrics/sink/Slf4jSink.scala
    > <https://github.com/apache/spark/pull/12697#discussion_r61053012>:
    >
    > > @@ -25,6 +25,9 @@ import com.codahale.metrics.{MetricRegistry, Slf4jReporter}
    > >  import org.apache.spark.SecurityManager
    > >  import org.apache.spark.metrics.MetricsSystem
    > >
    > > +import org.slf4j.Logger
    > > +import org.slf4j.LoggerFactory
    >
    > The problem you describe is that log messages aren't plumbed through to a
    > logger, right? that shouldn't require changing appender config for log4j.
    > Your JIRA says "Slf4jsink.scala should have class name for log4j to print
    > metrics in log files" but your code has a package name.
    >
    > —
    > You are receiving this because you authored the thread.
    > Reply to this email directly or view it on GitHub
    > <https://github.com/apache/spark/pull/12697/files/5a437a12fb0c6cfdd7119d63bebc3f3f2c935d5f#r61053012>
    >
    
    
    
    -- 
    Mihir Monani
    (+91)-9429473434



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14754][SPARK CORE] Metrics as logs are ...

Posted by mihir6692 <gi...@git.apache.org>.
Github user mihir6692 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12697#discussion_r61162331
  
    --- Diff: core/src/main/scala/org/apache/spark/metrics/sink/Slf4jSink.scala ---
    @@ -49,6 +52,7 @@ private[spark] class Slf4jSink(
       MetricsSystem.checkMinimalPollingPeriod(pollUnit, pollPeriod)
     
       val reporter: Slf4jReporter = Slf4jReporter.forRegistry(registry)
    +    .outputTo(LoggerFactory.getLogger("org.apache.spark.metrics.sink.Slf4jSink"))
    --- End diff --
    
    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.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14754][SPARK CORE] Metrics as logs are ...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12697#discussion_r61066099
  
    --- Diff: core/src/main/scala/org/apache/spark/metrics/sink/Slf4jSink.scala ---
    @@ -49,6 +52,7 @@ private[spark] class Slf4jSink(
       MetricsSystem.checkMinimalPollingPeriod(pollUnit, pollPeriod)
     
       val reporter: Slf4jReporter = Slf4jReporter.forRegistry(registry)
    +    .outputTo(LoggerFactory.getLogger("org.apache.spark.metrics.sink.Slf4jSink"))
    --- End diff --
    
    Sure, it can be `classOf[Slf4jSink].getName` for simplicity and to avoid forgetting to update it if for some reason this moves.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14754][SPARK CORE] Metrics as logs are ...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12697#discussion_r61066147
  
    --- Diff: conf/log4j.properties.template ---
    @@ -38,3 +38,14 @@ log4j.logger.parquet=ERROR
     # SPARK-9183: Settings to avoid annoying messages when looking up nonexistent UDFs in SparkSQL with Hive support
     log4j.logger.org.apache.hadoop.hive.metastore.RetryingHMSHandler=FATAL
     log4j.logger.org.apache.hadoop.hive.ql.exec.FunctionRegistry=ERROR
    +
    +# SPARK-14754: Metrics as logs are not coming through slf4j.
    +#log4j.logger.org.apache.spark.metrics=INFO, metricFileAppender
    +#log4j.additivity.org.apache.spark.metrics=true
    +
    +#log4j.appender.metricFileAppender=org.apache.log4j.RollingFileAppender
    +#log4j.appender.metricFileAppender.File=${logFilePath}
    +#log4j.appender.metricFileAppender.MaxFileSize=10MB
    --- End diff --
    
    I still don't think this has been addressed. It shouldn't need a new appender, right?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14754][SPARK CORE] Metrics as logs are ...

Posted by mihir6692 <gi...@git.apache.org>.
Github user mihir6692 commented on the pull request:

    https://github.com/apache/spark/pull/12697#issuecomment-214696213
  
    I have added 2 more commits and updated the pull request. 
    
    Please have a look. 
    Thanks


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #12697: [SPARK-14754][SPARK CORE] Metrics as logs are not...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/12697


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14754][SPARK CORE] Metrics as logs are ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/12697#issuecomment-214668687
  
    Can one of the admins verify this patch?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14754][SPARK CORE] Metrics as logs are ...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12697#discussion_r61048436
  
    --- Diff: conf/log4j.properties.template ---
    @@ -38,3 +38,14 @@ log4j.logger.parquet=ERROR
     # SPARK-9183: Settings to avoid annoying messages when looking up nonexistent UDFs in SparkSQL with Hive support
     log4j.logger.org.apache.hadoop.hive.metastore.RetryingHMSHandler=FATAL
     log4j.logger.org.apache.hadoop.hive.ql.exec.FunctionRegistry=ERROR
    +
    +# SPARK-14754: Metrics as logs are not coming through slf4j.
    +#log4j.logger.org.apache.spark.metrics=INFO, metricFileAppender
    +#log4j.additivity.org.apache.spark.metrics=true
    +
    +#log4j.appender.metricFileAppender=org.apache.log4j.RollingFileAppender
    +#log4j.appender.metricFileAppender.File=${logFilePath}
    +#log4j.appender.metricFileAppender.MaxFileSize=10MB
    --- End diff --
    
    Why is all this config needed?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14754][SPARK CORE] Metrics as logs are ...

Posted by mihir6692 <gi...@git.apache.org>.
Github user mihir6692 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12697#discussion_r61054007
  
    --- Diff: core/src/main/scala/org/apache/spark/metrics/sink/Slf4jSink.scala ---
    @@ -25,6 +25,9 @@ import com.codahale.metrics.{MetricRegistry, Slf4jReporter}
     import org.apache.spark.SecurityManager
     import org.apache.spark.metrics.MetricsSystem
     
    +import org.slf4j.Logger
    +import org.slf4j.LoggerFactory
    --- End diff --
    
    ok. I will push more commits and update pull request.
    Thanks.
    
    On Tue, Apr 26, 2016 at 2:52 PM, Sean Owen <no...@github.com> wrote:
    
    > In core/src/main/scala/org/apache/spark/metrics/sink/Slf4jSink.scala
    > <https://github.com/apache/spark/pull/12697#discussion_r61053629>:
    >
    > > @@ -25,6 +25,9 @@ import com.codahale.metrics.{MetricRegistry, Slf4jReporter}
    > >  import org.apache.spark.SecurityManager
    > >  import org.apache.spark.metrics.MetricsSystem
    > >
    > > +import org.slf4j.Logger
    > > +import org.slf4j.LoggerFactory
    >
    > No you simply push more commits to this branch
    >
    > —
    > You are receiving this because you authored the thread.
    > Reply to this email directly or view it on GitHub
    > <https://github.com/apache/spark/pull/12697/files/5a437a12fb0c6cfdd7119d63bebc3f3f2c935d5f#r61053629>
    >
    
    
    
    -- 
    Mihir Monani
    (+91)-9429473434



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14754][SPARK CORE] Metrics as logs are ...

Posted by mihir6692 <gi...@git.apache.org>.
Github user mihir6692 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12697#discussion_r61161998
  
    --- Diff: conf/log4j.properties.template ---
    @@ -38,3 +38,14 @@ log4j.logger.parquet=ERROR
     # SPARK-9183: Settings to avoid annoying messages when looking up nonexistent UDFs in SparkSQL with Hive support
     log4j.logger.org.apache.hadoop.hive.metastore.RetryingHMSHandler=FATAL
     log4j.logger.org.apache.hadoop.hive.ql.exec.FunctionRegistry=ERROR
    +
    +# SPARK-14754: Metrics as logs are not coming through slf4j.
    +#log4j.logger.org.apache.spark.metrics=INFO, metricFileAppender
    +#log4j.additivity.org.apache.spark.metrics=true
    +
    +#log4j.appender.metricFileAppender=org.apache.log4j.RollingFileAppender
    +#log4j.appender.metricFileAppender.File=${logFilePath}
    +#log4j.appender.metricFileAppender.MaxFileSize=10MB
    --- End diff --
    
    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.
    
    For log4j.properties :-
    
    Main reason to use new appenders is to get metrics in separate file which
    will reduce complexity if someone one wants to parse metric.
    
    It would be better to use new appender so when you disable root logger ,
    you will only disable application logs, not the metrics logs. ( for more
    detailed and through explanation :- http://stackoverflow.com/a/23323046 )
    
    On Tue, Apr 26, 2016 at 4:27 PM, Sean Owen <no...@github.com> wrote:
    
    > In conf/log4j.properties.template
    > <https://github.com/apache/spark/pull/12697#discussion_r61066147>:
    >
    > > @@ -38,3 +38,14 @@ log4j.logger.parquet=ERROR
    > >  # SPARK-9183: Settings to avoid annoying messages when looking up nonexistent UDFs in SparkSQL with Hive support
    > >  log4j.logger.org.apache.hadoop.hive.metastore.RetryingHMSHandler=FATAL
    > >  log4j.logger.org.apache.hadoop.hive.ql.exec.FunctionRegistry=ERROR
    > > +
    > > +# SPARK-14754: Metrics as logs are not coming through slf4j.
    > > +#log4j.logger.org.apache.spark.metrics=INFO, metricFileAppender
    > > +#log4j.additivity.org.apache.spark.metrics=true
    > > +
    > > +#log4j.appender.metricFileAppender=org.apache.log4j.RollingFileAppender
    > > +#log4j.appender.metricFileAppender.File=${logFilePath}
    > > +#log4j.appender.metricFileAppender.MaxFileSize=10MB
    >
    > I still don't think this has been addressed. It shouldn't need a new
    > appender, right?
    >
    > —
    > You are receiving this because you authored the thread.
    > Reply to this email directly or view it on GitHub
    > <https://github.com/apache/spark/pull/12697/files/5a437a12fb0c6cfdd7119d63bebc3f3f2c935d5f#r61066147>
    >
    
    
    
    -- 
    Mihir Monani
    (+91)-9429473434



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14754][SPARK CORE] Metrics as logs are ...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12697#discussion_r61053629
  
    --- Diff: core/src/main/scala/org/apache/spark/metrics/sink/Slf4jSink.scala ---
    @@ -25,6 +25,9 @@ import com.codahale.metrics.{MetricRegistry, Slf4jReporter}
     import org.apache.spark.SecurityManager
     import org.apache.spark.metrics.MetricsSystem
     
    +import org.slf4j.Logger
    +import org.slf4j.LoggerFactory
    --- End diff --
    
    No you simply push more commits to this branch


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14754][SPARK CORE] Metrics as logs are ...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12697#discussion_r61048906
  
    --- Diff: core/src/main/scala/org/apache/spark/metrics/sink/Slf4jSink.scala ---
    @@ -25,6 +25,9 @@ import com.codahale.metrics.{MetricRegistry, Slf4jReporter}
     import org.apache.spark.SecurityManager
     import org.apache.spark.metrics.MetricsSystem
     
    +import org.slf4j.Logger
    +import org.slf4j.LoggerFactory
    --- End diff --
    
    I think we should reorder imports. (Please see [databricks/scala-style-guide#imports](https://github.com/databricks/scala-style-guide#imports).)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14754][SPARK CORE] Metrics as logs are ...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12697#discussion_r61048386
  
    --- Diff: core/src/main/scala/org/apache/spark/metrics/sink/Slf4jSink.scala ---
    @@ -49,6 +52,7 @@ private[spark] class Slf4jSink(
       MetricsSystem.checkMinimalPollingPeriod(pollUnit, pollPeriod)
     
       val reporter: Slf4jReporter = Slf4jReporter.forRegistry(registry)
    +    .outputTo(LoggerFactory.getLogger("org.apache.spark.metrics"))
    --- End diff --
    
    Why not the class name for the logger name here, per your JIRA?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14754][SPARK CORE] Metrics as logs are ...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12697#discussion_r61053012
  
    --- Diff: core/src/main/scala/org/apache/spark/metrics/sink/Slf4jSink.scala ---
    @@ -25,6 +25,9 @@ import com.codahale.metrics.{MetricRegistry, Slf4jReporter}
     import org.apache.spark.SecurityManager
     import org.apache.spark.metrics.MetricsSystem
     
    +import org.slf4j.Logger
    +import org.slf4j.LoggerFactory
    --- End diff --
    
    The problem you describe is that log messages aren't plumbed through to a logger, right? that shouldn't require changing appender config for log4j. Your JIRA says "Slf4jsink.scala should have class name for log4j to print metrics in log files" but your code has a package name.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14754][SPARK CORE] Metrics as logs are ...

Posted by mihir6692 <gi...@git.apache.org>.
Github user mihir6692 commented on the pull request:

    https://github.com/apache/spark/pull/12697#issuecomment-214882784
  
    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.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: [SPARK-14754][SPARK CORE] Metrics as logs are ...

Posted by mihir6692 <gi...@git.apache.org>.
Github user mihir6692 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/12697#discussion_r61052655
  
    --- Diff: core/src/main/scala/org/apache/spark/metrics/sink/Slf4jSink.scala ---
    @@ -25,6 +25,9 @@ import com.codahale.metrics.{MetricRegistry, Slf4jReporter}
     import org.apache.spark.SecurityManager
     import org.apache.spark.metrics.MetricsSystem
     
    +import org.slf4j.Logger
    +import org.slf4j.LoggerFactory
    --- End diff --
    
    I will run "sbt scalastyle" before adding some commits in future.
    
    Regarding *log4j.properties.template* changes : -
    
    I have added configuration template for metrics as log in
    *log4j.properties.template,
    *so other user doesn't face problem.
    
    To Sean Owen :-
    
    I don't understand what you meant by "Why not the class name for the logger
    name here, per your JIRA?" ??
    
    I gave generalize name for class as i didnt found any convention for it.
    
    
    On Tue, Apr 26, 2016 at 2:37 PM, Hyukjin Kwon <no...@github.com>
    wrote:
    
    > In core/src/main/scala/org/apache/spark/metrics/sink/Slf4jSink.scala
    > <https://github.com/apache/spark/pull/12697#discussion_r61051439>:
    >
    > > @@ -25,6 +25,9 @@ import com.codahale.metrics.{MetricRegistry, Slf4jReporter}
    > >  import org.apache.spark.SecurityManager
    > >  import org.apache.spark.metrics.MetricsSystem
    > >
    > > +import org.slf4j.Logger
    > > +import org.slf4j.LoggerFactory
    >
    > (It would be great if you run sbt scalastyle before adding some more
    > commits which shows up such things)
    >
    > —
    > You are receiving this because you authored the thread.
    > Reply to this email directly or view it on GitHub
    > <https://github.com/apache/spark/pull/12697/files/5a437a12fb0c6cfdd7119d63bebc3f3f2c935d5f#r61051439>
    >
    
    
    
    -- 
    Mihir Monani
    (+91)-9429473434



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #12697: [SPARK-14754][SPARK CORE] Metrics as logs are not coming...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/12697
  
    gentle ping @mihir6692


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org