Skip to content

Allow multiple start states for a rule. Support nested Java exceptions. #52

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Feb 6, 2019
Merged

Allow multiple start states for a rule. Support nested Java exceptions. #52

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
adbac8e
Allow multiple start states for a rule. Support nested Java exceptions.
igorpeshansky Feb 6, 2019
bb348c3
Rename variable. Use proper indentation in test data.
igorpeshansky Feb 6, 2019
03a51a0
Handle Java logback as well.
igorpeshansky Feb 6, 2019
793032d
Be careful about line endings.
igorpeshansky Feb 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .rubocop.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ Metrics/AbcSize:
Metrics/ClassLength:
Max: 1300

# Offense count: 1
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the purpose of this configuration change on rubocop?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The module in exception_detector.rb became too large...

# Configuration parameters: CountComments.
Metrics/ModuleLength:
Max: 150

# Offense count: 3
Metrics/CyclomaticComplexity:
Max: 50
Expand Down
32 changes: 20 additions & 12 deletions lib/fluent/plugin/exception_detector.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
# limitations under the License.
#
module Fluent
Struct.new('Rule', :from_state, :pattern, :to_state)
Struct.new('Rule', :from_states, :pattern, :to_state)

# Configuration of the state machine that detects exceptions.
module ExceptionDetectorConfig
Expand Down Expand Up @@ -41,21 +41,28 @@ def state
end
end

def self.rule(from_state, pattern, to_state)
Struct::Rule.new(from_state, pattern, to_state)
def self.rule(from_state_or_states, pattern, to_state)
from_state_or_states = [from_state_or_states] unless
from_state_or_states.is_a?(Array)
Struct::Rule.new(from_state_or_states, pattern, to_state)
end

def self.supported
RULES_BY_LANG.keys
end

JAVA_RULES = [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Optional] Yesterday we chat about how we have more strict regex at client side than server side. Have we considered something looser? Like:

    JAVA_RULES = [
      rule(:start_state,
           /(?:Exception|Error|Throwable|V8 errors stack trace)[:\r\n]/,
           :java),
      rule(:java, /^[\t ]*nested exception is:[\t ]*$/, :java),
      rule(:java, /^$/, :java),
      rule(:java, /^[\t ]+(?:eval )?at /, :java),
      rule(:java, /^[\t ]*(?:Caused by|Suppressed):/, :java),
      rule(:java, /^[\t ]*... \d+\ more/, :java)
    ].freeze

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would miss parsing the nested exception itself after the nested exception is: line. That's why I've introduced :java_start_exception. I suppose we could simply add a transition from :java to :java_start_exception on nested exception is:, but that would not make the rules much simpler. WDYT?

rule(:start_state,
rule([:start_state, :java_start_exception],
/(?:Exception|Error|Throwable|V8 errors stack trace)[:\r\n]/,
:java),
rule(:java, /^[\t ]+(?:eval )?at /, :java),
rule(:java, /^[\t ]*(?:Caused by|Suppressed):/, :java),
rule(:java, /^[\t ]*... \d+\ more/, :java)
:java_after_exception),
rule(:java_after_exception, /^[\t ]*nested exception is:[\t ]*/,
:java_start_exception),
rule(:java_after_exception, /^[\r\n]*$/, :java_after_exception),
rule([:java_after_exception, :java], /^[\t ]+(?:eval )?at /, :java),
rule([:java_after_exception, :java], /^[\t ]*(?:Caused by|Suppressed):/,
:java_after_exception),
rule([:java_after_exception, :java],
/^[\t ]*... \d+ (?:more|common frames omitted)/, :java)
].freeze

PYTHON_RULES = [
Expand All @@ -76,13 +83,12 @@ def self.supported

GO_RULES = [
rule(:start_state, /\bpanic: /, :go_after_panic),
rule(:go_after_panic, /^$/, :go_goroutine),
rule([:go_after_panic, :go_after_signal, :go_frame_1],
/^$/, :go_goroutine),
rule(:go_after_panic, /^\[signal /, :go_after_signal),
rule(:go_after_signal, /^$/, :go_goroutine),
rule(:go_goroutine, /^goroutine \d+ \[[^\]]+\]:$/, :go_frame_1),
rule(:go_frame_1, /^(?:[^\s.:]+\.)*[^\s.():]+\(|^created by /,
:go_frame_2),
rule(:go_frame_1, /^$/, :go_goroutine),
rule(:go_frame_2, /^\s/, :go_frame_1)
].freeze

Expand Down Expand Up @@ -169,7 +175,9 @@ def initialize(*languages)
rule_config.each do |r|
target = ExceptionDetectorConfig::RuleTarget.new(r[:pattern],
r[:to_state])
@rules[r[:from_state]] << target
r[:from_states].each do |from_state|
@rules[from_state] << target
end
end
end

Expand Down
31 changes: 30 additions & 1 deletion test/plugin/test_exception_detector.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,35 @@ class ExceptionDetectorTest < Test::Unit::TestCase
at org.mortbay.jetty.servlet.ServletHolder.handle(ServletHolder.java:511)
at org.mortbay.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1166)
at com.example.myproject.OpenSessionInViewFilter.doFilter(OpenSessionInViewFilter.java:30)
... 27 more
... 27 common frames omitted
END

NESTED_JAVA_EXC = <<END.freeze
java.lang.RuntimeException: javax.mail.SendFailedException: Invalid Addresses;
nested exception is:
com.sun.mail.smtp.SMTPAddressFailedException: 550 5.7.1 <[REDACTED_EMAIL_ADDRESS]>... Relaying denied

at com.nethunt.crm.api.server.adminsync.AutomaticEmailFacade.sendWithSmtp(AutomaticEmailFacade.java:236)
at com.nethunt.crm.api.server.adminsync.AutomaticEmailFacade.sendSingleEmail(AutomaticEmailFacade.java:285)
at com.nethunt.crm.api.server.adminsync.AutomaticEmailFacade.lambda$sendSingleEmail$3(AutomaticEmailFacade.java:254)
at java.util.Optional.ifPresent(Optional.java:159)
at com.nethunt.crm.api.server.adminsync.AutomaticEmailFacade.sendSingleEmail(AutomaticEmailFacade.java:253)
at com.nethunt.crm.api.server.adminsync.AutomaticEmailFacade.sendSingleEmail(AutomaticEmailFacade.java:249)
at com.nethunt.crm.api.email.EmailSender.lambda$notifyPerson$0(EmailSender.java:80)
at com.nethunt.crm.api.util.ManagedExecutor.lambda$execute$0(ManagedExecutor.java:36)
at com.nethunt.crm.api.util.RequestContextActivator.lambda$withRequestContext$0(RequestContextActivator.java:36)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
at java.base/java.lang.Thread.run(Thread.java:748)
Caused by: javax.mail.SendFailedException: Invalid Addresses;
nested exception is:
com.sun.mail.smtp.SMTPAddressFailedException: 550 5.7.1 <[REDACTED_EMAIL_ADDRESS]>... Relaying denied

at com.sun.mail.smtp.SMTPTransport.rcptTo(SMTPTransport.java:2064)
at com.sun.mail.smtp.SMTPTransport.sendMessage(SMTPTransport.java:1286)
at com.nethunt.crm.api.server.adminsync.AutomaticEmailFacade.sendWithSmtp(AutomaticEmailFacade.java:229)
... 12 more
Caused by: com.sun.mail.smtp.SMTPAddressFailedException: 550 5.7.1 <[REDACTED_EMAIL_ADDRESS]>... Relaying denied
END

NODE_JS_EXC = <<END.freeze
Expand Down Expand Up @@ -531,6 +559,7 @@ def check_exception(exception, detects_end)
def test_java
check_exception(JAVA_EXC, false)
check_exception(COMPLEX_JAVA_EXC, false)
check_exception(NESTED_JAVA_EXC, false)
end

def test_js
Expand Down