Advanced ICMP options #240
Conversation
config/config.go
Outdated
| @@ -96,7 +96,8 @@ type TCPProbe struct { | |||
|
|
|||
| type ICMPProbe struct { | |||
| PreferredIPProtocol string `yaml:"preferred_ip_protocol,omitempty"` // Defaults to "ip6". | |||
|
|
|||
| Payload int `yaml:"payload,omitempty"` | |||
There was a problem hiding this comment.
PayloadSize would make more sense
config/config.go
Outdated
| @@ -96,7 +96,8 @@ type TCPProbe struct { | |||
|
|
|||
| type ICMPProbe struct { | |||
| PreferredIPProtocol string `yaml:"preferred_ip_protocol,omitempty"` // Defaults to "ip6". | |||
|
|
|||
| Payload int `yaml:"payload,omitempty"` | |||
| DontFragment bool `yaml:"dontfragment,omitempty"` | |||
prober/icmp.go
Outdated
| copy(data, "Prometheus Blackbox Exporter") | ||
| } else { | ||
| data = []byte("Prometheus Blackbox Exporter") | ||
| payload = 1500 // The usual MTU size. |
prober/icmp.go
Outdated
| @@ -136,3 +132,96 @@ func ProbeICMP(ctx context.Context, target string, module config.Module, registr | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| func getICMPSocket( | |||
There was a problem hiding this comment.
I don't think you're buying much by breaking this out, and it also makes it more difficult to review what has changed in the code
| } | ||
|
|
||
| func (c *dfConn) ReadFrom(b []byte) (int, net.Addr, error) { | ||
| h, p, _, err := c.c.ReadFrom(b) |
There was a problem hiding this comment.
This won't work on Windows, which is a supported platform.
There was a problem hiding this comment.
How do you propose I move about this? Different files for nix and windows? Or error when reading config?
There was a problem hiding this comment.
First see if this can be done with Windows, we should be aiming for feature parity
|
Feature idea seems good to me. |
gouthamve
force-pushed
the
icmp-payload-config
branch
2 times, most recently
from
6a6d3ca to
cebe76f
Compare
|
Updated to incorporate feedback and to error if |
Signed-off-by: Goutham Veeramachaneni <[email protected]>
gouthamve
force-pushed
the
icmp-payload-config
branch
from
cebe76f to
482d59b
Compare
CONFIGURATION.md
Outdated
| @@ -168,6 +168,12 @@ validate_additional_rrs: | |||
| # The preferred IP protocol of the ICMP probe (ip4, ip6). | |||
| [ preferred_ip_protocol: <string> | default = "ip6" ] | |||
|
|
|||
| # Set the DF-Bit in the IP-header. Only works with ip4 and on *nix systems. | |||
prober/icmp.go
Outdated
| copy(data[:], "Prometheus Blackbox Exporter") | ||
| if module.ICMP.PayloadSize != 0 { | ||
| data = make([]byte, module.ICMP.PayloadSize) | ||
| copy(data, "Prometheus Blackbox Exporter") |
There was a problem hiding this comment.
What if the payload size is smaller than this string?
There was a problem hiding this comment.
https://play.golang.org/p/qrDFrlW6He Then only the payload size will be sen't with truncated string.
Signed-off-by: Goutham Veeramachaneni <[email protected]>
gouthamve
force-pushed
the
icmp-payload-config
branch
from
a00c3ad to
00d6bae
Compare
| var data []byte | ||
| if module.ICMP.PayloadSize != 0 { | ||
| data = make([]byte, module.ICMP.PayloadSize) | ||
| copy(data, "Prometheus Blackbox Exporter") |
There was a problem hiding this comment.
Nitpick, but it might make sense to include the exporter version in here.
@RichiH wanted to add the payload size and DF-bit to be configurable in the ICMP prober. While I had fun implementing this, I would like to know if this is okay before I put in more effort.
cc/ @brian-brazil @SuperQ
This change is