-
Notifications
You must be signed in to change notification settings - Fork 17
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
Added the processing when did not match the rules. #1
base: master
Are you sure you want to change the base?
Conversation
@@ -21,10 +22,14 @@ def configure(conf) | |||
@rules = conf.elements.select { |element| | |||
element.name == 'rule' | |||
}.each { |element| | |||
if element.has_key?("pattern") | |||
if element["pattern"] && element["key"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ここ、なんでelement["pattern"]
に変更したんでしょうか。
element["pattern"]
だと、ルールにpattern
と書いただけで空文字が入ってしまう(つまりtrue
になる)ので、pattern
というキーを設定するってことは、それが空文字だろうがなんだろうが、なんか設定する意思を表示したものとして扱うためにhas_key?
したような記憶があります。
でもあんま憶えてないので嘘かもですが…
ご指摘の箇所を修正し、エラーメッセージのテストを追加しました。 ご確認よろしくお願いいたします。 |
@@ -52,7 +63,14 @@ def emit(tag, es, chain) | |||
|
|||
es.each do |time, record| | |||
filtered_tag, record = rewrite(tag, record) | |||
Engine.emit(filtered_tag, time, record) if filtered_tag && record | |||
if _tag == filtered_tag | |||
if !@warn_msg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ここの条件、通ることあるんですかね?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test_emit d3のケースで通過します。
・replaceが設定されているが、同じkeyに対してのruleで、append_to_tagが設定されていない場合
コメント書いてて気づきましたがコレはapply_ruleのところに書くべきですね・・・
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmhm じゃあそっちに移動しますかねー
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
あ、でもその場合はconfigErrorにしたほうが良い気もしてきました。
どう思われますか?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error投げるとfluentdとまりますよね? 止まったほうがいいのかなー
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
起動しなくなりますね。
でも、該当する場合は未来永劫emitしないruleなわけですからね・・・
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ああ、最初のconfigのところでErrorにするわけですね。それならいいとおもいます。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
言葉足らずで申し訳ありません。。
では、cofigureメソッドでチェックして、ダメな場合はerrorにしますね。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
例外をテストするなら、assert_raise
使う感じですねー
http://www.ruby-doc.org/stdlib-1.9.3/libdoc/test/unit/rdoc/Test/Unit/Assertions.html#method-i-assert_raise
・単一のkeyに対して最終的にappend_to_tagがセットされていない場合 に、raise ConfigErrorするようにしました。 |
Hi.
I've added the action to take if do not match the rules.
Changes
・Do not emit if the tag is not replaced.
・Output an error message depending on the situation.
・Set the rules required the 'key' and 'pattern'.
Thank you confirmation.