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

Add support Tcp dns probes #681

Merged
merged 11 commits into from Feb 21, 2024
Merged

Conversation

aitorpazos
Copy link
Contributor

This PR tries to address the issue I created myself sometime before: #680

Tested with the following config file:

probe {
  name: "google_com_default"
  type: DNS
  targets {
    host_names: "8.8.8.8"
  }
  interval_msec: 15000
  timeout_msec: 5000
  latency_distribution {
    explicit_buckets: "0.00025,0.0005,0.001,0.002,0.004,0.008,0.016,0.032,0.064,0.128,0.256,0.512,1.024,2.048,4.096,8.192"
  }
  latency_unit: "s"
  dns_probe {
    query_type: A
    resolved_domain: "google.com"
    resolve_first: true
  }
}

probe {
  name: "google_com_udp"
  type: DNS
  targets {
    host_names: "8.8.8.8"
  }
  interval_msec: 15000
  timeout_msec: 5000
  latency_distribution {
    explicit_buckets: "0.00025,0.0005,0.001,0.002,0.004,0.008,0.016,0.032,0.064,0.128,0.256,0.512,1.024,2.048,4.096,8.192"
  }
  additional_label {
    key: "dns_proto"
    value: "udp"
  }
  latency_unit: "s"
  dns_probe {
    query_type: A
    resolved_domain: "google.com"
    resolve_first: true
    dns_proto: "udp"
  }
}

probe {
  name: "google_com_tcp"
  type: DNS
  targets {
    host_names: "8.8.8.8"
  }
  interval_msec: 15000
  timeout_msec: 5000
  latency_distribution {
    explicit_buckets: "0.00025,0.0005,0.001,0.002,0.004,0.008,0.016,0.032,0.064,0.128,0.256,0.512,1.024,2.048,4.096,8.192"
  }
  additional_label {
    key: "dns_proto"
    value: "tcp"
  }
  latency_unit: "s"
  dns_probe {
    query_type: A
    resolved_domain: "google.com"
    resolve_first: true
    dns_proto: "tcp"
  }
}

I can see the different metrics being reported:

...
# TYPE total counter
total{ptype="dns",probe="google_com_default",dst="8.8.8.8"} 1 1707496047071
total{ptype="dns",probe="google_com_udp",dst="8.8.8.8",dns_proto="udp"} 1 1707496049072
total{ptype="dns",probe="google_com_tcp",dst="8.8.8.8",dns_proto="tcp"} 1 1707496051072
# TYPE success counter
success{ptype="dns",probe="google_com_default",dst="8.8.8.8"} 1 1707496047071
success{ptype="dns",probe="google_com_udp",dst="8.8.8.8",dns_proto="udp"} 1 1707496049072
success{ptype="dns",probe="google_com_tcp",dst="8.8.8.8",dns_proto="tcp"} 1 1707496051072
...

Also verified that the configuration is respected through some packets captures:

image

Copy link
Contributor

@manugarg manugarg left a comment

Choose a reason for hiding this comment

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

Thanks @aitorpazos for this PR.

Please see my comment inline.

probes/dns/proto/config.proto Outdated Show resolved Hide resolved
Copy link
Contributor

@manugarg manugarg left a comment

Choose a reason for hiding this comment

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

One more comment regarding the test.

probes/dns/dns_test.go Show resolved Hide resolved
@manugarg manugarg self-assigned this Feb 18, 2024
@manugarg manugarg added this to the v0.13.3 milestone Feb 18, 2024
@aitorpazos
Copy link
Contributor Author

With latest commit switching the DNSProto config from string to an Enum, this is the right config file:

probe {
  name: "google_com_default"
  type: DNS
  targets {
    host_names: "8.8.8.8"
  }
  interval_msec: 15000
  timeout_msec: 5000
  latency_distribution {
    explicit_buckets: "0.00025,0.0005,0.001,0.002,0.004,0.008,0.016,0.032,0.064,0.128,0.256,0.512,1.024,2.048,4.096,8.192"
  }
  latency_unit: "s"
  dns_probe {
    query_type: A
    resolved_domain: "google.com"
    resolve_first: true
  }
}

probe {
  name: "google_com_udp"
  type: DNS
  targets {
    host_names: "8.8.8.8"
  }
  interval_msec: 15000
  timeout_msec: 5000
  latency_distribution {
    explicit_buckets: "0.00025,0.0005,0.001,0.002,0.004,0.008,0.016,0.032,0.064,0.128,0.256,0.512,1.024,2.048,4.096,8.192"
  }
  additional_label {
    key: "dns_proto"
    value: "udp"
  }
  latency_unit: "s"
  dns_probe {
    query_type: A
    resolved_domain: "google.com"
    resolve_first: true
    dns_proto: UDP
  }
}

probe {
  name: "google_com_tcp"
  type: DNS
  targets {
    host_names: "8.8.8.8"
  }
  interval_msec: 15000
  timeout_msec: 5000
  latency_distribution {
    explicit_buckets: "0.00025,0.0005,0.001,0.002,0.004,0.008,0.016,0.032,0.064,0.128,0.256,0.512,1.024,2.048,4.096,8.192"
  }
  additional_label {
    key: "dns_proto"
    value: "tcp"
  }
  latency_unit: "s"
  dns_probe {
    query_type: A
    resolved_domain: "google.com"
    resolve_first: true
    dns_proto: TCP
  }
}

Copy link
Contributor

@manugarg manugarg left a comment

Choose a reason for hiding this comment

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

Thanks @aitorpazos! Looks mostly good, couple of minor suggestions.

probes/dns/dns.go Outdated Show resolved Hide resolved
probes/dns/dns_test.go Outdated Show resolved Hide resolved
probes/dns/dns_test.go Outdated Show resolved Hide resolved
probes/dns/dns_test.go Outdated Show resolved Hide resolved
probes/dns/dns.go Outdated Show resolved Hide resolved
Copy link
Contributor

@manugarg manugarg left a comment

Choose a reason for hiding this comment

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

One last super minor cleanup, looks good otherwise :)

probes/dns/dns.go Outdated Show resolved Hide resolved
probes/dns/dns_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@manugarg manugarg left a comment

Choose a reason for hiding this comment

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

Awesome. Thanks!

@manugarg manugarg merged commit 4b2f50c into cloudprober:master Feb 21, 2024
5 checks passed
@aitorpazos aitorpazos deleted the tcp_dns_probes branch February 21, 2024 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants