Skip to content
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

dns_query should not report 0ms query time on error #2548

Closed
phemmer opened this issue Mar 18, 2017 · 3 comments · Fixed by #4118
Closed

dns_query should not report 0ms query time on error #2548

phemmer opened this issue Mar 18, 2017 · 3 comments · Fixed by #4118
Labels
bug unexpected problem or unintended behavior
Milestone

Comments

@phemmer
Copy link
Contributor

phemmer commented Mar 18, 2017

Bug report

Relevant telegraf.conf:

[[inputs.dns_query]]
  servers = ["8.8.8.8"]
  record_type = "A"
  domains = ["google.com"]
  timeout = 10

System info:

Telegraf 1.2.1

Steps to reproduce:

  1. Run telegraf
  2. Interrupt resolution to cause an error, such as a timeout.

Expected behavior:

One of:

  1. query_time_ms=10000
  2. null value

Actual behavior:

query_time_ms=0

Additional info:

This is highly problematic as the value indicates that the response came back immediately. However this is not the case. The response didn't come back at all. The result should somehow indicate this by either setting the value to the maximum (the timeout value), or not setting it at all, and providing some other field indicating an error occurred.

@danielnelson danielnelson added the bug unexpected problem or unintended behavior label Mar 20, 2017
@danielnelson
Copy link
Contributor

danielnelson commented Apr 5, 2017

I think it makes sense to have a field that is emitted only when a timeout occurs with the value being the time waited: query_timeout_ms=10000. I wonder if we should try to stick to the same units across fields, should timeouts always be reported in seconds? Certainly this would be silly for some points so maybe not.

@discoduck2x
Copy link

@danielnelson - any update on this? the dns query input is rather useless unfortunatley in its current state

@danielnelson
Copy link
Contributor

I'll try to knock out a fix in the next few days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants