Solr
  1. Solr
  2. SOLR-1716

Add logging support for ScriptTransformer

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: 4.9, 5.0
    • Labels:
      None

      Description

      Currently it's very hard to debug the logic embedded in the script ran by the ScriptTransformer. There should be a possibility to add a logger to the function signature, which can be used for logging.

      1. SOLR-1716.patch
        9 kB
        Uri Boness
      2. SOLR-1716.patch
        10 kB
        Uri Boness

        Activity

        Hide
        Uri Boness added a comment -

        Basic implementation

        Show
        Uri Boness added a comment - Basic implementation
        Hide
        Erik Hatcher added a comment -

        Couldn't this be done with some kind of script context global or something, rather than adding a logger to the method signature?

        And if it is added to the signature, will this work with methods without it too?

        Show
        Erik Hatcher added a comment - Couldn't this be done with some kind of script context global or something, rather than adding a logger to the method signature? And if it is added to the signature, will this work with methods without it too?
        Hide
        Uri Boness added a comment -

        yeah, I thought about the global context as well, but this was just something that I implemented anyway (as I needed it myself) and it works. You don't have to supply the logger, but if you do you need to specify the full method signature, that is:

        function(row, context, logger) {
        
        }
        

        but the following will work as well:

        function(row, context) {
        
        }
        

        and

        function(row) {
        
        }
        
        Show
        Uri Boness added a comment - yeah, I thought about the global context as well, but this was just something that I implemented anyway (as I needed it myself) and it works. You don't have to supply the logger, but if you do you need to specify the full method signature, that is: function(row, context, logger) { } but the following will work as well: function(row, context) { } and function(row) { }
        Hide
        Erik Hatcher added a comment -

        good enough for me, with the flexible signature handling.

        Show
        Erik Hatcher added a comment - good enough for me, with the flexible signature handling.
        Hide
        Uri Boness added a comment -

        working on a new patch to put the logging in a global context (and cleaning up the code a bit)

        Show
        Uri Boness added a comment - working on a new patch to put the logging in a global context (and cleaning up the code a bit)
        Hide
        Uri Boness added a comment -

        This patch puts the logger in the global scope, so you don't have to specify the logger as part of the function signature. It also cleans up the code the related classes a bit.

        Show
        Uri Boness added a comment - This patch puts the logger in the global scope, so you don't have to specify the logger as part of the function signature. It also cleans up the code the related classes a bit.
        Hide
        Uri Boness added a comment -

        There is still one thing to improve here. Right now a ScriptTransformer is created for each function in the script, which means an engine is created for each function. This can be optimized by creating one script engine per EntityProcessor which each ScriptTransformer will use to execute a dedicated function.

        Show
        Uri Boness added a comment - There is still one thing to improve here. Right now a ScriptTransformer is created for each function in the script, which means an engine is created for each function. This can be optimized by creating one script engine per EntityProcessor which each ScriptTransformer will use to execute a dedicated function.
        Hide
        Michael Henson added a comment -

        Is this intended to replace the Rhino built-in "print()" function? I've used that to successfully debug script transformers recently.

        Show
        Michael Henson added a comment - Is this intended to replace the Rhino built-in "print()" function? I've used that to successfully debug script transformers recently.
        Hide
        Lance Norskog added a comment -

        Is this intended to replace the Rhino built-in "print()" function? I've used that to successfully debug script transformers recently.

        ScriptTransformer supports Rhino,, Groove, Scala, etc. Someday even Erjang. They probably all have such things, but a consistent debugging allows better debugging tools.

        Show
        Lance Norskog added a comment - Is this intended to replace the Rhino built-in "print()" function? I've used that to successfully debug script transformers recently. ScriptTransformer supports Rhino,, Groove, Scala, etc. Someday even Erjang . They probably all have such things, but a consistent debugging allows better debugging tools.
        Hide
        Uri Boness added a comment -

        ScriptTransformer supports Rhino,, Groove, Scala, etc. Someday even Erjang. They probably all have such things, but a consistent debugging allows better debugging tools.

        True. Using the logger will print to the same log Solr log files. Indeed it's great for debugging, but also when you have fancy complex logic in the scripts general purpose logging (e.g. INFO, ERROR, TRACE) should also be considered.

        Show
        Uri Boness added a comment - ScriptTransformer supports Rhino,, Groove, Scala, etc. Someday even Erjang. They probably all have such things, but a consistent debugging allows better debugging tools. True. Using the logger will print to the same log Solr log files. Indeed it's great for debugging, but also when you have fancy complex logic in the scripts general purpose logging (e.g. INFO, ERROR, TRACE) should also be considered.
        Hide
        Hoss Man added a comment -

        Bulk updating 240 Solr issues to set the Fix Version to "next" per the process outlined in this email...

        http://mail-archives.apache.org/mod_mbox/lucene-dev/201005.mbox/%3Calpine.DEB.1.10.1005251052040.24672@radix.cryptio.net%3E

        Selection criteria was "Unresolved" with a Fix Version of 1.5, 1.6, 3.1, or 4.0. email notifications were suppressed.

        A unique token for finding these 240 issues in the future: hossversioncleanup20100527

        Show
        Hoss Man added a comment - Bulk updating 240 Solr issues to set the Fix Version to "next" per the process outlined in this email... http://mail-archives.apache.org/mod_mbox/lucene-dev/201005.mbox/%3Calpine.DEB.1.10.1005251052040.24672@radix.cryptio.net%3E Selection criteria was "Unresolved" with a Fix Version of 1.5, 1.6, 3.1, or 4.0. email notifications were suppressed. A unique token for finding these 240 issues in the future: hossversioncleanup20100527
        Hide
        Robert Muir added a comment -

        Bulk move 3.2 -> 3.3

        Show
        Robert Muir added a comment - Bulk move 3.2 -> 3.3
        Hide
        Robert Muir added a comment -

        3.4 -> 3.5

        Show
        Robert Muir added a comment - 3.4 -> 3.5
        Hide
        Hoss Man added a comment -

        Bulk of fixVersion=3.6 -> fixVersion=4.0 for issues that have no assignee and have not been updated recently.

        email notification suppressed to prevent mass-spam
        psuedo-unique token identifying these issues: hoss20120321nofix36

        Show
        Hoss Man added a comment - Bulk of fixVersion=3.6 -> fixVersion=4.0 for issues that have no assignee and have not been updated recently. email notification suppressed to prevent mass-spam psuedo-unique token identifying these issues: hoss20120321nofix36
        Hide
        Steve Rowe added a comment -

        Bulk move 4.4 issues to 4.5 and 5.0

        Show
        Steve Rowe added a comment - Bulk move 4.4 issues to 4.5 and 5.0
        Hide
        Uwe Schindler added a comment -

        Move issue to Solr 4.9.

        Show
        Uwe Schindler added a comment - Move issue to Solr 4.9.

          People

          • Assignee:
            Unassigned
            Reporter:
            Uri Boness
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:

              Development