From 11f67cea8ee22b1f01e409473be6c75733874681 Mon Sep 17 00:00:00 2001
From: Joshua MacDonald <jmacd@users.noreply.github.com>
Date: Mon, 23 Dec 2019 16:38:35 -0800
Subject: [PATCH] Comments in the metrics SDK (#399)

* Comments from walkthrough

* Update
---
 sdk/metric/batcher/defaultkeys/defaultkeys.go | 4 ++++
 sdk/metric/batcher/ungrouped/ungrouped.go     | 7 +++++++
 sdk/metric/controller/push/push.go            | 2 +-
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/sdk/metric/batcher/defaultkeys/defaultkeys.go b/sdk/metric/batcher/defaultkeys/defaultkeys.go
index 56da38a65..4b0a36ebb 100644
--- a/sdk/metric/batcher/defaultkeys/defaultkeys.go
+++ b/sdk/metric/batcher/defaultkeys/defaultkeys.go
@@ -115,6 +115,8 @@ func (b *Batcher) Process(_ context.Context, record export.Record) error {
 	}
 	rag, ok := b.aggCheckpoint[key]
 	if ok {
+		// Combine the input aggregator with the current
+		// checkpoint state.
 		return rag.Aggregator().Merge(agg, desc)
 	}
 	// If this Batcher is stateful, create a copy of the
@@ -123,6 +125,8 @@ func (b *Batcher) Process(_ context.Context, record export.Record) error {
 	// again, overwriting the long-lived state.
 	if b.stateful {
 		tmp := agg
+		// Note: the call to AggregatorFor() followed by Merge
+		// is effectively a Clone() operation.
 		agg = b.AggregatorFor(desc)
 		if err := agg.Merge(tmp, desc); err != nil {
 			return err
diff --git a/sdk/metric/batcher/ungrouped/ungrouped.go b/sdk/metric/batcher/ungrouped/ungrouped.go
index 2bda79ddb..76f53fc2b 100644
--- a/sdk/metric/batcher/ungrouped/ungrouped.go
+++ b/sdk/metric/batcher/ungrouped/ungrouped.go
@@ -64,6 +64,11 @@ func (b *Batcher) Process(_ context.Context, record export.Record) error {
 	agg := record.Aggregator()
 	value, ok := b.batchMap[key]
 	if ok {
+		// Note: The call to Merge here combines only
+		// identical records.  It is required even for a
+		// stateless Batcher because such identical records
+		// may arise in the Meter implementation due to race
+		// conditions.
 		return value.aggregator.Merge(agg, desc)
 	}
 	// If this Batcher is stateful, create a copy of the
@@ -72,6 +77,8 @@ func (b *Batcher) Process(_ context.Context, record export.Record) error {
 	// again, overwriting the long-lived state.
 	if b.stateful {
 		tmp := agg
+		// Note: the call to AggregatorFor() followed by Merge
+		// is effectively a Clone() operation.
 		agg = b.AggregatorFor(desc)
 		if err := agg.Merge(tmp, desc); err != nil {
 			return err
diff --git a/sdk/metric/controller/push/push.go b/sdk/metric/controller/push/push.go
index b6aa8fbc3..cb2294a1c 100644
--- a/sdk/metric/controller/push/push.go
+++ b/sdk/metric/controller/push/push.go
@@ -108,7 +108,7 @@ func (c *Controller) SetErrorHandler(errorHandler sdk.ErrorHandler) {
 
 // Meter returns a named Meter, satisifying the metric.Provider
 // interface.
-func (c *Controller) Meter(name string) metric.Meter {
+func (c *Controller) Meter(_ string) metric.Meter {
 	return c.sdk
 }