Repository: knative/pkg
Pull request #1329 (2020-05-13 10:41:56)

Add a library to parse config map values.
We keep copy and pasting code that handles this, so why not make a reusable library for all our projects? :slightly_smiling_face: 

Proof it works: https://github.com/knative/serving/pull/7935

knative-prow-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1329#" title="Author self-approved">markusthoemmes</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **vagababov**
You can assign the PR to them by writing `/assign @vagababov` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative%2Fpkg).

<details open>
Needs approval from an approver in each of these files:

- **[configmap/OWNERS](https://github.com/knative/pkg/blob/master/configmap/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["vagababov"]} -->
knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-pkg-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[configmap/parse.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1329/pull-knative-pkg-go-coverage/1260520742315888641/artifacts/line-cov.html#file0) | Do not exist | 100.0% | 

knative-metrics-robot: The following is the coverage report on the affected files.
Say `/test pull-knative-pkg-go-coverage` to re-run this coverage report

File | Old Coverage | New Coverage | Delta
---- |:------------:|:------------:|:-----:
[configmap/parse.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1329/pull-knative-pkg-go-coverage/1260537729867845635/artifacts/line-cov.html#file0) | Do not exist | 100.0% | 


Patch:
diff --git a/configmap/parse.go b/configmap/parse.go
new file mode 100644
index 000000000..097b38f90
--- /dev/null
+++ b/configmap/parse.go
@@ -0,0 +1,112 @@
+/*
+Copyright 2020 The Knative Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package configmap
+
+import (
+	"strconv"
+	"strings"
+	"time"
+)
+
+// ParseFunc is a function taking ConfigMap data and applying a parse operation to it.
+type ParseFunc func(map[string]string) error
+
+// AsString passes the value at key through into the target, if it exists.
+func AsString(key string, target *string) ParseFunc {
+	return func(data map[string]string) error {
+		if raw, ok := data[key]; ok {
+			*target = raw
+		}
+		return nil
+	}
+}
+
+// AsBool parses the value at key as a boolean into the target, if it exists.
+func AsBool(key string, target *bool) ParseFunc {
+	return func(data map[string]string) error {
+		if raw, ok := data[key]; ok {
+			*target = strings.EqualFold(raw, "true")
+		}
+		return nil
+	}
+}
+
+// AsInt32 parses the value at key as an int32 into the target, if it exists.
+func AsInt32(key string, target *int32) ParseFunc {
+	return func(data map[string]string) error {
+		if raw, ok := data[key]; ok {
+			val, err := strconv.ParseInt(raw, 10, 32)
+			if err != nil {
+				return err
+			}
+			*target = int32(val)
+		}
+		return nil
+	}
+}
+
+// AsInt64 parses the value at key as an int64 into the target, if it exists.
+func AsInt64(key string, target *int64) ParseFunc {
+	return func(data map[string]string) error {
+		if raw, ok := data[key]; ok {
+			val, err := strconv.ParseInt(raw, 10, 64)
+			if err != nil {
+				return err
+			}
+			*target = val
+		}
+		return nil
+	}
+}
+
+// AsFloat64 parses the value at key as a float64 into the target, if it exists.
+func AsFloat64(key string, target *float64) ParseFunc {
+	return func(data map[string]string) error {
+		if raw, ok := data[key]; ok {
+			val, err := strconv.ParseFloat(raw, 64)
+			if err != nil {
+				return err
+			}
+			*target = val
+		}
+		return nil
+	}
+}
+
+// AsDuration parses the value at key as a time.Duration into the target, if it exists.
+func AsDuration(key string, target *time.Duration) ParseFunc {
+	return func(data map[string]string) error {
+		if raw, ok := data[key]; ok {
+			val, err := time.ParseDuration(raw)
+			if err != nil {
+				return err
+			}
+			*target = val
+		}
+		return nil
+	}
+}
+
+// Parse parses the given map using the parser functions passed in.
+func Parse(data map[string]string, parsers ...ParseFunc) error {
+	for _, parse := range parsers {
+		if err := parse(data); err != nil {
+			return err
+		}
+	}
+	return nil
+}
diff --git a/go.mod b/go.mod
index 6df275d5a..976998bf5 100644
--- a/go.mod
+++ b/go.mod
@@ -48,7 +48,7 @@ require (
 	k8s.io/klog v1.0.0
 	k8s.io/kube-openapi v0.0.0-20191107075043-30be4d16710a
 	k8s.io/test-infra v0.0.0-20200407001919-bc7f71ef65b8
-	knative.dev/test-infra v0.0.0-20200509000045-c7114387eed5
+	knative.dev/test-infra v0.0.0-20200513011557-d03429a76034
 )
 
 replace (
diff --git a/go.sum b/go.sum
index ed3ec2f5d..1673431ef 100644
--- a/go.sum
+++ b/go.sum
@@ -1350,8 +1350,8 @@ knative.dev/pkg v0.0.0-20200428194351-90fc61bae7f7/go.mod h1:o+e8OVEJKIuvXPsGVPI
 knative.dev/pkg v0.0.0-20200505191044-3da93ebb24c2/go.mod h1:Q6sL35DdGs8hIQZKdaCXJGgY8f90BmNBKSb8z6d/BTM=
 knative.dev/test-infra v0.0.0-20200407185800-1b88cb3b45a5/go.mod h1:xcdUkMJrLlBswIZqL5zCuBFOC22WIPMQoVX1L35i0vQ=
 knative.dev/test-infra v0.0.0-20200505052144-5ea2f705bb55/go.mod h1:WqF1Azka+FxPZ20keR2zCNtiQA1MP9ZB4BH4HuI+SIU=
-knative.dev/test-infra v0.0.0-20200509000045-c7114387eed5 h1:4lQU2AMChE3Hb2WgHDFfhRbYlITCMUgQ2Y6eeIIEgxM=
-knative.dev/test-infra v0.0.0-20200509000045-c7114387eed5/go.mod h1:aMif0KXL4g19YCYwsy4Ocjjz5xgPlseYV+B95Oo4JGE=
+knative.dev/test-infra v0.0.0-20200513011557-d03429a76034 h1:JxqONCZVS7or+Fv3ebVQoipuIBH7Ig3Qbx170hgIF+A=
+knative.dev/test-infra v0.0.0-20200513011557-d03429a76034/go.mod h1:aMif0KXL4g19YCYwsy4Ocjjz5xgPlseYV+B95Oo4JGE=
 modernc.org/cc v1.0.0/go.mod h1:1Sk4//wdnYJiUIxnW8ddKpaOJCF37yAdqYnkxUpaYxw=
 modernc.org/golex v1.0.0/go.mod h1:b/QX9oBD/LhixY6NDh+IdGv17hgB+51fET1i2kPSmvk=
 modernc.org/mathutil v1.0.0/go.mod h1:wU0vUrJsVWBZ4P6e7xtFJEhFSNsfRLJ8H458uRjg03k=
diff --git a/metrics/metricskey/constants.go b/metrics/metricskey/constants.go
index d9e2a1ace..569067681 100644
--- a/metrics/metricskey/constants.go
+++ b/metrics/metricskey/constants.go
@@ -41,6 +41,12 @@ const (
 	// LabelResponseCodeClass is the label for the HTTP response status code class. For example, "2xx", "3xx", etc.
 	LabelResponseCodeClass = "response_code_class"
 
+	// LabelResponseError is the label for client error. For HTTP, A non-2xx status code doesn't cause an error.
+	LabelResponseError = "response_error"
+
+	// LabelResponseTimeout is the label timeout.
+	LabelResponseTimeout = "response_timeout"
+
 	// ValueUnknown is the default value if the field is unknown, e.g. project will be unknown if Knative
 	// is not running on GKE.
 	ValueUnknown = "unknown"
diff --git a/metrics/utils.go b/metrics/utils.go
index 7345cec92..e2e56a415 100644
--- a/metrics/utils.go
+++ b/metrics/utils.go
@@ -16,7 +16,11 @@ limitations under the License.
 
 package metrics
 
-import "strconv"
+import (
+	"strconv"
+
+	"go.opencensus.io/tag"
+)
 
 // ResponseCodeClass converts an HTTP response code to a string representing its response code class.
 // E.g., The response code class is "5xx" for response code 503.
@@ -24,3 +28,27 @@ func ResponseCodeClass(responseCode int) string {
 	// Get the hundred digit of the response code and concatenate "xx".
 	return strconv.Itoa(responseCode/100) + "xx"
 }
+
+// MaybeInsertIntTag conditionally insert the tag when cond is true.
+func MaybeInsertIntTag(key tag.Key, value int, cond bool) tag.Mutator {
+	if cond {
+		return tag.Insert(key, strconv.Itoa(value))
+	}
+	return tag.Insert(key, "")
+}
+
+// MaybeInsertBoolTag conditionally insert the tag when cond is true.
+func MaybeInsertBoolTag(key tag.Key, value bool, cond bool) tag.Mutator {
+	if cond {
+		return tag.Insert(key, strconv.FormatBool(value))
+	}
+	return tag.Insert(key, "")
+}
+
+// MaybeInsertStringTag conditionally insert the tag when cond is true.
+func MaybeInsertStringTag(key tag.Key, value string, cond bool) tag.Mutator {
+	if cond {
+		return tag.Insert(key, value)
+	}
+	return tag.Insert(key, "")
+}
diff --git a/network/transports.go b/network/transports.go
index 019f4ba87..745e6a5a8 100644
--- a/network/transports.go
+++ b/network/transports.go
@@ -18,7 +18,6 @@ package network
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"net"
 	"net/http"
@@ -54,8 +53,6 @@ var backOffTemplate = wait.Backoff{
 	Steps:    15,
 }
 
-var errDialTimeout = errors.New("timed out dialing")
-
 // DialWithBackOff executes `net.Dialer.DialContext()` with exponentially increasing
 // dial timeouts. In addition it sleeps with random jitter between tries.
 var DialWithBackOff = NewBackoffDialer(backOffTemplate)
@@ -91,7 +88,7 @@ func dialBackOffHelper(ctx context.Context, network, address string, bo wait.Bac
 		}
 		return c, nil
 	}
-	elapsed := time.Now().Sub(start)
+	elapsed := time.Since(start)
 	return nil, fmt.Errorf("timed out dialing after %.2fs", elapsed.Seconds())
 }
 
diff --git a/source/source_stats_reporter.go b/source/source_stats_reporter.go
index 482c5a42e..b22eff94a 100644
--- a/source/source_stats_reporter.go
+++ b/source/source_stats_reporter.go
@@ -18,7 +18,6 @@ package source
 
 import (
 	"context"
-	"strconv"
 
 	"go.opencensus.io/stats/view"
 	"knative.dev/pkg/metrics"
@@ -48,6 +47,8 @@ var (
 	sourceResourceGroupKey = tag.MustNewKey(metricskey.LabelResourceGroup)
 	responseCodeKey        = tag.MustNewKey(metricskey.LabelResponseCode)
 	responseCodeClassKey   = tag.MustNewKey(metricskey.LabelResponseCodeClass)
+	responseError          = tag.MustNewKey(metricskey.LabelResponseError)
+	responseTimeout        = tag.MustNewKey(metricskey.LabelResponseTimeout)
 )
 
 type ReportArgs struct {
@@ -56,6 +57,8 @@ type ReportArgs struct {
 	EventSource   string
 	Name          string
 	ResourceGroup string
+	Error         string
+	Timeout       bool
 }
 
 func init() {
@@ -103,8 +106,10 @@ func (r *reporter) generateTag(args *ReportArgs, responseCode int) (context.Cont
 		tag.Insert(eventTypeKey, args.EventType),
 		tag.Insert(sourceNameKey, args.Name),
 		tag.Insert(sourceResourceGroupKey, args.ResourceGroup),
-		tag.Insert(responseCodeKey, strconv.Itoa(responseCode)),
-		tag.Insert(responseCodeClassKey, metrics.ResponseCodeClass(responseCode)))
+		metrics.MaybeInsertIntTag(responseCodeKey, responseCode, responseCode > 0),
+		metrics.MaybeInsertStringTag(responseCodeClassKey, metrics.ResponseCodeClass(responseCode), responseCode > 0),
+		tag.Insert(responseError, args.Error),
+		metrics.MaybeInsertBoolTag(responseTimeout, args.Timeout, args.Error != ""))
 }
 
 func register() {
@@ -115,7 +120,9 @@ func register() {
 		sourceNameKey,
 		sourceResourceGroupKey,
 		responseCodeKey,
-		responseCodeClassKey}
+		responseCodeClassKey,
+		responseError,
+		responseTimeout}
 
 	// Create view to see our measurements.
 	if err := view.Register(
diff --git a/vendor/modules.txt b/vendor/modules.txt
index 0292d4afe..9464bd40b 100644
--- a/vendor/modules.txt
+++ b/vendor/modules.txt
@@ -914,7 +914,7 @@ k8s.io/utils/buffer
 k8s.io/utils/integer
 k8s.io/utils/pointer
 k8s.io/utils/trace
-# knative.dev/test-infra v0.0.0-20200509000045-c7114387eed5
+# knative.dev/test-infra v0.0.0-20200513011557-d03429a76034
 ## explicit
 knative.dev/test-infra/scripts
 # sigs.k8s.io/yaml v1.1.0


Test patch:
diff --git a/configmap/parse_test.go b/configmap/parse_test.go
new file mode 100644
index 000000000..65b9ab56d
--- /dev/null
+++ b/configmap/parse_test.go
@@ -0,0 +1,128 @@
+/*
+Copyright 2020 The Knative Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package configmap
+
+import (
+	"testing"
+	"time"
+)
+
+type testConfig struct {
+	str string
+	boo bool
+	i32 int32
+	i64 int64
+	f64 float64
+	dur time.Duration
+}
+
+func TestParse(t *testing.T) {
+	tests := []struct {
+		name      string
+		conf      testConfig
+		data      map[string]string
+		want      testConfig
+		expectErr bool
+	}{{
+		name: "all good",
+		data: map[string]string{
+			"test-string":   "foo.bar",
+			"test-bool":     "true",
+			"test-int32":    "1",
+			"test-int64":    "2",
+			"test-float64":  "1.0",
+			"test-duration": "1m",
+		},
+		want: testConfig{
+			str: "foo.bar",
+			boo: true,
+			i32: 1,
+			i64: 2,
+			f64: 1.0,
+			dur: time.Minute,
+		},
+	}, {
+		name: "respect defaults",
+		conf: testConfig{
+			str: "foo.bar",
+			boo: true,
+			i32: 1,
+			i64: 2,
+			f64: 1.0,
+			dur: time.Minute,
+		},
+		want: testConfig{
+			str: "foo.bar",
+			boo: true,
+			i32: 1,
+			i64: 2,
+			f64: 1.0,
+			dur: time.Minute,
+		},
+	}, {
+		name: "bool defaults to false",
+		data: map[string]string{
+			"test-bool": "foo",
+		},
+		want: testConfig{
+			boo: false,
+		},
+	}, {
+		name: "int32 error",
+		data: map[string]string{
+			"test-int32": "foo",
+		},
+		expectErr: true,
+	}, {
+		name: "int64 error",
+		data: map[string]string{
+			"test-int64": "foo",
+		},
+		expectErr: true,
+	}, {
+		name: "float64 error",
+		data: map[string]string{
+			"test-float64": "foo",
+		},
+		expectErr: true,
+	}, {
+		name: "duration error",
+		data: map[string]string{
+			"test-duration": "foo",
+		},
+		expectErr: true,
+	}}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			if err := Parse(test.data,
+				AsString("test-string", &test.conf.str),
+				AsBool("test-bool", &test.conf.boo),
+				AsInt32("test-int32", &test.conf.i32),
+				AsInt64("test-int64", &test.conf.i64),
+				AsFloat64("test-float64", &test.conf.f64),
+				AsDuration("test-duration", &test.conf.dur),
+			); (err == nil) == test.expectErr {
+				t.Fatal("Failed to parse data:", err)
+			}
+
+			if test.conf != test.want {
+				t.Fatalf("parsed = %v, want %v", test.conf, test.want)
+			}
+		})
+	}
+}
diff --git a/kmp/reporters_test.go b/kmp/reporters_test.go
index 29e1b729b..2daaf42d7 100644
--- a/kmp/reporters_test.go
+++ b/kmp/reporters_test.go
@@ -31,7 +31,7 @@ type testStruct struct {
 	Omit        string      `json:"omit,omitempty"`
 	Ignore      string      `json:"-"`
 	Dash        string      `json:"-,"`
-	//lint:ignore SA5008 we actually want to test this broken json options work
+	//nolint:staticcheck // We actually want to test this broken json options work
 	MultiComma string `json:"multi,omitempty,somethingelse"`
 }
 
diff --git a/test/e2e_flags.go b/test/e2e_flags.go
index 3dfb2eb3d..bc6f90bff 100644
--- a/test/e2e_flags.go
+++ b/test/e2e_flags.go
@@ -32,7 +32,6 @@ import (
 )
 
 var (
-	klogFlags = flag.NewFlagSet("klog", flag.ExitOnError)
 	// Flags holds the command line flags or defaults for settings in the user's environment.
 	// See EnvironmentFlags for a list of supported fields.
 	Flags = initializeFlags()
diff --git a/test/logging/example_tlogger_test.go b/test/logging/example_tlogger_test.go
index 1395822fa..7c8660034 100644
--- a/test/logging/example_tlogger_test.go
+++ b/test/logging/example_tlogger_test.go
@@ -34,8 +34,7 @@ var (
 
 func init() { someStruct = testStruct{"hello", 42.0} }
 
-// godoc limitation; this would be named Test*(), of course
-
+// nolint:govet // godoc limitation; this would be named Test*(), of course
 func Example(legacy *testing.T) {
 	// Get our TLogger and ready the cleanup function
 	t, cancel := logging.NewTLogger(legacy)
diff --git a/test/webhook-apicoverage/resourcetree/test_util.go b/test/webhook-apicoverage/resourcetree/test_util.go
index 34fca0545..3b922d44a 100644
--- a/test/webhook-apicoverage/resourcetree/test_util.go
+++ b/test/webhook-apicoverage/resourcetree/test_util.go
@@ -49,11 +49,13 @@ type arrayType struct {
 	baseArr   []bool
 }
 
+//nolint:structcheck,unused // Disable warnings about unused fields.
 type otherType struct {
 	structMap map[string]baseType
 	baseMap   map[string]string
 }
 
+//nolint:structcheck,unused // Disable warnings about unused fields.
 type combinedNodeType struct {
 	b baseType
 	a arrayType
diff --git a/vendor/knative.dev/test-infra/scripts/presubmit-tests.sh b/vendor/knative.dev/test-infra/scripts/presubmit-tests.sh
index 43a23df31..1435a3469 100644
--- a/vendor/knative.dev/test-infra/scripts/presubmit-tests.sh
+++ b/vendor/knative.dev/test-infra/scripts/presubmit-tests.sh
@@ -302,6 +302,8 @@ function main() {
     kubectl version --client
     echo ">> go version"
     go version
+    echo ">> go env"
+    go env
     echo ">> python3 version"
     python3 --version
     echo ">> git version"