Maven
  1. Maven
  2. MNG-2727

Fix Logging in threadsafe components

    Details

    • Type: Task Task
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Won't Fix
    • Affects Version/s: None
    • Fix Version/s: 3.x / Backlog
    • Component/s: Embedding
    • Labels:
      None
    • Complexity:
      Intermediate
    • Number of attachments :
      0

      Activity

      Hide
      Jason van Zyl added a comment -

      We have a general problem with plexus components that are singletons in that they use
      he same logger for their lifespan. This is not good in that many requests may be fired
      off and the singleton plexus component will continue to funnel their output to the same
      logger. We need to be able to swap the logger.

      Show
      Jason van Zyl added a comment - We have a general problem with plexus components that are singletons in that they use he same logger for their lifespan. This is not good in that many requests may be fired off and the singleton plexus component will continue to funnel their output to the same logger. We need to be able to swap the logger.
      Hide
      Trygve Laugstøl added a comment -

      Shouldn't the component use a monitor instead of logging in the cases where the output is useful on a per-build basis?

      Show
      Trygve Laugstøl added a comment - Shouldn't the component use a monitor instead of logging in the cases where the output is useful on a per-build basis?
      Hide
      Jason van Zyl added a comment -

      If it's relavent to the API of the application yes, if you want debug output or general tracing then this falls outside the monitor. Though this is a general problem that would be solved using a more direct use of log4j and it's NDC facilities.

      Show
      Jason van Zyl added a comment - If it's relavent to the API of the application yes, if you want debug output or general tracing then this falls outside the monitor. Though this is a general problem that would be solved using a more direct use of log4j and it's NDC facilities.
      Hide
      Geoffrey De Smet added a comment -

      log4j has been unchanged (=dead?) for 4 years.
      It's creator created slf4j with logback (AKA log4j 2), similar license, much better code, still maintained, but due to politics it never got the name "log4j 2".

      Show
      Geoffrey De Smet added a comment - log4j has been unchanged (=dead?) for 4 years. It's creator created slf4j with logback (AKA log4j 2), similar license, much better code, still maintained, but due to politics it never got the name "log4j 2".
      Hide
      jieryn added a comment -

      Maven should use its own logging facade which then is backed by logback (which supports NDC). Then if Geki creates something even better down the road we can swap it under the covers without any issue.

      See Spring Framework for the devastation depending on a specific logging mechanism can do.

      Show
      jieryn added a comment - Maven should use its own logging facade which then is backed by logback (which supports NDC). Then if Geki creates something even better down the road we can swap it under the covers without any issue. See Spring Framework for the devastation depending on a specific logging mechanism can do.
      Hide
      Ceki Gulcu added a comment -

      FYI, SLF4J supports MDC but not NDC. There are counter arguments for making SLF4J an optional dependency: see http://slf4j.org/faq.html#optional_dependency

      However, whether you choose to depend on SLF4J or go through your own facade, I am fairly certain that neither approach will be future proof.

      Show
      Ceki Gulcu added a comment - FYI, SLF4J supports MDC but not NDC. There are counter arguments for making SLF4J an optional dependency: see http://slf4j.org/faq.html#optional_dependency However, whether you choose to depend on SLF4J or go through your own facade, I am fairly certain that neither approach will be future proof.
      Hide
      Kristian Rosenvold added a comment - - edited

      I am a bit unsure of exactly which application use-case is being addressed in this issue;

      For the current concurrency effort (MNG-3004) my current favourite strategy is replacing the LoggerManager with a ConcurrentLoggerManager that
      provides a MavenProject-aware proxy for the existing logger subsystem. Such a proxy would typically be able to collect all logger output for
      the project to a single outputstream, no matter how this intertwines with other builds.

      A rough draft of this logic can be seen in LifecycleThreadedBuilder line 164 & 167, where the MavenProject is associated with the current thread (Code is commented out because the current implementation is not good enough). This association changes logger output, and the setThisModuleComplete releases the output.

      I fully sense that there may be some other use cases, possibly related to embedding. But it seems like a solution that captures the "logical stream" that constitutes the output from a single build should be expandable to these different cases.

      Show
      Kristian Rosenvold added a comment - - edited I am a bit unsure of exactly which application use-case is being addressed in this issue; For the current concurrency effort ( MNG-3004 ) my current favourite strategy is replacing the LoggerManager with a ConcurrentLoggerManager that provides a MavenProject-aware proxy for the existing logger subsystem. Such a proxy would typically be able to collect all logger output for the project to a single outputstream, no matter how this intertwines with other builds. A rough draft of this logic can be seen in LifecycleThreadedBuilder line 164 & 167, where the MavenProject is associated with the current thread (Code is commented out because the current implementation is not good enough). This association changes logger output, and the setThisModuleComplete releases the output. I fully sense that there may be some other use cases, possibly related to embedding. But it seems like a solution that captures the "logical stream" that constitutes the output from a single build should be expandable to these different cases.
      Hide
      Jason van Zyl added a comment -

      Stuart is taking care of this as part of moving over to Guice. We just need to inject a threadsafe logger. There's no point in doing this in Plexus and in Guice.

      Show
      Jason van Zyl added a comment - Stuart is taking care of this as part of moving over to Guice. We just need to inject a threadsafe logger. There's no point in doing this in Plexus and in Guice.
      Hide
      Brett Porter added a comment -

      since the switch to Guice happened, is there anything still left to do here, or was it taken care of as suggested in the last comment?

      Show
      Brett Porter added a comment - since the switch to Guice happened, is there anything still left to do here, or was it taken care of as suggested in the last comment?
      Hide
      Kristian Rosenvold added a comment -

      I'm not really sure we need to do anything more about this issue, the 3.x logging seems to work fairly well in parallel.

      Show
      Kristian Rosenvold added a comment - I'm not really sure we need to do anything more about this issue, the 3.x logging seems to work fairly well in parallel.
      Hide
      Michael Osipov added a comment -

      Please refer to https://cwiki.apache.org/confluence/display/MAVEN/The+Great+JIRA+Cleanup+of+2014 if you're wondering why this issue was closed out.

      Assignee, if you think you can fix this bug anytime soon, please reopen and proceed appropriately.

      Show
      Michael Osipov added a comment - Please refer to https://cwiki.apache.org/confluence/display/MAVEN/The+Great+JIRA+Cleanup+of+2014 if you're wondering why this issue was closed out. Assignee, if you think you can fix this bug anytime soon, please reopen and proceed appropriately.

        People

        • Assignee:
          Jason van Zyl
          Reporter:
          Jason van Zyl
        • Votes:
          4 Vote for this issue
          Watchers:
          5 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved: