Metrics: Avoid defensive copies of MetricPoint struct

[CodeBlanch]

Description

We have this pattern in our metric exporters where we do...

foreach (ref readonly var metricPoint in metric.GetMetricPoints())
{
   long value = metricPoint.GetSumLong();
}

MetricPoint is a mutable struct:

opentelemetry-dotnet/src/OpenTelemetry/Metrics/MetricPoint.cs

Line 27 in a1e7167

public struct MetricPoint

When the compiler sees the GetSumLong invocation (for example) it doesn't know if that call is going to mutate the state of the local metricPoint so it will create a defensive copy. This can be avoided by declaring members which do not modify the state of the struct as readonly.

Here's a doc which goes over this in more detail: https://docs.microsoft.com/en-us/dotnet/csharp/write-safe-efficient-code#declare-readonly-members-for-mutable-structs

Benchmarks

(I used PrometheusSerializerBenchmarks but this will be universal wherever MetricPoint is used as a ref readonly.)

Before

Method	NumberOfSerializeCalls	Mean	Error	StdDev	Allocated
WriteMetric	1	5.324 us	0.1063 us	0.2781 us	-
WriteMetric	1000	5,499.300 us	109.2757 us	112.2181 us	6 B
WriteMetric	10000	54,968.431 us	799.3971 us	708.6447 us	172 B

After

Method	NumberOfSerializeCalls	Mean	Error	StdDev	Allocated
WriteMetric	1	4.427 us	0.0462 us	0.0432 us	-
WriteMetric	1000	4,814.730 us	67.4480 us	90.0411 us	11 B
WriteMetric	10000	48,092.389 us	780.6455 us	692.0219 us	70 B

TODOs

• Appropriate CHANGELOG.md updated for non-trivial changes

[codecov]

Codecov Report

Merging #3065 (4796d3d) into main (ea5d11d) will increase coverage by 0.01%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##             main    #3065      +/-   ##
==========================================
+ Coverage   84.70%   84.71%   +0.01%     
==========================================
  Files         259      259              
  Lines        9119     9121       +2     
==========================================
+ Hits         7724     7727       +3     
+ Misses       1395     1394       -1     

Impacted Files	Coverage Δ
...ementation/ExportClient/OtlpGrpcLogExportClient.cs	0.00% <0.00%> (ø)
...tation/ExportClient/OtlpGrpcMetricsExportClient.cs	35.71% <0.00%> (ø)
...entation/ExportClient/OtlpGrpcTraceExportClient.cs	50.00% <50.00%> (ø)
...mentation/ExportClient/BaseOtlpGrpcExportClient.cs	61.11% <75.00%> (-1.39%)	⬇️
...mentation/ExportClient/BaseOtlpHttpExportClient.cs	100.00% <100.00%> (ø)
...tation/ExportClient/OtlpHttpMetricsExportClient.cs	66.66% <100.00%> (ø)
...entation/ExportClient/OtlpHttpTraceExportClient.cs	90.47% <100.00%> (ø)
src/OpenTelemetry/Metrics/MetricPoint.cs	85.21% <100.00%> (ø)
...ter.ZPages/Implementation/ZPagesActivityTracker.cs	100.00% <0.00%> (+2.85%)	⬆️
...ZPages/Implementation/ZPagesExporterEventSource.cs	62.50% <0.00%> (+6.25%)	⬆️

[utpilla]

Why are the allocation numbers changing randomly? Is this change supposed to affect allocation as well? If yes, why does it go up for WriteMetric with 1000 calls but go down for WriteMetric with 10000 calls?

[CodeBlanch]

@utpilla That is a good question I do not have an answer for. Should be no change at all to the allocations for this work. Even more then that, there should no allocations at all ever shown with this benchmark! Prometheus reuses a buffer so that it doesn't need allocations. Seems to be noise coming from somewhere. Something I would like to get to the bottom of, but prefer to not block this PR for that.