Skip to content
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

Support println in Painless #20263

Closed
pickypg opened this issue Aug 31, 2016 · 12 comments

Comments

Projects
None yet
9 participants
@pickypg
Copy link
Member

commented Aug 31, 2016

Without a full debugger, the fallback approach for Painless is currently to throw exceptions with messages that the scripting engine has to catch. This can be limiting without planning (maintaining a string and throwing at the end) ahead since it's one-and-done.

It would be convenient to support println as a pass-thru to System.out (if not a the internal logger, but that touches the file system and therefore may need extra permissions, which creates different risks).

@nik9000

This comment has been minimized.

Copy link
Contributor

commented Aug 31, 2016

Having some way to get messages back to the user from an embedded scripting languages isn't without precedent. I'd prefer dumping the results to a logger and/or throwing them back at the user.

@ppf2

This comment has been minimized.

Copy link
Member

commented Aug 31, 2016

Plus it's will be extremely helpful for debugging Watcher scripts :)

@geekpete

This comment has been minimized.

Copy link
Member

commented Sep 1, 2016

How about provide a logging facility but for safety just log to an elasticsearch index.

This would allow other cool use cases such as alerting/monitoring on script activity or custom output.

Imagine you want to do profiling of a script as part of a larger overall pipeline monitoring effort, this would allow you the granularity of APM type detail from within your script.

@s1monw

This comment has been minimized.

Copy link
Contributor

commented Sep 1, 2016

I don't think the script should do ANY IO whatsoever. I wonder if we should have a size limited logger buffer that we can log out after the script is executed? I also think it should be disabled by default and only enabled if a flag is passed to the script compilation such that we optimize it away in the common case

@rjernst

This comment has been minimized.

Copy link
Member

commented Sep 1, 2016

I agree we should not do any IO. I think exceptions are the way to go for debugging. They are immediate, and don't require pouring through a log file.

@nik9000

This comment has been minimized.

Copy link
Contributor

commented Sep 1, 2016

I like the size limited buffer returned to the user. Using a compilation flag to skip it entirely is cool I think as well.

I think this is lower priority than the assert proposal because asserts are so very nice and immediate.

@pickypg

This comment has been minimized.

Copy link
Member Author

commented Sep 1, 2016

They are immediate, and don't require pouring through a log file.

In practice, I find that people aren't actually using the log file, so much as the standard out aspect of it as they make attempts at refinement/correction or they're tailing the log to get the same effect.

As such, I really like the size-limited buffer idea. Perhaps we only expose the logger to scripts that explicitly request it via a request-level parameter?

"script": {
  "inline": "logger.info('blah')",
  "lang": "painless",
  "logger": "true"
}
@nik9000

This comment has been minimized.

Copy link
Contributor

commented Sep 1, 2016

Perhaps we only expose the logger to scripts that explicitly request it via a request-level parameter?

We could make it a noop if "logger": false so people can use the same scripts and not pay the cost when they aren't debugging.

@jdconrad

This comment has been minimized.

Copy link
Contributor

commented Sep 1, 2016

I lie somewhere between @s1monw and @rjernst positions on this. I would like to provide a better debug environment for some of the more complex scripts, but at the same time I'm hesitant to allow a user to do any kind of logging even to just a buffer given how easy it is to abuse something like this.

@clintongormley clintongormley removed the discuss label Oct 7, 2016

nik9000 added a commit to nik9000/elasticsearch that referenced this issue Nov 21, 2016

Whitelist Class in painless and document it
This whitelists `java.lang.Class`, `Class#getName` and
`Class#getSimpleName` and documents using `getName` to find the
class you are working with.

Closes elastic#20263

nik9000 added a commit to nik9000/elasticsearch that referenced this issue Nov 22, 2016

Add Debug.explain to painless
You can use `Debug.explain(someObject)` in painless to throw an
`Error` that can't be caught by painless code and contains an
object's class. This is useful because painless's sandbox doesn't
allow you to call `someObject.getClass()`.

Closes elastic#20263

nik9000 added a commit that referenced this issue Nov 22, 2016

Add Debug.explain to painless
You can use `Debug.explain(someObject)` in painless to throw an
`Error` that can't be caught by painless code and contains an
object's class. This is useful because painless's sandbox doesn't
allow you to call `someObject.getClass()`.

Closes #20263
@shivamkrpandey

This comment has been minimized.

Copy link

commented Apr 2, 2018

I have the below script that I'm trying to execute to update a document. But Debug.explain() doesn't seem to be working. Any insights into this would be appreciated. My Elasticsearch version is 5.5.0.

Script script = new Script(
      Script.DEFAULT_SCRIPT_TYPE,
      Script.DEFAULT_SCRIPT_LANG,
      """
            Object details = ctx._source.personDetails;
            Debug.explain(details);   // This is not working

            // some other code

            return details;
       """,
       Collections.emptyMap()
 )

esClient.prepareUpdate(index, type, documentId).setScript(script).get()

Thanks!

@rjernst

This comment has been minimized.

Copy link
Member

commented Apr 2, 2018

@shivamcausecode Please ask questions on our forum. Closed issues are not normally tracked by developers and your comments can easily be overlooked.

@shivamkrpandey

This comment has been minimized.

Copy link

commented Apr 3, 2018

@rjernst Done, thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.