Add comments and fix few todos in Metrics (#2939)

src/OpenTelemetry/Metrics/AggregatorStore.cs

@@ -25,6 +25,7 @@ namespace OpenTelemetry.Metrics
 {
     internal sealed class AggregatorStore
     {
+        private static readonly string MetricPointCapHitFixMessage = "Modify instrumentation to reduce the number of unique key/value pair combinations. Or use Views to drop unwanted tags. Or use MeterProviderBuilder.SetMaxMetricPointsPerMetricStream to set higher limit.";
         private readonly object lockZeroTags = new object();
         private readonly HashSet<string> tagKeysInteresting;
         private readonly int tagsKeysInterestingCount;
@@ -309,7 +310,7 @@ private void UpdateLong(long value, ReadOnlySpan<KeyValuePair<string, object>> t
                 {
                     if (Interlocked.CompareExchange(ref this.metricCapHitMessageLogged, 1, 0) == 0)
                     {
-                        OpenTelemetrySdkEventSource.Log.MeasurementDropped(this.name, this.metricPointCapHitMessage, "Modify instrumentation to reduce the number of unique key/value pair combinations. Or use MeterProviderBuilder.SetMaxMetricPointsPerMetricStream to set higher limit.");
+                        OpenTelemetrySdkEventSource.Log.MeasurementDropped(this.name, this.metricPointCapHitMessage, MetricPointCapHitFixMessage);
                     }
 
                     return;
@@ -332,7 +333,7 @@ private void UpdateLongCustomTags(long value, ReadOnlySpan<KeyValuePair<string,
                 {
                     if (Interlocked.CompareExchange(ref this.metricCapHitMessageLogged, 1, 0) == 0)
                     {
-                        OpenTelemetrySdkEventSource.Log.MeasurementDropped(this.name, this.metricPointCapHitMessage, "Modify instrumentation to reduce the number of unique key/value pair combinations. Or use MeterProviderBuilder.SetMaxMetricPointsPerMetricStream to set higher limit.");
+                        OpenTelemetrySdkEventSource.Log.MeasurementDropped(this.name, this.metricPointCapHitMessage, MetricPointCapHitFixMessage);
                     }
 
                     return;
@@ -355,7 +356,7 @@ private void UpdateDouble(double value, ReadOnlySpan<KeyValuePair<string, object
                 {
                     if (Interlocked.CompareExchange(ref this.metricCapHitMessageLogged, 1, 0) == 0)
                     {
-                        OpenTelemetrySdkEventSource.Log.MeasurementDropped(this.name, this.metricPointCapHitMessage, "Modify instrumentation to reduce the number of unique key/value pair combinations. Or use MeterProviderBuilder.SetMaxMetricPointsPerMetricStream to set higher limit.");
+                        OpenTelemetrySdkEventSource.Log.MeasurementDropped(this.name, this.metricPointCapHitMessage, MetricPointCapHitFixMessage);
                     }
 
                     return;
@@ -378,7 +379,7 @@ private void UpdateDoubleCustomTags(double value, ReadOnlySpan<KeyValuePair<stri
                 {
                     if (Interlocked.CompareExchange(ref this.metricCapHitMessageLogged, 1, 0) == 0)
                     {
-                        OpenTelemetrySdkEventSource.Log.MeasurementDropped(this.name, this.metricPointCapHitMessage, "Modify instrumentation to reduce the number of unique key/value pair combinations. Or use MeterProviderBuilder.SetMaxMetricPointsPerMetricStream to set higher limit.");
+                        OpenTelemetrySdkEventSource.Log.MeasurementDropped(this.name, this.metricPointCapHitMessage, MetricPointCapHitFixMessage);
                     }
 
                     return;

src/OpenTelemetry/Metrics/BaseExportingMetricReader.cs

@@ -21,12 +21,20 @@
 
 namespace OpenTelemetry.Metrics
 {
+    /// <summary>
+    /// MetricReader implementation which exports metrics to the configured
+    /// MetricExporter upon <see cref="MetricReader.Collect(int)"/>.
+    /// </summary>
     public class BaseExportingMetricReader : MetricReader
     {
         protected readonly BaseExporter<Metric> exporter;
         private readonly ExportModes supportedExportModes = ExportModes.Push | ExportModes.Pull;
         private bool disposed;
 
+        /// <summary>
+        /// Initializes a new instance of the <see cref="BaseExportingMetricReader"/> class.
+        /// </summary>
+        /// <param name="exporter">Exporter instance to export Metrics to.</param>
         public BaseExportingMetricReader(BaseExporter<Metric> exporter)
         {
             Guard.ThrowIfNull(exporter);

src/OpenTelemetry/Metrics/CompositeMetricReader.cs

@@ -141,10 +141,9 @@ protected override void Dispose(bool disposing)
                         {
                             cur.Value?.Dispose();
                         }
-                        catch (Exception)
+                        catch (Exception ex)
                         {
-                            // TODO: which event source do we use?
-                            // OpenTelemetrySdkEventSource.Log.SpanProcessorException(nameof(this.Dispose), ex);
+                            OpenTelemetrySdkEventSource.Log.MetricReaderException(nameof(this.Dispose), ex);
                         }
                     }
                 }

src/OpenTelemetry/Metrics/ExplicitBucketHistogramConfiguration.cs

@@ -24,6 +24,8 @@ public class ExplicitBucketHistogramConfiguration : MetricStreamConfiguration
         /// </summary>
         /// <remarks>
         /// The array must be in ascending order with distinct values.
+        /// An empty array would result in no histogram buckets being calculated.
+        /// A null value would result in default bucket boundaries being used.
         /// </remarks>
         public double[] Boundaries { get; set; }
     }

src/OpenTelemetry/Metrics/Metric.cs

@@ -20,6 +20,9 @@
 
 namespace OpenTelemetry.Metrics
 {
+    /// <summary>
+    /// Represents a Metric stream which can contain multiple MetricPoints.
+    /// </summary>
     public sealed class Metric
     {
         internal static readonly double[] DefaultHistogramBounds = new double[] { 0, 5, 10, 25, 50, 75, 100, 250, 500, 1000 };

src/OpenTelemetry/Metrics/MetricPoint.cs

@@ -22,7 +22,7 @@
 namespace OpenTelemetry.Metrics
 {
     /// <summary>
-    /// Stores details about a metric data point.
+    /// Represents a metric data point.
     /// </summary>
     public struct MetricPoint
     {

src/OpenTelemetry/Metrics/MetricReaderType.cs

@@ -28,7 +28,7 @@ public enum MetricReaderType
         Manual,
 
         /// <summary>
-        /// Use the <see cref="PeriodicExportingMetricReader" />.
+        /// Uses the <see cref="PeriodicExportingMetricReader" />.
         /// <c>MetricReader.Collect()</c> will be invoked on a defined interval.
         /// </summary>
         Periodic,

src/OpenTelemetry/Metrics/MetricStreamConfiguration.cs

@@ -28,6 +28,9 @@ internal enum Aggregation
 #pragma warning restore SA1602 // Enumeration items should be documented
     }
 
+    /// <summary>
+    /// Holds the configuration for a MetricStream.
+    /// </summary>
     public class MetricStreamConfiguration
     {
         public static readonly MetricStreamConfiguration Drop = new DropConfiguration();
@@ -40,7 +43,9 @@ public class MetricStreamConfiguration
 
         internal virtual Aggregation Aggregation { get; set; }
 
-        // TODO: MetricPoints caps can be configured here
+        // TODO: MetricPoints caps can be configured here on
+        // a per stream basis, when we add such a capability
+        // in the future.
 
         private sealed class DropConfiguration : MetricStreamConfiguration
         {

src/OpenTelemetry/Metrics/PeriodicExportingMetricReader.cs

@@ -21,6 +21,11 @@
 
 namespace OpenTelemetry.Metrics
 {
+    /// <summary>
+    /// MetricReader implementation which collects metrics based on
+    /// a user-configurable time interval and passes the metrics to
+    /// the configured MetricExporter.
+    /// </summary>
     public class PeriodicExportingMetricReader : BaseExportingMetricReader
     {
         internal const int DefaultExportIntervalMilliseconds = 60000;
@@ -33,6 +38,12 @@ public class PeriodicExportingMetricReader : BaseExportingMetricReader
         private readonly ManualResetEvent shutdownTrigger = new ManualResetEvent(false);
         private bool disposed;
 
+        /// <summary>
+        /// Initializes a new instance of the <see cref="PeriodicExportingMetricReader"/> class.
+        /// </summary>
+        /// <param name="exporter">Exporter instance to export Metrics to.</param>
+        /// <param name="exportIntervalMilliseconds">The interval in milliseconds between two consecutive exports. The default value is 60000.</param>
+        /// <param name="exportTimeoutMilliseconds">How long the export can run before it is cancelled. The default value is 30000.</param>
         public PeriodicExportingMetricReader(
             BaseExporter<Metric> exporter,
             int exportIntervalMilliseconds = DefaultExportIntervalMilliseconds,

test/Benchmarks/Metrics/MetricsBenchmarks.cs

@@ -20,7 +20,6 @@
 using System.Diagnostics.Metrics;
 using BenchmarkDotNet.Attributes;
 using OpenTelemetry;
-using OpenTelemetry.Exporter;
 using OpenTelemetry.Metrics;
 using OpenTelemetry.Tests;
 

test/Benchmarks/Metrics/MetricsViewBenchmarks.cs

@@ -20,7 +20,6 @@
 using System.Threading;
 using BenchmarkDotNet.Attributes;
 using OpenTelemetry;
-using OpenTelemetry.Exporter;
 using OpenTelemetry.Metrics;
 using OpenTelemetry.Tests;
 

test/OpenTelemetry.Tests/Metrics/MetricAPITest.cs

@@ -19,7 +19,6 @@
 using System.Diagnostics;
 using System.Diagnostics.Metrics;
 using System.Threading;
-using OpenTelemetry.Exporter;
 using OpenTelemetry.Tests;
 using Xunit;
 using Xunit.Abstractions;