Add tag(s) DNS lookup time out. #67
Conversation
Keep the config orders in a right way. Co-authored-by: João Duarte <jsvd@users.noreply.github.com>
…meouts. This commit also covers unit tests.
…version, this will not break the behavior and they will have an opportunity to configure the warn on timeout.
lib/logstash/filters/dns.rb
Outdated
| @logger.warn(log_message, :field => field, :value => raw) | ||
| else | ||
| @logger.info(log_message, :field => field, :value => raw) | ||
| end |
There was a problem hiding this comment.
Same as the the other relevant change.
lib/logstash/filters/dns.rb
Outdated
| @logger.warn(log_message, :field => field, :value => raw) | ||
| else | ||
| @logger.info(log_message, :field => field, :value => raw) | ||
| end |
There was a problem hiding this comment.
it seems this change tries to address this issue.
The requested feature is asking for a way to disable logging. It is NOT asking for a way to change this logging from WARN to INFO. Can you please clarify why is this change relevant to the attached issue?
There was a problem hiding this comment.
another option is to make this DEBUG as the original PR.
There was a problem hiding this comment.
Long history and it is true that now the logic is adding extra noise :( and doesn't solve the user need.
Stepping back and analyzing, I agree with the debug approach as we added the tag. Users will have two granular options: receiving with tag and debug logs.
Please take a look at the final approach and provide your comments.
Thanks so much for catching this up!
…ly warn timeouts. Also, as user requested, it is now a noise but can be triggered in debug logs.
| @@ -219,8 +222,9 @@ def resolve(event) | |||
| end | |||
| rescue Resolv::ResolvTimeout | |||
| @failed_cache[raw] = true if @failed_cache | |||
| @logger.warn("DNS: timeout on resolving the hostname.", | |||
| @logger.debug("DNS: timeout on resolving the hostname.", | |||
There was a problem hiding this comment.
Ok I think it is fair to say that changing the log level is not a breaking change, and we need to re-evalute in the future how we handle logging for event processing, so I'm fine leaving this as debug.
Description
We had feature requests (#24, #47) to support the tag on DNS look up timeouts and it seems we had a contribution (PR) but author didn't follow up to end.
This change covers followings:
_dnstimeoutwarn_on_timeoutconfig to tune the log level when dns time out occursThis PR intends to cover remain works of original PR.
Closes
Closes #24
Closes #47