From 7530bc3d10e7191d9a7d5845ae30184e9d6be77a Mon Sep 17 00:00:00 2001
From: Mike Dame <mikedame@google.com>
Date: Fri, 23 Jun 2023 15:17:20 +0000
Subject: [PATCH] feedback

---
 exporter/collector/README.md  |  4 ++--
 exporter/collector/config.go  |  6 +++---
 exporter/collector/metrics.go | 18 ++++++++----------
 3 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/exporter/collector/README.md b/exporter/collector/README.md
index 86ada17a2..5fb3c43bc 100644
--- a/exporter/collector/README.md
+++ b/exporter/collector/README.md
@@ -180,8 +180,8 @@ Additional configuration for the metric exporter:
 - `metric.prefix` (optional): MetricPrefix overrides the prefix / namespace of the Google Cloud metric type identifier. If not set, defaults to "custom.googleapis.com/opencensus/"
 - `metric.skip_create_descriptor` (optional): Whether to skip creating the
   metric descriptor.
-- `metric.wal.directory` (optional): Path to local write-ahead-log file.
-- `metric.wal.max_backoff` (optional): Maximum duration to retry entries from
+- `metric.experimental_wal.experimental_directory` (optional): Path to local write-ahead-log file.
+- `metric.experimental_wal.experimental_max_backoff` (optional): Maximum duration to retry entries from
   the WAL on network errors.
 
 Addition configuration for the logging exporter:
diff --git a/exporter/collector/config.go b/exporter/collector/config.go
index a9fc76eb5..c0207a1aa 100644
--- a/exporter/collector/config.go
+++ b/exporter/collector/config.go
@@ -104,7 +104,7 @@ type MetricConfig struct {
 	// suffixes for summary metrics, but does not (yet) include the domain prefix
 	GetMetricName func(baseName string, metric pmetric.Metric) (string, error)
 	// WALConfig holds configuration settings for the write ahead log.
-	WALConfig *WALConfig `mapstructure:"wal_config"`
+	WALConfig *WALConfig `mapstructure:"experimental_wal_config"`
 	Prefix    string     `mapstructure:"prefix"`
 	// KnownDomains contains a list of prefixes. If a metric already has one
 	// of these prefixes, the prefix is not added.
@@ -148,9 +148,9 @@ type MetricConfig struct {
 // it preserves both the data on disk and the order of the data points.
 type WALConfig struct {
 	// Directory is the location to store WAL files.
-	Directory string `mapstructure:"directory"`
+	Directory string `mapstructure:"experimental_directory"`
 	// MaxBackoff sets the length of time to exponentially re-try failed exports.
-	MaxBackoff time.Duration `mapstructure:"max_backoff"`
+	MaxBackoff time.Duration `mapstructure:"experimental_max_backoff"`
 }
 
 // ImpersonateConfig defines configuration for service account impersonation.
diff --git a/exporter/collector/metrics.go b/exporter/collector/metrics.go
index 0ad600a3e..c71186fd6 100644
--- a/exporter/collector/metrics.go
+++ b/exporter/collector/metrics.go
@@ -81,7 +81,8 @@ type MetricsExporter struct {
 	// A channel that receives metric descriptor and sends them to GCM once
 	metricDescriptorC chan *monitoringpb.CreateMetricDescriptorRequest
 	client            *monitoring.MetricClient
-	exportFunc        func(context.Context, *monitoringpb.CreateTimeSeriesRequest) error
+	// Only used for testing purposes in lieu of initializing a fake client
+	exportFunc func(context.Context, *monitoringpb.CreateTimeSeriesRequest) error
 	// requestOpts applies options to the context for requests, such as additional headers.
 	requestOpts []func(*context.Context, requestInfo)
 	mapper      metricMapper
@@ -138,12 +139,6 @@ func (me *MetricsExporter) Shutdown(ctx context.Context) error {
 	go func() {
 		// Wait until all goroutines are done
 		me.goroutines.Wait()
-		// Close the WAL if open
-		if me.wal != nil {
-			if err := me.wal.Close(); err != nil {
-				me.obs.log.Error(fmt.Sprintf("error closing WAL: %+v\n", err))
-			}
-		}
 		close(c)
 	}()
 	select {
@@ -573,8 +568,11 @@ func (me *MetricsExporter) watchWALFile(ctx context.Context) error {
 
 func (me *MetricsExporter) runWALReadAndExportLoop(ctx context.Context) {
 	defer me.goroutines.Done()
-	runCtx, cancel := context.WithCancel(ctx)
-	defer cancel()
+	defer func() {
+		if err := me.wal.Close(); err != nil {
+			me.obs.log.Error(fmt.Sprintf("error closing WAL: %+v\n", err))
+		}
+	}()
 	for {
 		select {
 		case <-ctx.Done():
@@ -583,7 +581,7 @@ func (me *MetricsExporter) runWALReadAndExportLoop(ctx context.Context) {
 			// do one last final read/export then return
 			// otherwise the runner goroutine could leave some hanging metrics unexported
 			for {
-				err := me.readWALAndExport(runCtx)
+				err := me.readWALAndExport(ctx)
 				if err != nil {
 					if !errors.Is(err, wal.ErrOutOfRange) {
 						me.obs.log.Error(fmt.Sprintf("error flushing remaining WAL entries: %+v", err))