net/portmapper: add clientmetric for UPnP error codes

This should allow us to gather a bit more information about errors that
we encounter when creating UPnP mappings. Since we don't have a
"LabelMap" construction for clientmetrics, do what sockstats does and
lazily register a new metric when we see a new code.

Updates #9343

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: Ibb5aadd6138beb58721f98123debcc7273b611ba
This commit is contained in:
Andrew Dunham 2023-09-12 19:16:51 -04:00
parent 19a9d9037f
commit d9ae7d670e
2 changed files with 34 additions and 10 deletions

View File

@ -28,6 +28,7 @@ import (
"tailscale.com/types/logger"
"tailscale.com/types/nettype"
"tailscale.com/util/clientmetric"
"tailscale.com/util/mak"
)
// DebugKnobs contains debug configuration that can be provided when creating a
@ -1024,3 +1025,22 @@ var (
// we received a UPnP response with a new meta.
metricUPnPUpdatedMeta = clientmetric.NewCounter("portmap_upnp_updated_meta")
)
// UPnP error metric that's keyed by code; lazily registered on first read
var (
metricUPnPErrorsByCodeMu sync.Mutex
metricUPnPErrorsByCode map[int]*clientmetric.Metric
)
func getUPnPErrorsMetric(code int) *clientmetric.Metric {
metricUPnPErrorsByCodeMu.Lock()
defer metricUPnPErrorsByCodeMu.Unlock()
mm := metricUPnPErrorsByCode[code]
if mm != nil {
return mm
}
mm = clientmetric.NewCounter(fmt.Sprintf("portmap_upnp_errors_with_code_%d", code))
mak.Set(&metricUPnPErrorsByCode, code, mm)
return mm
}

View File

@ -337,9 +337,14 @@ func (c *Client) getUPnPPortMapping(
// duration; see the following issue for details:
// https://github.com/tailscale/tailscale/issues/9343
if err != nil {
code, ok := getUPnPErrorCode(err)
if ok {
getUPnPErrorsMetric(code).Add(1)
}
// From the UPnP spec: http://upnp.org/specs/gw/UPnP-gw-WANIPConnection-v2-Service.pdf
// 725: OnlyPermanentLeasesSupported
if isUPnPError(err, 725) {
if ok && code == 725 {
newPort, err = addAnyPortMapping(
ctx,
client,
@ -387,13 +392,13 @@ func (c *Client) getUPnPPortMapping(
return upnp.external, true
}
// isUPnPError returns whether the provided error is a UPnP error response with
// the given error code. It returns false if the error is not a SOAP error, or
// the inner error details are not a UPnP error.
func isUPnPError(err error, errCode int) bool {
// getUPnPErrorCode returns the UPnP error code from the given response, if the
// error is a SOAP error in the proper format, and a boolean indicating whether
// the provided error was actually a UPnP error.
func getUPnPErrorCode(err error) (int, bool) {
soapErr, ok := err.(*soap.SOAPFaultError)
if !ok {
return false
return 0, false
}
var upnpErr struct {
@ -402,13 +407,12 @@ func isUPnPError(err error, errCode int) bool {
Description string `xml:"errorDescription"`
}
if err := xml.Unmarshal([]byte(soapErr.Detail.Raw), &upnpErr); err != nil {
return false
return 0, false
}
if upnpErr.XMLName.Local != "UPnPError" {
return false
return 0, false
}
return upnpErr.Code == errCode
return upnpErr.Code, true
}
type uPnPDiscoResponse struct {