Repository: knative/pkg
Pull request #1975 (2020-12-24 01:04:21)

Add nostackdriver to metrics as well

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

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1975#" title="Author self-approved">mattmoor</a>*

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

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

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

- ~~[metrics/OWNERS](https://github.com/knative/pkg/blob/master/metrics/OWNERS)~~ [mattmoor]

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":[]} -->
codecov[bot]: # [Codecov](https://codecov.io/gh/knative/pkg/pull/1975?src=pr&el=h1) Report
> Merging [#1975](https://codecov.io/gh/knative/pkg/pull/1975?src=pr&el=desc) (938a9cc) into [master](https://codecov.io/gh/knative/pkg/commit/9d0775512af8df6df7ea94b049f0da0277c59702?el=desc) (9d07755) will **increase** coverage by `0.05%`.
> The diff coverage is `71.05%`.

[![Impacted file tree graph](https://codecov.io/gh/knative/pkg/pull/1975/graphs/tree.svg?width=650&height=150&src=pr&token=rlqpDnfxbO)](https://codecov.io/gh/knative/pkg/pull/1975?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1975      +/-   ##
==========================================
+ Coverage   69.05%   69.11%   +0.05%     
==========================================
  Files         209      210       +1     
  Lines        8792     8796       +4     
==========================================
+ Hits         6071     6079       +8     
+ Misses       2451     2449       -2     
+ Partials      270      268       -2     
```


| [Impacted Files](https://codecov.io/gh/knative/pkg/pull/1975?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [metrics/gcp\_metadata.go](https://codecov.io/gh/knative/pkg/pull/1975/diff?src=pr&el=tree#diff-bWV0cmljcy9nY3BfbWV0YWRhdGEuZ28=) | `0.00% <ø> (ø)` | |
| [metrics/testing.go](https://codecov.io/gh/knative/pkg/pull/1975/diff?src=pr&el=tree#diff-bWV0cmljcy90ZXN0aW5nLmdv) | `100.00% <ø> (ø)` | |
| [tracing/opencensus.go](https://codecov.io/gh/knative/pkg/pull/1975/diff?src=pr&el=tree#diff-dHJhY2luZy9vcGVuY2Vuc3VzLmdv) | `43.05% <0.00%> (+1.72%)` | :arrow_up: |
| [tracing/stackdriver.go](https://codecov.io/gh/knative/pkg/pull/1975/diff?src=pr&el=tree#diff-dHJhY2luZy9zdGFja2RyaXZlci5nbw==) | `0.00% <0.00%> (ø)` | |
| [depcheck/depcheck.go](https://codecov.io/gh/knative/pkg/pull/1975/diff?src=pr&el=tree#diff-ZGVwY2hlY2svZGVwY2hlY2suZ28=) | `82.45% <60.00%> (+0.31%)` | :arrow_up: |
| [metrics/stackdriver\_exporter.go](https://codecov.io/gh/knative/pkg/pull/1975/diff?src=pr&el=tree#diff-bWV0cmljcy9zdGFja2RyaXZlcl9leHBvcnRlci5nbw==) | `82.48% <90.47%> (+1.07%)` | :arrow_up: |
| [metrics/config.go](https://codecov.io/gh/knative/pkg/pull/1975/diff?src=pr&el=tree#diff-bWV0cmljcy9jb25maWcuZ28=) | `82.24% <100.00%> (-0.96%)` | :arrow_down: |
| [test/gcs/mock/mock.go](https://codecov.io/gh/knative/pkg/pull/1975/diff?src=pr&el=tree#diff-dGVzdC9nY3MvbW9jay9tb2NrLmdv) | `92.47% <0.00%> (+2.15%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/knative/pkg/pull/1975?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/knative/pkg/pull/1975?src=pr&el=footer). Last update [42c8f15...2561223](https://codecov.io/gh/knative/pkg/pull/1975?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

knative-prow-robot: @mattmoor: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-unit-tests | e30a622446bc03c9c324a456b5fc25eb79457aac | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/1975/pull-knative-pkg-unit-tests/1341913641556905984) | `/test pull-knative-pkg-unit-tests`

[Full PR test history](https://gubernator.knative.dev/pr/knative_pkg/1975). [Your PR dashboard](https://gubernator.knative.dev/pr/mattmoor).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
mattmoor: /retest

Patch:
diff --git a/.github/workflows/knative-verify.yaml b/.github/workflows/knative-verify.yaml
index 3443a0496..5f20c548b 100644
--- a/.github/workflows/knative-verify.yaml
+++ b/.github/workflows/knative-verify.yaml
@@ -45,7 +45,9 @@ jobs:
 
     - name: Install Dependencies
       run: |
-        go get github.com/google/ko/cmd/ko
+        curl -L https://github.com/google/ko/releases/download/v0.6.0/ko_0.6.0_Linux_x86_64.tar.gz | tar xzf - ko
+        chmod +x ./ko
+        sudo mv ko /usr/local/bin
         go get github.com/google/go-licenses
 
     - name: Check out code onto GOPATH
diff --git a/metrics/config.go b/metrics/config.go
index 535c8e9bc..c4c9c7f84 100644
--- a/metrics/config.go
+++ b/metrics/config.go
@@ -22,7 +22,6 @@ import (
 	"errors"
 	"fmt"
 	"os"
-	"path"
 	"strconv"
 	"strings"
 	"time"
@@ -65,6 +64,12 @@ const (
 	prometheusHostEnvName = "METRICS_PROMETHEUS_HOST"
 )
 
+var (
+	// TestOverrideBundleCount is a variable for testing to reduce the size (number of metrics) buffered before
+	// Stackdriver will send a bundled metric report. Only applies if non-zero.
+	TestOverrideBundleCount = 0
+)
+
 // Metrics backend "enum".
 const (
 	// stackdriver is used for Stackdriver backend
@@ -248,37 +253,8 @@ func createMetricsConfig(ctx context.Context, ops ExporterOptions) (*metricsConf
 		mc.prometheusPort = pp
 		mc.prometheusHost = prometheusHost()
 	case stackdriver:
-		// If stackdriverClientConfig is not provided for stackdriver backend destination, OpenCensus will try to
-		// use the application default credentials. If that is not available, Opencensus would fail to create the
-		// metrics exporter.
-		scc := NewStackdriverClientConfigFromMap(m)
-		mc.stackdriverClientConfig = *scc
-		mc.isStackdriverBackend = true
-		var allowCustomMetrics bool
-		var err error
-		mc.stackdriverMetricTypePrefix = path.Join(mc.domain, mc.component)
-
-		customMetricsSubDomain := m[stackdriverCustomMetricSubDomainKey]
-		if customMetricsSubDomain == "" {
-			customMetricsSubDomain = defaultCustomMetricSubDomain
-		}
-		mc.stackdriverCustomMetricTypePrefix = path.Join(customMetricTypePrefix, customMetricsSubDomain, mc.component)
-		if ascmStr := m[allowStackdriverCustomMetricsKey]; ascmStr != "" {
-			allowCustomMetrics, err = strconv.ParseBool(ascmStr)
-			if err != nil {
-				return nil, fmt.Errorf("invalid %s value %q", allowStackdriverCustomMetricsKey, ascmStr)
-			}
-		}
-
-		mc.recorder = sdCustomMetricsRecorder(mc, allowCustomMetrics)
-
-		if scc.UseSecret {
-			secret, err := getStackdriverSecret(ctx, ops.Secrets)
-			if err != nil {
-				return nil, err
-			}
-
-			mc.secret = secret
+		if err := sdinit(ctx, m, &mc, ops); err != nil {
+			return nil, err
 		}
 	}
 
diff --git a/metrics/gcp_metadata.go b/metrics/gcp_metadata.go
index ec6ae878e..3c1a6a09a 100644
--- a/metrics/gcp_metadata.go
+++ b/metrics/gcp_metadata.go
@@ -1,3 +1,5 @@
+// +build !nostackdriver
+
 /*
 Copyright 2019 The Knative Authors
 
diff --git a/metrics/nostackdriver.go b/metrics/nostackdriver.go
new file mode 100644
index 000000000..b5c71eb77
--- /dev/null
+++ b/metrics/nostackdriver.go
@@ -0,0 +1,35 @@
+// +build nostackdriver
+
+/*
+Copyright 2019 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 metrics
+
+import (
+	"context"
+	"errors"
+
+	"go.opencensus.io/stats/view"
+	"go.uber.org/zap"
+)
+
+func sdinit(ctx context.Context, m map[string]string, mc *metricsConfig, ops ExporterOptions) error {
+	return errors.New("Stackdriver support is not included")
+}
+
+func newStackdriverExporter(config *metricsConfig, logger *zap.SugaredLogger) (view.Exporter, ResourceExporterFactory, error) {
+	return nil, nil, errors.New("Stackdriver support is not included")
+}
diff --git a/metrics/stackdriver_exporter.go b/metrics/stackdriver_exporter.go
index ab14ef266..a875accef 100644
--- a/metrics/stackdriver_exporter.go
+++ b/metrics/stackdriver_exporter.go
@@ -1,3 +1,5 @@
+// +build !nostackdriver
+
 /*
 Copyright 2019 The Knative Authors
 
@@ -20,6 +22,7 @@ import (
 	"context"
 	"fmt"
 	"path"
+	"strconv"
 	"sync"
 	"time"
 
@@ -88,10 +91,6 @@ var (
 	// metricToResourceLabels provides a quick lookup from a custom metric name to the set of tags
 	// which should be promoted to Stackdriver Resource labels via opencensus resources.
 	metricToResourceLabels = map[string]*resourceTemplate{}
-
-	// TestOverrideBundleCount is a variable for testing to reduce the size (number of metrics) buffered before
-	// Stackdriver will send a bundled metric report. Only applies if non-zero.
-	TestOverrideBundleCount = 0
 )
 
 type resourceTemplate struct {
@@ -99,6 +98,42 @@ type resourceTemplate struct {
 	LabelKeys sets.String
 }
 
+func sdinit(ctx context.Context, m map[string]string, mc *metricsConfig, ops ExporterOptions) error {
+	// If stackdriverClientConfig is not provided for stackdriver backend destination, OpenCensus will try to
+	// use the application default credentials. If that is not available, Opencensus would fail to create the
+	// metrics exporter.
+	scc := NewStackdriverClientConfigFromMap(m)
+	mc.stackdriverClientConfig = *scc
+	mc.isStackdriverBackend = true
+	var allowCustomMetrics bool
+	var err error
+	mc.stackdriverMetricTypePrefix = path.Join(mc.domain, mc.component)
+
+	customMetricsSubDomain := m[stackdriverCustomMetricSubDomainKey]
+	if customMetricsSubDomain == "" {
+		customMetricsSubDomain = defaultCustomMetricSubDomain
+	}
+	mc.stackdriverCustomMetricTypePrefix = path.Join(customMetricTypePrefix, customMetricsSubDomain, mc.component)
+	if ascmStr := m[allowStackdriverCustomMetricsKey]; ascmStr != "" {
+		allowCustomMetrics, err = strconv.ParseBool(ascmStr)
+		if err != nil {
+			return fmt.Errorf("invalid %s value %q", allowStackdriverCustomMetricsKey, ascmStr)
+		}
+	}
+
+	mc.recorder = sdCustomMetricsRecorder(*mc, allowCustomMetrics)
+
+	if scc.UseSecret {
+		secret, err := getStackdriverSecret(ctx, ops.Secrets)
+		if err != nil {
+			return err
+		}
+
+		mc.secret = secret
+	}
+	return nil
+}
+
 // SetStackdriverSecretLocation sets the name and namespace of the Secret that can be used to authenticate with Stackdriver.
 // The Secret is only used if both:
 // 1. This function has been explicitly called to set the name and namespace


Test patch:
diff --git a/metrics/config_test.go b/metrics/config_test.go
index 53fa89639..890f27ddb 100644
--- a/metrics/config_test.go
+++ b/metrics/config_test.go
@@ -1,3 +1,5 @@
+// +build !nostackdriver
+
 /*
 Copyright 2018 The Knative Authors
 
@@ -47,7 +49,6 @@ const (
 	eventingDomain         = "knative.dev/eventing"
 	internalEventingDomain = "knative.dev/internal/eventing"
 	customSubDomain        = "test.domain"
-	testComponent          = "testComponent"
 	testProj               = "test-project"
 	anotherProj            = "another-project"
 )
diff --git a/metrics/depcheck_test.go b/metrics/depcheck_test.go
new file mode 100644
index 000000000..28ea56b17
--- /dev/null
+++ b/metrics/depcheck_test.go
@@ -0,0 +1,29 @@
+/*
+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 metrics_test
+
+import (
+	"testing"
+
+	"knative.dev/pkg/depcheck"
+)
+
+func TestNoDeps(t *testing.T) {
+	depcheck.AssertNoDependency(t, map[string][]string{
+		"knative.dev/pkg/metrics": depcheck.KnownHeavyDependencies,
+	}, "-tags=nostackdriver")
+}
diff --git a/metrics/e2e_test.go b/metrics/e2e_test.go
index 2163f0c1b..b8ec21d32 100644
--- a/metrics/e2e_test.go
+++ b/metrics/e2e_test.go
@@ -1,3 +1,5 @@
+// +build !nostackdriver
+
 /*
 Copyright 2020 The Knative Authors
 
diff --git a/metrics/exporter_test.go b/metrics/exporter_test.go
index 871c8eb4c..c40f44c80 100644
--- a/metrics/exporter_test.go
+++ b/metrics/exporter_test.go
@@ -1,15 +1,21 @@
+// +build !nostackdriver
+
 /*
-Copyright 2018 The Knative Authors.
+Copyright 2018 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 metrics
 
 import (
diff --git a/metrics/prometheus_exporter_test.go b/metrics/prometheus_exporter_test.go
index 447458e4a..ef2dcf687 100644
--- a/metrics/prometheus_exporter_test.go
+++ b/metrics/prometheus_exporter_test.go
@@ -29,7 +29,7 @@ func TestNewPrometheusExporter(t *testing.T) {
 	}{{
 		name: "port 9090",
 		config: metricsConfig{
-			domain:             servingDomain,
+			domain:             "does not matter",
 			component:          testComponent,
 			backendDestination: prometheus,
 			prometheusPort:     9090,
@@ -39,7 +39,7 @@ func TestNewPrometheusExporter(t *testing.T) {
 	}, {
 		name: "port 9091",
 		config: metricsConfig{
-			domain:             servingDomain,
+			domain:             "does not matter",
 			component:          testComponent,
 			backendDestination: prometheus,
 			prometheusPort:     9091,
@@ -64,7 +64,7 @@ func TestNewPrometheusExporter(t *testing.T) {
 func TestNewPrometheusExporter_fromEnv(t *testing.T) {
 	exporterOptions := ExporterOptions{
 		ConfigMap: map[string]string{},
-		Domain:    servingDomain,
+		Domain:    "does not matter",
 		Component: testComponent,
 	}
 	testCases := []struct {
diff --git a/metrics/stackdriver_exporter_test.go b/metrics/stackdriver_exporter_test.go
index 7586abb10..6a083160a 100644
--- a/metrics/stackdriver_exporter_test.go
+++ b/metrics/stackdriver_exporter_test.go
@@ -1,3 +1,5 @@
+// +build !nostackdriver
+
 /*
 Copyright 2019 The Knative Authors
 
diff --git a/metrics/testing.go b/metrics/testing.go
index eb048f9cb..28b2e5878 100644
--- a/metrics/testing.go
+++ b/metrics/testing.go
@@ -16,6 +16,10 @@ limitations under the License.
 
 package metrics
 
+const (
+	testComponent = "testComponent"
+)
+
 // InitForTesting initialize the necessary global variables for unit tests.
 func InitForTesting() {
 	setCurMetricsConfig(&metricsConfig{
diff --git a/metrics/workqueue_test.go b/metrics/workqueue_test.go
index e9d021635..34af58cff 100644
--- a/metrics/workqueue_test.go
+++ b/metrics/workqueue_test.go
@@ -22,8 +22,10 @@ import (
 
 	"go.opencensus.io/stats"
 	"go.opencensus.io/stats/view"
+	"golang.org/x/net/context"
 	"k8s.io/client-go/util/workqueue"
 
+	"knative.dev/pkg/logging"
 	"knative.dev/pkg/metrics/metricstest"
 )
 
@@ -64,6 +66,12 @@ func TestWorkqueueMetrics(t *testing.T) {
 	// Reset the metrics configuration to avoid leaked state from other tests.
 	InitForTesting()
 
+	_, ref, err := newPrometheusExporter(getCurMetricsConfig(), logging.FromContext(context.Background()))
+	if err != nil {
+		t.Fatalf("newPrometheusExporter() = %v", err)
+	}
+	setFactory(ref)
+
 	views := wp.DefaultViews()
 	if got, want := len(views), 7; got != want {
 		t.Errorf("len(DefaultViews()) = %d, want %d", got, want)