Add support for parsing SNMP transport from target

[hhromic]

This PR implements support for parsing the SNMP transport to use from the target query parameter.

• Accepted syntax for target: [transport://]host[:port]
• New dedicated function: configures a gosnmp.GoSNMP instance from a parsed target string.
• If transport:// is not provided, Transport is left unconfigured and GoSNMP defaults to udp.
• Setting Target and Port in GoSNMP is left as it was in the original implementation.
• There is no transport validation during parsing, this is delegated to GoSNMP during connect.
• Added unit test and cases for the new dedicated parsing function.
• Added better documentation for the target parameter in README that includes transport support.

I tested connectivity as well. The snmp exporter can now connect via TCP to an SNMP server.

Closes #555

[SuperQ]

Very nice, thanks.

[SuperQ]

@RichiH Any comments on the syntax here?

[RichiH]

I remain torn on "URL via SD" vs "config", but have come to accept that URL is probably Least Bad overall.

Feature itself is very nice, even though I have never needed it myself.

I suspect some tests are missing, see comment. Other than that, LGTM.

[hhromic]

I remain torn on "URL via SD" vs "config", but have come to accept that URL is probably Least Bad overall.

As a further argument, take as an example the approach from the blackbox exporter:

scrape_configs:
  - job_name: 'blackbox'
    metrics_path: /probe
    params:
      module: [http_2xx]  # Look for a HTTP 200 response.
    static_configs:
      - targets:
        - http://prometheus.io    # Target to probe with http.
        - https://prometheus.io   # Target to probe with https.
        - http://example.com:8080 # Target to probe with http on port 8080.
    relabel_configs:
      - source_labels: [__address__]
        target_label: __param_target
      - source_labels: [__param_target]
        target_label: instance
      - target_label: __address__
        replacement: 127.0.0.1:9115  # The blackbox exporter's real hostname:port.

As you can see, the targets contain the scheme (http:// or https://) in the target parameter as well for the http probe.

[RichiH]

Yes, thinking about blackbox_exporter yesterday night is what made me flip over to URL scheme. I had a whole thing written on why config is slightly preferable, but... I was wrong and didn't send it :)

[hhromic]

I now added an example SNMP device via TCP for Prometheus to the README file as well.

[SuperQ]

I still think config is slightly preferable. But I also don't object to this format.

[hhromic]

I still think config is slightly preferable. But I also don't object to this format.

I still don't see how that would work :). The transport is a simple property of each target, like the port number to use, independent of the auth or module used. In the same way insecure or secure HTTP (the scheme) is a property of an URL.

In any case, thanks for merging this contribution!
I'm just curious how you envision it as a config and still being practical.

[SuperQ]

It would simply be a property of either the auth or module, like the SNMP version or the walk params (max_repititions, etc). Which is also part of the "Transport" IMO.

The thing is, the module is meant to represent a device or device class. So having some devices with UDP and some with TCP represent different modules.

[hhromic]

It would simply be a property of either the auth or module, like the SNMP version or the walk params (max_repititions, etc). Which is also part of the "Transport" IMO.

Ah I see, yes seems closer to module via WalkParams:

snmp_exporter/config/config.go

Lines 77 to 83 in 03bc586

	type WalkParams struct {
	MaxRepetitions uint32 `yaml:"max_repetitions,omitempty"`
	Retries *int `yaml:"retries,omitempty"`
	Timeout time.Duration `yaml:"timeout,omitempty"`
	UseUnconnectedUDPSocket bool `yaml:"use_unconnected_udp_socket,omitempty"`
	AllowNonIncreasingOIDs bool `yaml:"allow_nonincreasing_oids,omitempty"`
	}

The thing is, the module is meant to represent a device or device class. So having some devices with UDP and some with TCP represent different modules.

I always have seen module as a representation of a device class indeed but with the target query parameter representing a concrete device of a certain class (module query parameter) to be scraped using certain parameters (e.g. transport). In my humble opinion, that a device uses UDP or TCP, or both, should not mean different modules. The same device can even be reconfigured to change its transport at different times, never changing its class.

The same could be argued about timeouts and retries even. In my humble opinion, most of these walk params are more a specific target-related set of properties than the class of the target (module). Especially when changing those. For example, if we want a different transport or timeout for the same device, we would have to modify the huge snmp.yaml file. In the case of Docker, even maybe regenerate the Docker image.