You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "ayesheepatra07 (via GitHub)" <gi...@apache.org> on 2025/04/08 01:56:36 UTC
[PR] Improve logging for groovy transform functions [pinot]
ayesheepatra07 opened a new pull request, #15483:
URL: https://github.com/apache/pinot/pull/15483
This change enhances error handling and display in Groovy transformation functions. When a Groovy script fails to compile, the error is logged into the controller logs and passed through the call stack.
Changes -
Wrapped the call (createSafeShell(_binding).parse(scriptText))to compile the Groovy script in a try-catch block. When a compilation error occurs, the exception is now:
- Logged using LOGGER.error, including the full stack trace and original error message.
- Rethrown as an IllegalStateException with a message that includes the original exception’s message from the compiler.
For a invalid groovy script like:
Groovy({def slrpr= new groovy.json.JsonSlurper(); def input = slrpr.parseText(opens); def df = new SimpleDateFormat(\"yyyy-MM-dd'T'HH:mm:ss'Z'\"); input.collect(df.parse(it.created_at[$date]).getTime())}, opens)
An error message is displayed like:
“ERROR [GroovyFunctionEvaluator] [grizzly-http-server-22] Groovy compilation error: Groovy({def slrpr= new groovy.json.JsonSlurper(); def input = slrpr.parseText(opens); def df = new SimpleDateFormat(\"yyyy-MM-dd'T'HH:mm:ss'Z'\"); input.collect(df.parse(it.created_at[$date]).getTime())}, opens) org.codehaus.groovy.control.MultipleCompilationErrorsException: startup failed: Script1.groovy: 1: unexpected char: '\' @ line 1, column 109.
def df = new SimpleDateFormat(\"yyyy-MM-
^
1 error”
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Improve logging for groovy transform functions [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #15483:
URL: https://github.com/apache/pinot/pull/15483#discussion_r2033972227
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/function/GroovyFunctionEvaluator.java:
##########
@@ -83,7 +83,12 @@ public GroovyFunctionEvaluator(String closure) {
_numArguments = _arguments.size();
_binding = new Binding();
String scriptText = matcher.group(SCRIPT_GROUP_NAME);
- _script = createSafeShell(_binding).parse(scriptText);
+ try {
+ _script = createSafeShell(_binding).parse(scriptText);
+ } catch (Exception e) {
+ LOGGER.error("Groovy compilation error: " + closure, e);
Review Comment:
We don't usually log error if exception is thrown out, especially for user error
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/function/FunctionEvaluatorFactory.java:
##########
@@ -62,7 +62,7 @@ public static FunctionEvaluator getExpressionEvaluator(FieldSpec fieldSpec) {
} catch (Exception e) {
throw new IllegalStateException(
"Caught exception while constructing expression evaluator for transform expression:" + transformExpression
- + ", of column:" + columnName);
+ + ", of column:" + columnName + e.getMessage(), e);
Review Comment:
```suggestion
+ ", of column: " + columnName + ", exception: " + e.getMessage(), e);
```
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java:
##########
@@ -538,7 +538,8 @@ public static void validateIngestionConfig(TableConfig tableConfig, Schema schem
expressionEvaluator = FunctionEvaluatorFactory.getExpressionEvaluator(transformFunction);
} catch (Exception e) {
throw new IllegalStateException(
- "Invalid transform function '" + transformFunction + "' for column '" + columnName + "'", e);
+ "Invalid transform function '" + transformFunction + "' for column '" + columnName
+ + "'" + e.getMessage(), e);
Review Comment:
```suggestion
+ "', exception: " + e.getMessage(), e);
```
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java:
##########
@@ -377,7 +377,7 @@ public static void validateIngestionConfig(TableConfig tableConfig, Schema schem
try {
FunctionEvaluatorFactory.getExpressionEvaluator(filterFunction);
} catch (Exception e) {
- throw new IllegalStateException("Invalid filter function " + filterFunction, e);
+ throw new IllegalStateException("Invalid filter function " + filterFunction + e.getMessage(), e);
Review Comment:
```suggestion
throw new IllegalStateException("Invalid filter function: " + filterFunction + ", exception: " + e.getMessage(), e);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Improve logging for groovy transform functions [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #15483:
URL: https://github.com/apache/pinot/pull/15483
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Improve logging for groovy transform functions [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #15483:
URL: https://github.com/apache/pinot/pull/15483#issuecomment-2785079637
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/15483?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `75.00000%` with `2 lines` in your changes missing coverage. Please review.
> Project coverage is 55.85%. Comparing base [(`59551e4`)](https://app.codecov.io/gh/apache/pinot/commit/59551e45224f1535c4863fd577622b37366ccc97?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`29f5366`)](https://app.codecov.io/gh/apache/pinot/commit/29f5366b2a326a7803953430a793948152999f68?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 2011 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/pinot/pull/15483?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...he/pinot/segment/local/utils/TableConfigUtils.java](https://app.codecov.io/gh/apache/pinot/pull/15483?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Futils%2FTableConfigUtils.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91dGlscy9UYWJsZUNvbmZpZ1V0aWxzLmphdmE=) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/15483?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (59551e4) and HEAD (29f5366). Click for more details.
>
> <details><summary>HEAD has 53 uploads less than BASE</summary>
>
>| Flag | BASE (59551e4) | HEAD (29f5366) |
>|------|------|------|
>|integration|7|0|
>|integration2|3|0|
>|temurin|12|1|
>|java-21|7|1|
>|skip-bytebuffers-true|3|0|
>|skip-bytebuffers-false|7|1|
>|unittests|5|1|
>|unittests1|2|1|
>|java-11|5|0|
>|unittests2|3|0|
>|integration1|2|0|
>|custom-integration1|2|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #15483 +/- ##
============================================
- Coverage 61.75% 55.85% -5.90%
- Complexity 207 681 +474
============================================
Files 2436 2223 -213
Lines 133233 120064 -13169
Branches 20636 19184 -1452
============================================
- Hits 82274 67066 -15208
- Misses 44911 47463 +2552
+ Partials 6048 5535 -513
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/15483/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/15483/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/15483/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/15483/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/15483/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/15483/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/15483/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.85% <75.00%> (-5.77%)` | :arrow_down: |
| [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/15483/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.85% <75.00%> (-5.89%)` | :arrow_down: |
| [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/15483/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/15483/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.85% <75.00%> (-5.90%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/15483/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.85% <75.00%> (-5.89%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/15483/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.85% <75.00%> (+8.96%)` | :arrow_up: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/15483/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/15483?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org