From 06b14ce98470e4db1d82224ad37c3cadc3fb4b65 Mon Sep 17 00:00:00 2001
From: Tyler Yahn <codingalias@gmail.com>
Date: Thu, 27 Jul 2023 09:10:36 -0700
Subject: [PATCH] Revise the exemplar default reservoirs

Make it normative that an SDK is recommended to include both types of
default reservoirs.

Make the description of both default reservoirs their own sub-sections.

Correct the "SimpleExemplarReservoir" section to be titled
"SimpleFixedSizeExemplarReservoir".

Loosen the SimpleFixedSizeExemplarReservoir to allow any
uniformly-weighted sampling algorithm instead of just the naive one that
is not optimal.
---
 specification/metrics/sdk.md | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/specification/metrics/sdk.md b/specification/metrics/sdk.md
index fdfe6693cd9..263878eaff0 100644
--- a/specification/metrics/sdk.md
+++ b/specification/metrics/sdk.md
@@ -55,6 +55,8 @@ linkTitle: SDK
     + [TraceBased](#tracebased)
   * [ExemplarReservoir](#exemplarreservoir)
   * [Exemplar defaults](#exemplar-defaults)
+    + [SimpleFixedSizeExemplarReservoir](#simplefixedsizeexemplarreservoir)
+    + [AlignedHistogramBucketExemplarReservoir](#alignedhistogrambucketexemplarreservoir)
 - [MetricReader](#metricreader)
   * [MetricReader operations](#metricreader-operations)
     + [RegisterProducer(metricProducer)](#registerproducermetricproducer)
@@ -972,20 +974,21 @@ The `ExemplarReservoir` SHOULD avoid allocations when sampling exemplars.
 
 ### Exemplar defaults
 
-The SDK will come with two types of built-in exemplar reservoirs:
+The SDK SHOULD include two types of built-in exemplar reservoirs:
 
-1. SimpleFixedSizeExemplarReservoir
-2. AlignedHistogramBucketExemplarReservoir
+1. `SimpleFixedSizeExemplarReservoir`
+2. `AlignedHistogramBucketExemplarReservoir`
 
 By default, explicit bucket histogram aggregation with more than 1 bucket will
 use `AlignedHistogramBucketExemplarReservoir`. All other aggregations will use
 `SimpleFixedSizeExemplarReservoir`.
 
-_SimpleExemplarReservoir_
-This Exemplar reservoir MAY take a configuration parameter for the size of the
-reservoir pool.  The reservoir will accept measurements using an equivalent of
-the [naive reservoir sampling
-algorithm](https://en.wikipedia.org/wiki/Reservoir_sampling)
+#### SimpleFixedSizeExemplarReservoir
+
+This reservoir MUST use an uniformly-weighted sampling algorithm to determine
+if the offered measurements should be sampled. For example, the [simple
+reservoir sampling algorithm](https://en.wikipedia.org/wiki/Reservoir_sampling)
+can be used:
 
   ```
   bucket = random_integer(0, num_measurements_seen)
@@ -994,10 +997,15 @@ algorithm](https://en.wikipedia.org/wiki/Reservoir_sampling)
   end
   ```
 
-Additionally, the `num_measurements_seen` count SHOULD be reset at every
-collection cycle.
+The reservoir SHOULD be reset for every collection cycle. So, following the
+above example, the `num_measurements_seen` count would be reset every time the
+reservoir is collected.
+
+This Exemplar reservoir MAY take a configuration parameter for the size of the
+reservoir pool. 
+
+#### AlignedHistogramBucketExemplarReservoir
 
-_AlignedHistogramBucketExemplarReservoir_
 This Exemplar reservoir MUST take a configuration parameter that is the
 configuration of a Histogram.  This implementation MUST keep the last seen
 measurement that falls within a histogram bucket.  The reservoir will accept