Repository: knative/pkg
Pull request #1274 (2020-05-01 20:36:15)

Add elapsed wait time in timeout message.
This would give us some ideas about the total wait time when we look at logs, to make it easier to decide if we want to adjust the timeout setting.

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

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1274#" title="Author self-approved">tcnghia</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:

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

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":[]} -->
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
---- |:------------:|:------------:|:-----:
[network/transports.go](https://storage.cloud.google.com/knative-prow/pr-logs/pull/knative_pkg/1274/pull-knative-pkg-go-coverage/1256321653030260741/artifacts/line-cov.html#file0) | 95.7% | 96.2% | 0.5


Patch:
diff --git a/metrics/config.go b/metrics/config.go
index 4403ddb3d..71161baa1 100644
--- a/metrics/config.go
+++ b/metrics/config.go
@@ -29,6 +29,7 @@ import (
 
 	"go.opencensus.io/stats"
 	"go.uber.org/zap"
+	corev1 "k8s.io/api/core/v1"
 	"knative.dev/pkg/metrics/metricskey"
 )
 
@@ -88,9 +89,8 @@ type metricsConfig struct {
 	// writing the metrics to the stats.RecordWithOptions interface.
 	recorder func(context.Context, []stats.Measurement, ...stats.Options) error
 
-	// secretFetcher provides access for fetching Kubernetes Secrets from an
-	// informer cache.
-	secretFetcher SecretFetcher
+	// secret contains credentials for an exporter to use for authentication.
+	secret *corev1.Secret
 
 	// ---- OpenCensus specific below ----
 	// collectorAddress is the address of the collector, if not `localhost:55678`
@@ -162,10 +162,6 @@ func (mc *metricsConfig) record(ctx context.Context, mss []stats.Measurement, ro
 func createMetricsConfig(ops ExporterOptions, logger *zap.SugaredLogger) (*metricsConfig, error) {
 	var mc metricsConfig
 
-	// We don't check if this is `nil` right now, because this is a transition step.
-	// Eventually, this should be a startup check.
-	mc.secretFetcher = ops.Secrets
-
 	if ops.Domain == "" {
 		return nil, errors.New("metrics domain cannot be empty")
 	}
@@ -205,6 +201,13 @@ func createMetricsConfig(ops ExporterOptions, logger *zap.SugaredLogger) (*metri
 			if mc.requireSecure, err = strconv.ParseBool(isSecure); err != nil {
 				return nil, fmt.Errorf("invalid %s value %q", CollectorSecureKey, isSecure)
 			}
+
+			if mc.requireSecure {
+				mc.secret, err = getOpenCensusSecret(ops.Component, ops.Secrets)
+				if err != nil {
+					return nil, err
+				}
+			}
 		}
 	}
 
@@ -265,6 +268,15 @@ func createMetricsConfig(ops ExporterOptions, logger *zap.SugaredLogger) (*metri
 				return stats.RecordWithOptions(ctx, append(ros, stats.WithMeasurements(mss...))...)
 			}
 		}
+
+		if scc.UseSecret {
+			secret, err := getStackdriverSecret(ops.Secrets)
+			if err != nil {
+				return nil, err
+			}
+
+			mc.secret = secret
+		}
 	}
 
 	// If reporting period is specified, use the value from the configuration.
diff --git a/metrics/exporter.go b/metrics/exporter.go
index f3f0006e4..b8f4fd12f 100644
--- a/metrics/exporter.go
+++ b/metrics/exporter.go
@@ -121,6 +121,7 @@ func UpdateExporterFromConfigMapWithOpts(opts ExporterOptions, logger *zap.Sugar
 			Component:      opts.Component,
 			ConfigMap:      configMap.Data,
 			PrometheusPort: opts.PrometheusPort,
+			Secrets:        opts.Secrets,
 		}, logger)
 	}, nil
 }
@@ -130,6 +131,7 @@ func UpdateExporterFromConfigMapWithOpts(opts ExporterOptions, logger *zap.Sugar
 // to prevent a race condition between reading the current configuration
 // and updating the current exporter.
 func UpdateExporter(ops ExporterOptions, logger *zap.SugaredLogger) error {
+	// TODO(https://github.com/knative/pkg/issues/1273): check if ops.secrets is `nil` after new metrics plan lands
 	newConfig, err := createMetricsConfig(ops, logger)
 	if err != nil {
 		if getCurMetricsConfig() == nil {
@@ -141,28 +143,33 @@ func UpdateExporter(ops ExporterOptions, logger *zap.SugaredLogger) error {
 		return err
 	}
 
+	// Updating the metrics config and the metrics exporters needs to be atomic to
+	// avoid using an outdated metrics config with new exporters.
+	metricsMux.Lock()
+	defer metricsMux.Unlock()
+
 	if isNewExporterRequired(newConfig) {
 		logger.Info("Flushing the existing exporter before setting up the new exporter.")
-		FlushExporter()
+		flushGivenExporter(curMetricsExporter)
 		e, err := newMetricsExporter(newConfig, logger)
 		if err != nil {
 			logger.Errorf("Failed to update a new metrics exporter based on metric config %v. error: %v", newConfig, err)
 			return err
 		}
-		existingConfig := getCurMetricsConfig()
-		setCurMetricsExporter(e)
+		existingConfig := curMetricsConfig
+		curMetricsExporter = e
 		logger.Infof("Successfully updated the metrics exporter; old config: %v; new config %v", existingConfig, newConfig)
 	}
 
-	setCurMetricsConfig(newConfig)
+	setCurMetricsConfigUnlocked(newConfig)
 	return nil
 }
 
 // isNewExporterRequired compares the non-nil newConfig against curMetricsConfig. When backend changes,
 // or stackdriver project ID changes for stackdriver backend, we need to update the metrics exporter.
-// This function is not implicitly thread-safe.
+// This function must be called with the metricsMux reader (or writer) locked.
 func isNewExporterRequired(newConfig *metricsConfig) bool {
-	cc := getCurMetricsConfig()
+	cc := curMetricsConfig
 	if cc == nil || newConfig.backendDestination != cc.backendDestination {
 		return true
 	}
@@ -177,15 +184,14 @@ func isNewExporterRequired(newConfig *metricsConfig) bool {
 }
 
 // newMetricsExporter gets a metrics exporter based on the config.
-// This function is not implicitly thread-safe.
+// This function must be called with the metricsMux reader (or writer) locked.
 func newMetricsExporter(config *metricsConfig, logger *zap.SugaredLogger) (view.Exporter, error) {
-	ce := getCurMetricsExporter()
 	// If there is a Prometheus Exporter server running, stop it.
 	resetCurPromSrv()
 
 	// TODO(https://github.com/knative/pkg/issues/866): Move Stackdriver and Promethus
 	// operations before stopping to an interface.
-	if se, ok := ce.(stoppable); ok {
+	if se, ok := curMetricsExporter.(stoppable); ok {
 		se.StopMetricsExporter()
 	}
 
@@ -230,6 +236,10 @@ func getCurMetricsConfig() *metricsConfig {
 func setCurMetricsConfig(c *metricsConfig) {
 	metricsMux.Lock()
 	defer metricsMux.Unlock()
+	setCurMetricsConfigUnlocked(c)
+}
+
+func setCurMetricsConfigUnlocked(c *metricsConfig) {
 	if c != nil {
 		view.SetReportingPeriod(c.reportingPeriod)
 	} else {
@@ -244,6 +254,10 @@ func setCurMetricsConfig(c *metricsConfig) {
 // Return value indicates whether the exporter is flushable or not.
 func FlushExporter() bool {
 	e := getCurMetricsExporter()
+	return flushGivenExporter(e)
+}
+
+func flushGivenExporter(e view.Exporter) bool {
 	if e == nil {
 		return false
 	}
diff --git a/metrics/opencensus_exporter.go b/metrics/opencensus_exporter.go
index 5e3924646..dd80c855d 100644
--- a/metrics/opencensus_exporter.go
+++ b/metrics/opencensus_exporter.go
@@ -24,6 +24,7 @@ import (
 	"go.opencensus.io/stats/view"
 	"go.uber.org/zap"
 	"google.golang.org/grpc/credentials"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 )
 
@@ -33,49 +34,53 @@ func newOpenCensusExporter(config *metricsConfig, logger *zap.SugaredLogger) (vi
 		opts = append(opts, ocagent.WithAddress(config.collectorAddress))
 	}
 	if config.requireSecure {
-		opts = append(opts, ocagent.WithTLSCredentials(credentialFetcher(config.component, config.secretFetcher, logger)))
+		opts = append(opts, ocagent.WithTLSCredentials(getCredentials(config.component, config.secret, logger)))
 	} else {
 		opts = append(opts, ocagent.WithInsecure())
 	}
 	e, err := ocagent.NewExporter(opts...)
 	if err != nil {
-		logger.Errorw("Failed to create the OpenCensus exporter.", zap.Error(err))
+		logger.Errorw("failed to create the OpenCensus exporter.", zap.Error(err))
 		return nil, err
 	}
-	logger.Infof("Created OpenCensus exporter with config: %+v.", *config)
+	logger.Infof("created OpenCensus exporter with config: %+v.", *config)
 	view.RegisterExporter(e)
 	return e, nil
 }
 
-// credentialFetcher attempts to locate a secret containing TLS credentials
+// getOpenCensusSecret attempts to locate a secret containing TLS credentials
 // for communicating with the OpenCensus Agent. To do this, it first looks
 // for a secret named "<component>-opencensus", then for a generic
 // "opencensus" secret.
-func credentialFetcher(component string, lister SecretFetcher, logger *zap.SugaredLogger) credentials.TransportCredentials {
+func getOpenCensusSecret(component string, lister SecretFetcher) (*corev1.Secret, error) {
 	if lister == nil {
-		logger.Errorf("No secret lister provided for component %q; cannot use requireSecure=true", component)
+		return nil, fmt.Errorf("no secret lister provided for component %q; cannot use requireSecure=true", component)
+	}
+	secret, err := lister(component + "-opencensus")
+	if errors.IsNotFound(err) {
+		secret, err = lister("opencensus")
+	}
+	if err != nil {
+		return nil, fmt.Errorf("unable to fetch opencensus secret for %q, cannot use requireSecure=true: %+v", component, err)
+	}
+
+	return secret, nil
+}
+
+// getCredentials attempts to create a certificate containing TLS credentials
+// for communicating with the OpenCensus Agent.
+func getCredentials(component string, secret *corev1.Secret, logger *zap.SugaredLogger) credentials.TransportCredentials {
+	if secret == nil {
+		logger.Errorf("no secret provided for component %q; cannot use requireSecure=true", component)
 		return nil
 	}
 	return credentials.NewTLS(&tls.Config{
 		GetClientCertificate: func(*tls.CertificateRequestInfo) (*tls.Certificate, error) {
-			// We ignore the CertificateRequestInfo for now, and hand back a single fixed certificate.
-			// TODO(evankanderson): maybe do something SPIFFE-ier?
-			cert, err := certificateFetcher(component+"-opencensus", lister)
-			if errors.IsNotFound(err) {
-				cert, err = certificateFetcher("opencensus", lister)
-			}
+			cert, err := tls.X509KeyPair(secret.Data["client-cert.pem"], secret.Data["client-key.pem"])
 			if err != nil {
-				return nil, fmt.Errorf("Unable to fetch opencensus secret for %q, cannot use requireSecure=true: %+v", component, err)
+				return nil, err
 			}
-			return &cert, err
+			return &cert, nil
 		},
 	})
 }
-
-func certificateFetcher(secretName string, lister SecretFetcher) (tls.Certificate, error) {
-	secret, err := lister(secretName)
-	if err != nil {
-		return tls.Certificate{}, err
-	}
-	return tls.X509KeyPair(secret.Data["client-cert.pem"], secret.Data["client-key.pem"])
-}
diff --git a/metrics/stackdriver_exporter.go b/metrics/stackdriver_exporter.go
index e87a3e37f..87bb722a2 100644
--- a/metrics/stackdriver_exporter.go
+++ b/metrics/stackdriver_exporter.go
@@ -115,7 +115,7 @@ func newOpencensusSDExporter(o stackdriver.Options) (view.Exporter, error) {
 func newStackdriverExporter(config *metricsConfig, logger *zap.SugaredLogger) (view.Exporter, error) {
 	gm := getMergedGCPMetadata(config)
 	mpf := getMetricPrefixFunc(config.stackdriverMetricTypePrefix, config.stackdriverCustomMetricTypePrefix)
-	co, err := getStackdriverExporterClientOptions(&config.stackdriverClientConfig)
+	co, err := getStackdriverExporterClientOptions(config)
 	if err != nil {
 		logger.Warnw("Issue configuring Stackdriver exporter client options, no additional client options will be used: ", zap.Error(err))
 	}
@@ -140,21 +140,21 @@ func newStackdriverExporter(config *metricsConfig, logger *zap.SugaredLogger) (v
 
 // getStackdriverExporterClientOptions creates client options for the opencensus Stackdriver exporter from the given stackdriverClientConfig.
 // On error, an empty array of client options is returned.
-func getStackdriverExporterClientOptions(sdconfig *StackdriverClientConfig) ([]option.ClientOption, error) {
+func getStackdriverExporterClientOptions(config *metricsConfig) ([]option.ClientOption, error) {
 	var co []option.ClientOption
-	if sdconfig.UseSecret && useStackdriverSecretEnabled {
-		secret, err := getStackdriverSecret(sdconfig)
-		if err != nil {
-			return co, err
+
+	// SetStackdriverSecretLocation must have been called by calling package for this to work.
+	if config.stackdriverClientConfig.UseSecret {
+		if config.secret == nil {
+			return co, fmt.Errorf("No secret provided for component %q; cannot use stackdriver-use-secret=true", config.component)
 		}
 
-		if opt, err := convertSecretToExporterOption(secret); err == nil {
+		if opt, err := convertSecretToExporterOption(config.secret); err == nil {
 			co = append(co, opt)
 		} else {
 			return co, err
 		}
 	}
-
 	return co, nil
 }
 
@@ -215,19 +215,31 @@ func getMetricPrefixFunc(metricTypePrefix, customMetricTypePrefix string) func(n
 }
 
 // getStackdriverSecret returns the Kubernetes Secret specified in the given config.
+// SetStackdriverSecretLocation must have been called by calling package for this to work.
 // TODO(anniefu): Update exporter if Secret changes (https://github.com/knative/pkg/issues/842)
-func getStackdriverSecret(sdconfig *StackdriverClientConfig) (*corev1.Secret, error) {
-	if err := ensureKubeclient(); err != nil {
-		return nil, err
-	}
-
+func getStackdriverSecret(secretFetcher SecretFetcher) (*corev1.Secret, error) {
 	stackdriverMtx.RLock()
 	defer stackdriverMtx.RUnlock()
 
-	sec, secErr := kubeclient.CoreV1().Secrets(secretNamespace).Get(secretName, metav1.GetOptions{})
+	if !useStackdriverSecretEnabled {
+		return nil, nil
+	}
+
+	var secErr error
+	var sec *corev1.Secret
+	if secretFetcher != nil {
+		sec, secErr = secretFetcher(fmt.Sprintf("%s/%s", secretNamespace, secretName))
+	} else {
+		// This else-block can be removed once UpdateExporterFromConfigMap is fully deprecated in favor of ConfigMapWatcher
+		if err := ensureKubeclient(); err != nil {
+			return nil, err
+		}
+
+		sec, secErr = kubeclient.CoreV1().Secrets(secretNamespace).Get(secretName, metav1.GetOptions{})
+	}
 
 	if secErr != nil {
-		return nil, fmt.Errorf("Error getting Secret [%s] in namespace [%s]: %w", secretName, secretNamespace, secErr)
+		return nil, fmt.Errorf("error getting Secret [%v] in namespace [%v]: %v", secretName, secretNamespace, secErr)
 	}
 
 	return sec, nil
diff --git a/network/transports.go b/network/transports.go
index 1ff6f4615..019f4ba87 100644
--- a/network/transports.go
+++ b/network/transports.go
@@ -19,6 +19,7 @@ package network
 import (
 	"context"
 	"errors"
+	"fmt"
 	"net"
 	"net/http"
 	"time"
@@ -74,6 +75,7 @@ func dialBackOffHelper(ctx context.Context, network, address string, bo wait.Bac
 		KeepAlive: 5 * time.Second,
 		DualStack: true,
 	}
+	start := time.Now()
 	for {
 		c, err := dialer.DialContext(ctx, network, address)
 		if err != nil {
@@ -89,7 +91,8 @@ func dialBackOffHelper(ctx context.Context, network, address string, bo wait.Bac
 		}
 		return c, nil
 	}
-	return nil, errDialTimeout
+	elapsed := time.Now().Sub(start)
+	return nil, fmt.Errorf("timed out dialing after %.2fs", elapsed.Seconds())
 }
 
 func newHTTPTransport(connTimeout time.Duration, disableKeepAlives bool) http.RoundTripper {


Test patch:
diff --git a/metrics/config_test.go b/metrics/config_test.go
index 914f9ed52..1fd0d18dd 100644
--- a/metrics/config_test.go
+++ b/metrics/config_test.go
@@ -32,6 +32,7 @@ import (
 	"knative.dev/pkg/metrics/metricstest"
 
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 // TODO UTs should move to eventing and serving, as appropriate.
@@ -201,6 +202,12 @@ var (
 				},
 				Domain:    servingDomain,
 				Component: testComponent,
+				Secrets: fakeSecretList(corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      StackdriverSecretNameDefault,
+						Namespace: StackdriverSecretNamespaceDefault,
+					},
+				}).Get,
 			},
 			expectedConfig: metricsConfig{
 				domain:                            servingDomain,
@@ -216,6 +223,12 @@ var (
 					ClusterName: "cluster",
 					UseSecret:   true,
 				},
+				secret: &corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      StackdriverSecretNameDefault,
+						Namespace: StackdriverSecretNamespaceDefault,
+					},
+				},
 			},
 			expectedNewExporter: true,
 		}, {
@@ -253,6 +266,15 @@ var (
 				},
 				Domain:    servingDomain,
 				Component: testComponent,
+				Secrets: fakeSecretList(corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "opencensus",
+					},
+					Data: map[string][]byte{
+						"client-cert.pem": {},
+						"client-key.pem":  {},
+					},
+				}).Get,
 			},
 			expectedConfig: metricsConfig{
 				domain:             servingDomain,
@@ -260,6 +282,15 @@ var (
 				backendDestination: OpenCensus,
 				collectorAddress:   "external-svc:55678",
 				requireSecure:      true,
+				secret: &corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "opencensus",
+					},
+					Data: map[string][]byte{
+						"client-cert.pem": {},
+						"client-key.pem":  {},
+					},
+				},
 			},
 			expectedNewExporter: true,
 		}, {
@@ -514,6 +545,10 @@ var (
 		}}
 )
 
+func successTestsInit() {
+	SetStackdriverSecretLocation(StackdriverSecretNameDefault, StackdriverSecretNamespaceDefault)
+}
+
 func TestGetMetricsConfig(t *testing.T) {
 	for _, test := range errorTests {
 		t.Run(test.name, func(t *testing.T) {
@@ -525,6 +560,7 @@ func TestGetMetricsConfig(t *testing.T) {
 		})
 	}
 
+	successTestsInit()
 	for _, test := range successTests {
 		t.Run(test.name, func(t *testing.T) {
 			defer ClearAll()
@@ -558,6 +594,7 @@ func TestGetMetricsConfig_fromEnv(t *testing.T) {
 
 func TestIsNewExporterRequiredFromNilConfig(t *testing.T) {
 	setCurMetricsConfig(nil)
+	successTestsInit()
 	for _, test := range successTests {
 		t.Run(test.name, func(t *testing.T) {
 			defer ClearAll()
@@ -685,6 +722,7 @@ func TestIsNewExporterRequired(t *testing.T) {
 func TestUpdateExporter(t *testing.T) {
 	setCurMetricsConfig(nil)
 	oldConfig := getCurMetricsConfig()
+	successTestsInit()
 	for _, test := range successTests[1:] {
 		t.Run(test.name, func(t *testing.T) {
 			defer ClearAll()
@@ -715,6 +753,7 @@ func TestUpdateExporter(t *testing.T) {
 func TestUpdateExporterFromConfigMapWithOpts(t *testing.T) {
 	setCurMetricsConfig(nil)
 	oldConfig := getCurMetricsConfig()
+	successTestsInit()
 	for _, test := range successTests[1:] {
 		t.Run(test.name, func(t *testing.T) {
 			defer ClearAll()
@@ -722,6 +761,7 @@ func TestUpdateExporterFromConfigMapWithOpts(t *testing.T) {
 				Component:      test.ops.Component,
 				Domain:         test.ops.Domain,
 				PrometheusPort: test.ops.PrometheusPort,
+				Secrets:        test.ops.Secrets,
 			}
 			updateFunc, err := UpdateExporterFromConfigMapWithOpts(opts, TestLogger(t))
 			if err != nil {
diff --git a/metrics/opencensus_exporter_test.go b/metrics/opencensus_exporter_test.go
index c21380fe6..40ba726cd 100644
--- a/metrics/opencensus_exporter_test.go
+++ b/metrics/opencensus_exporter_test.go
@@ -65,7 +65,7 @@ func TestOpenCensusConfig(t *testing.T) {
 			domain:             "secure",
 			component:          "test",
 			backendDestination: OpenCensus,
-			secretFetcher: fakeSecretList(corev1.Secret{
+			secret: &corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "test-opencensus",
 				},
@@ -73,7 +73,7 @@ func TestOpenCensusConfig(t *testing.T) {
 					"client-cert.pem": cert,
 					"client-key.pem":  key,
 				},
-			}).Get,
+			},
 			requireSecure: true,
 		},
 		tls: &tls.Config{},
@@ -140,6 +140,10 @@ func fakeSecretList(s ...corev1.Secret) *fakeSecrets {
 
 func (f *fakeSecrets) Get(name string) (*corev1.Secret, error) {
 	for _, s := range f.secrets {
+		if fmt.Sprintf("%s/%s", s.Namespace, s.Name) == name {
+			return &s, nil
+		}
+
 		if s.Name == name {
 			return &s, nil
 		}
diff --git a/metrics/stackdriver_exporter_test.go b/metrics/stackdriver_exporter_test.go
index 6f4663d03..e88901045 100644
--- a/metrics/stackdriver_exporter_test.go
+++ b/metrics/stackdriver_exporter_test.go
@@ -24,6 +24,8 @@ import (
 	"contrib.go.opencensus.io/exporter/stackdriver"
 	"go.opencensus.io/metric/metricdata"
 	"go.opencensus.io/stats/view"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	. "knative.dev/pkg/logging/testing"
 	"knative.dev/pkg/metrics/metricskey"
 )
@@ -471,48 +473,45 @@ func assertStringsEqual(t *testing.T, description string, expected string, actua
 }
 
 func TestSetStackdriverSecretLocation(t *testing.T) {
+	// Prevent pollution from other tests
+	useStackdriverSecretEnabled = false
 	// Reset global state after test
 	defer func() {
 		secretName = StackdriverSecretNameDefault
 		secretNamespace = StackdriverSecretNamespaceDefault
+		useStackdriverSecretEnabled = false
 	}()
 
-	sdConfig := &StackdriverClientConfig{
-		ProjectID:   "project",
-		GCPLocation: "us-west2",
-		ClusterName: "cluster",
-		UseSecret:   false,
+	const testName, testNamespace = "test-name", "test-namespace"
+	secretFetcher := func(name string) (*corev1.Secret, error) {
+		return &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      testName,
+				Namespace: testNamespace,
+			},
+		}, nil
 	}
 
 	// Sanity checks
 	assertStringsEqual(t, "DefaultSecretName", secretName, StackdriverSecretNameDefault)
 	assertStringsEqual(t, "DefaultSecretNamespace", secretNamespace, StackdriverSecretNamespaceDefault)
-	if _, err := getStackdriverExporterClientOptions(sdConfig); err != nil {
-		t.Errorf("Got unexpected error when creating exporter client options: [%v]", err)
+	sec, err := getStackdriverSecret(secretFetcher)
+	if err != nil {
+		t.Errorf("Got unexpected error when getting secret: %v", err)
 	}
-
-	// Check configuration's UseSecret value is ignored until the consuming package calls SetStackdriverSecretLocation
-	// If an attempt to read a Secret was made, there should be an error because there's no valid in-cluster kubeclient.
-	sdConfig.UseSecret = true
-	if _, e1 := getStackdriverExporterClientOptions(sdConfig); e1 != nil {
-		t.Errorf("Got unexpected error when creating exporter client options: [%v]", e1)
+	if sec != nil {
+		t.Errorf("Stackdriver secret should not be fetched unless SetStackdriverSecretLocation has been called")
 	}
 
-	testName, testNamespace := "test-name", "test-namespace"
-	// SetStackdriverSecretLocation has been called & config's UseSecret value is set
-	// There should be an attempt to read the Secret, and an error because there's no valid in-cluster kubeclient.
-	SetStackdriverSecretLocation("test-name", "test-namespace")
-	if _, e1 := getStackdriverExporterClientOptions(sdConfig); e1 == nil {
-		t.Errorf("Expected a known error when getting exporter options with Secrets enabled (cannot create valid kubeclient in tests). Did the function run as expected?")
+	// Once SetStackdriverSecretLocation has been called, attempts to get the secret should complete.
+	SetStackdriverSecretLocation(testName, testNamespace)
+	sec, err = getStackdriverSecret(secretFetcher)
+	if err != nil {
+		t.Errorf("Got unexpected error when getting secret: %v", err)
+	}
+	if sec == nil {
+		t.Error("expected secret to be non-nil if there is no error and SetStackdriverSecretLocation has been called")
 	}
 	assertStringsEqual(t, "secretName", secretName, testName)
 	assertStringsEqual(t, "secretNamespace", secretNamespace, testNamespace)
-
-	randomName, randomNamespace := "random-name", "random-namespace"
-	SetStackdriverSecretLocation(randomName, randomNamespace)
-	if _, e1 := getStackdriverExporterClientOptions(sdConfig); e1 == nil {
-		t.Errorf("Expected a known error when getting exporter options with Secrets enabled (cannot create valid kubeclient in tests). Did the function run as expected?")
-	}
-	assertStringsEqual(t, "secretName", secretName, randomName)
-	assertStringsEqual(t, "secretNamespace", secretNamespace, randomNamespace)
 }
diff --git a/network/transports_test.go b/network/transports_test.go
index b7d3ae97b..f8f2e837d 100644
--- a/network/transports_test.go
+++ b/network/transports_test.go
@@ -90,8 +90,9 @@ func TestDialWithBackoff(t *testing.T) {
 		c.Close()
 		t.Error("Unexpected success dialing")
 	}
-	if err != errDialTimeout {
-		t.Errorf("Error = %v, want: %v", err, errDialTimeout)
+	const expectedErrPrefix = "timed out dialing"
+	if err == nil || !strings.HasPrefix(err.Error(), expectedErrPrefix) {
+		t.Errorf("Error = %v, want: %s(...)", err, expectedErrPrefix)
 	}
 
 	s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))