You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nifi.apache.org by rickysaltzer <gi...@git.apache.org> on 2016/03/16 22:19:06 UTC
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
GitHub user rickysaltzer opened a pull request:
https://github.com/apache/nifi/pull/285
NIFI-1636: Print Stacktrace When Unepected OnTrigger Exception Caught
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/rickysaltzer/nifi NIFI-1636
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/285.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 #285
----
commit e82464694435ee2e5f27cfbb00c0e6d7b53d89ce
Author: ricky <ri...@cloudera.com>
Date: 2016-03-16T20:43:05Z
NIFI-1636: Print Stacktrace When Unepected OnTrigger Exception Caught
----
---
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.
---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
Posted by rickysaltzer <gi...@git.apache.org>.
Github user rickysaltzer commented on a diff in the pull request:
https://github.com/apache/nifi/pull/285#discussion_r56421131
--- Diff: nifi-api/src/main/java/org/apache/nifi/processor/AbstractProcessor.java ---
@@ -27,7 +30,12 @@ public final void onTrigger(final ProcessContext context, final ProcessSessionFa
onTrigger(context, session);
session.commit();
} catch (final Throwable t) {
- getLogger().error("{} failed to process due to {}; rolling back session", new Object[]{this, t});
+ StringWriter stacktraceWriter = new StringWriter();
--- End diff --
Yep that would be the easiest way, and I can go ahead and throw this mess I added in above.
---
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.
---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unexpected OnT...
Posted by rickysaltzer <gi...@git.apache.org>.
Github user rickysaltzer commented on the pull request:
https://github.com/apache/nifi/pull/285#issuecomment-201317890
Thanks, @olegz! Thankfully that wasn't the commit message :). I took a look at NIFI-1447, and I believe this could be what I'm looking for. I understand the concern for overly verbose logging in the case of a FlowFile getting stuck in some sort of infinite loop.
Regarding NIFI-1447, are uncaught stacktraces logged at `ERROR` level?
---
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.
---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
Posted by olegz <gi...@git.apache.org>.
Github user olegz commented on the pull request:
https://github.com/apache/nifi/pull/285#issuecomment-200557146
Ricky
Could you please take a look at https://issues.apache.org/jira/browse/NIFI-1447 and see if it's what you are looking for? If I remember correctly we don't really want to log stack traces unless we're in DEBUG mode and that is what the above JIRA addresses.
---
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.
---
[GitHub] nifi issue #285: NIFI-1636: Print Stacktrace When Unexpected OnTrigger Excep...
Posted by apiri <gi...@git.apache.org>.
Github user apiri commented on the issue:
https://github.com/apache/nifi/pull/285
@rickysaltzer Was combing over PRs in rounding up items for 0.7.0. Curious if the effort highlighted by Oleg makes this particular PR OBE. 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.
---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
Posted by rickysaltzer <gi...@git.apache.org>.
Github user rickysaltzer commented on a diff in the pull request:
https://github.com/apache/nifi/pull/285#discussion_r56418891
--- Diff: nifi-api/src/main/java/org/apache/nifi/processor/AbstractProcessor.java ---
@@ -27,7 +30,12 @@ public final void onTrigger(final ProcessContext context, final ProcessSessionFa
onTrigger(context, session);
session.commit();
} catch (final Throwable t) {
- getLogger().error("{} failed to process due to {}; rolling back session", new Object[]{this, t});
+ StringWriter stacktraceWriter = new StringWriter();
--- End diff --
That makes sense @joewitt. Should legitimate unexpected exceptions be surfaced to `INFO` by default? I feel like these are uncommon enough to warrant it, but I could be wrong.
---
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.
---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
Posted by rickysaltzer <gi...@git.apache.org>.
Github user rickysaltzer commented on the pull request:
https://github.com/apache/nifi/pull/285#issuecomment-197998896
@joewitt posted a new version of the patch. I tested it out on my local machine and it seems to work as expected. That is, the stacktrace is logged to the `nifi-app.log`, while the the shortened version is logged to the UI. The `contrib-check` also checks out ;)
---
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.
---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
Posted by joewitt <gi...@git.apache.org>.
Github user joewitt commented on a diff in the pull request:
https://github.com/apache/nifi/pull/285#discussion_r56419500
--- Diff: nifi-api/src/main/java/org/apache/nifi/processor/AbstractProcessor.java ---
@@ -27,7 +30,12 @@ public final void onTrigger(final ProcessContext context, final ProcessSessionFa
onTrigger(context, session);
session.commit();
} catch (final Throwable t) {
- getLogger().error("{} failed to process due to {}; rolling back session", new Object[]{this, t});
+ StringWriter stacktraceWriter = new StringWriter();
--- End diff --
I would say generally no. They are uncommon in general but when they happen they happen in bursts. As a general rule I think we should strive to make what the user sees be something a lot more friendly than a wild stack trace. But, the logs should have them. We did discuss this on dev list a while back and there were some great inputs from various folks. We def need to do something here.
---
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.
---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
Posted by joewitt <gi...@git.apache.org>.
Github user joewitt commented on a diff in the pull request:
https://github.com/apache/nifi/pull/285#discussion_r56418107
--- Diff: nifi-api/src/main/java/org/apache/nifi/processor/AbstractProcessor.java ---
@@ -27,7 +30,12 @@ public final void onTrigger(final ProcessContext context, final ProcessSessionFa
onTrigger(context, session);
session.commit();
} catch (final Throwable t) {
- getLogger().error("{} failed to process due to {}; rolling back session", new Object[]{this, t});
+ StringWriter stacktraceWriter = new StringWriter();
--- End diff --
This logic needs to be in the logger and that is its intent. You will note that the throwable is being passed into it. The stack traces are getting logged when debug level is engaged for a given logger. This allows the user to control whether they are generated or not. That said...it is clearly counterintuitive and we need to improve.
---
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.
---
[GitHub] nifi issue #285: NIFI-1636: Print Stacktrace When Unexpected OnTrigger Excep...
Posted by trixpan <gi...@git.apache.org>.
Github user trixpan commented on the issue:
https://github.com/apache/nifi/pull/285
@rickysaltzer - is the PR still relevant? Would you mind if we close otherwise?
---
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.
---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
Posted by joewitt <gi...@git.apache.org>.
Github user joewitt commented on a diff in the pull request:
https://github.com/apache/nifi/pull/285#discussion_r56420919
--- Diff: nifi-api/src/main/java/org/apache/nifi/processor/AbstractProcessor.java ---
@@ -27,7 +30,12 @@ public final void onTrigger(final ProcessContext context, final ProcessSessionFa
onTrigger(context, session);
session.commit();
} catch (final Throwable t) {
- getLogger().error("{} failed to process due to {}; rolling back session", new Object[]{this, t});
+ StringWriter stacktraceWriter = new StringWriter();
--- End diff --
Right. So i think the short version is in the UI now. But you want the stacktrace too. We should make that available in the logs. And this class is what controls that I believe https://github.com/apache/nifi/blob/master/nifi-commons/nifi-logging-utils/src/main/java/org/apache/nifi/logging/NiFiLog.java You'll see it has some ifDebugEnabled guards. We should probably just toss those out. @markap14 what say you?
---
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.
---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
Posted by joewitt <gi...@git.apache.org>.
Github user joewitt commented on a diff in the pull request:
https://github.com/apache/nifi/pull/285#discussion_r56421212
--- Diff: nifi-api/src/main/java/org/apache/nifi/processor/AbstractProcessor.java ---
@@ -27,7 +30,12 @@ public final void onTrigger(final ProcessContext context, final ProcessSessionFa
onTrigger(context, session);
session.commit();
} catch (final Throwable t) {
- getLogger().error("{} failed to process due to {}; rolling back session", new Object[]{this, t});
+ StringWriter stacktraceWriter = new StringWriter();
--- End diff --
I should clarify...i agree with you this is confusing and not quite doing what we want. We need to fix it. Can you share a screenshot of what you see for a given error in the UI vs what shows in the logs and then let's agree on what it should ideally be and then figure out how to get there :-)
---
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.
---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
Posted by rickysaltzer <gi...@git.apache.org>.
Github user rickysaltzer commented on a diff in the pull request:
https://github.com/apache/nifi/pull/285#discussion_r56422806
--- Diff: nifi-api/src/main/java/org/apache/nifi/processor/AbstractProcessor.java ---
@@ -27,7 +30,12 @@ public final void onTrigger(final ProcessContext context, final ProcessSessionFa
onTrigger(context, session);
session.commit();
} catch (final Throwable t) {
- getLogger().error("{} failed to process due to {}; rolling back session", new Object[]{this, t});
+ StringWriter stacktraceWriter = new StringWriter();
--- End diff --
Sure thing, my battery just died so I will as soon as able. 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.
---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
Posted by rickysaltzer <gi...@git.apache.org>.
Github user rickysaltzer commented on a diff in the pull request:
https://github.com/apache/nifi/pull/285#discussion_r56420502
--- Diff: nifi-api/src/main/java/org/apache/nifi/processor/AbstractProcessor.java ---
@@ -27,7 +30,12 @@ public final void onTrigger(final ProcessContext context, final ProcessSessionFa
onTrigger(context, session);
session.commit();
} catch (final Throwable t) {
- getLogger().error("{} failed to process due to {}; rolling back session", new Object[]{this, t});
+ StringWriter stacktraceWriter = new StringWriter();
--- End diff --
So if I understand correctly, this happens now but only in `DEBUG` mode. What we would like to see is that behavior but even if we're in `INFO` mode. That is, shortened version logged to the UI and the stacktrace logged to the log file?
---
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.
---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
Posted by rickysaltzer <gi...@git.apache.org>.
Github user rickysaltzer commented on a diff in the pull request:
https://github.com/apache/nifi/pull/285#discussion_r56419796
--- Diff: nifi-api/src/main/java/org/apache/nifi/processor/AbstractProcessor.java ---
@@ -27,7 +30,12 @@ public final void onTrigger(final ProcessContext context, final ProcessSessionFa
onTrigger(context, session);
session.commit();
} catch (final Throwable t) {
- getLogger().error("{} failed to process due to {}; rolling back session", new Object[]{this, t});
+ StringWriter stacktraceWriter = new StringWriter();
--- End diff --
I wonder if it would make sense to surface the shorthand message to the user in the UI but log the stacktrace to the log file. I imagine this would take some re-working.
---
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.
---
[GitHub] nifi issue #285: NIFI-1636: Print Stacktrace When Unexpected OnTrigger Excep...
Posted by rickysaltzer <gi...@git.apache.org>.
Github user rickysaltzer commented on the issue:
https://github.com/apache/nifi/pull/285
@trixpan thanks for the reminder, closing...
---
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.
---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unexpected OnT...
Posted by olegz <gi...@git.apache.org>.
Github user olegz commented on the pull request:
https://github.com/apache/nifi/pull/285#issuecomment-211081495
@rickysaltzer where do you think we are with this one? I mean, do you think NIFI-1447 solved your issue or this PR is still valid?
---
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.
---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unexpected OnT...
Posted by olegz <gi...@git.apache.org>.
Github user olegz commented on the pull request:
https://github.com/apache/nifi/pull/285#issuecomment-205353808
They were logged at DEBUG only level and that was the issue since you would not be able to see them, but now if there is an error the stack trace will be printed
---
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.
---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
Posted by olegz <gi...@git.apache.org>.
Github user olegz commented on the pull request:
https://github.com/apache/nifi/pull/285#issuecomment-200557606
Also, you may want to amend the commit message "Unepected" unless its a new English word I haven't learned yet ;)
---
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.
---
[GitHub] nifi pull request: NIFI-1636: Print Stacktrace When Unepected OnTr...
Posted by joewitt <gi...@git.apache.org>.
Github user joewitt commented on a diff in the pull request:
https://github.com/apache/nifi/pull/285#discussion_r56420228
--- Diff: nifi-api/src/main/java/org/apache/nifi/processor/AbstractProcessor.java ---
@@ -27,7 +30,12 @@ public final void onTrigger(final ProcessContext context, final ProcessSessionFa
onTrigger(context, session);
session.commit();
} catch (final Throwable t) {
- getLogger().error("{} failed to process due to {}; rolling back session", new Object[]{this, t});
+ StringWriter stacktraceWriter = new StringWriter();
--- End diff --
that is actually the current behavior/intent. I think the change needed is to simply get rid of the 'if debug enabled' stuff we did in the logger and instead just always log the stacktrace when it is there. This is what Adam Taft had advocated for previously as I recall. I now see why he was saying it. So yeah in the UI it should be a short and ideally meaningful message (not always possible) and if a throwable shows up we should put the whole stack trace in the logs.
The current idea that a user will go in an do debug enabled....just has proven to be a failed experiment in my view and as someone who advocated for that I now think i was wrong.
---
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.
---
[GitHub] nifi pull request #285: NIFI-1636: Print Stacktrace When Unexpected OnTrigge...
Posted by rickysaltzer <gi...@git.apache.org>.
Github user rickysaltzer closed the pull request at:
https://github.com/apache/nifi/pull/285
---
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.
---