[Exporter.Geneva] Resolve AOT warnings in OpenTelemetry.Exporter.Geneva

[eerhardt]

Changes

Make OpenTelemetry.Exporter.Geneva compatible with native AOT.

• Also use Regex source generator for faster startup, less memory usage, and the ability to trim the Regex engine.

For significant contributions please make sure you have completed the following items:

• Appropriate CHANGELOG.md updated for non-trivial changes
• Design discussion issue #
• Changes in public API reviewed

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.28%. Comparing base (71655ce) to head (77ed99a).
Report is 209 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1666      +/-   ##
==========================================
+ Coverage   73.91%   74.28%   +0.36%     
==========================================
  Files         267      264       -3     
  Lines        9615     9977     +362     
==========================================
+ Hits         7107     7411     +304     
- Misses       2508     2566      +58     

Flag	Coverage Δ
unittests-Exporter.Geneva	61.38% <100.00%> (?)
unittests-Exporter.OneCollector	89.72% <ø> (?)
unittests-Extensions	82.75% <ø> (?)
unittests-Instrumentation.AspNet	76.83% <ø> (?)
unittests-Instrumentation.EventCounters	76.36% <ø> (?)
unittests-Instrumentation.Owin	83.43% <ø> (?)
unittests-Instrumentation.Process	100.00% <ø> (?)
unittests-Instrumentation.Runtime	100.00% <ø> (?)
unittests-Instrumentation.StackExchangeRedis	71.00% <ø> (?)
unittests-Instrumentation.Wcf	78.47% <ø> (?)
unittests-PersistentStorage	65.78% <ø> (?)
unittests-ResourceDetectors.Azure	81.53% <ø> (?)
unittests-ResourceDetectors.Host	54.11% <ø> (?)
unittests-ResourceDetectors.Process	100.00% <ø> (?)
unittests-ResourceDetectors.ProcessRuntime	76.08% <ø> (?)
unittests-Solution	79.85% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...ry.Exporter.Geneva/Metrics/GenevaMetricExporter.cs	76.47% <100.00%> (+9.80%)	⬆️
...Geneva/Metrics/Transport/MetricEtwDataTransport.cs	53.33% <ø> (-20.01%)	⬇️
...neva/MsgPackExporter/Transport/EtwDataTransport.cs	85.00% <ø> (ø)

... and 219 files with indirect coverage changes

[utpilla]

@eerhardt GenevaExporter also uses Expression.Lambda from System.Linq.Expressions. Did we not have a Trim/AOT issue when using that?

opentelemetry-dotnet-contrib/src/OpenTelemetry.Exporter.Geneva/Internal/ReentrantExportProcessor.cs

Lines 18 to 25 in ab7bdab

	static ReentrantExportProcessor()
	{
	var flags = BindingFlags.Instance | BindingFlags.NonPublic;
	var ctor = typeof(Batch<T>).GetConstructor(flags, null, new Type[] { typeof(T) }, null);
	var value = Expression.Parameter(typeof(T), null);
	var lambda = Expression.Lambda<Func<T, Batch<T>>>(Expression.New(ctor, value), value);
	CreateBatch = lambda.Compile();
	}

[eerhardt]

Did we not have a Trim/AOT issue when using that?

There wasn't an AOT warning for it, which means that it will work correctly when AOT'd.

Linq Expressions do work with trimming and AOT, as long as the tooling can statically tell what methods/properties/etc you are referencing. In this case, it can since the types are all static - Batch<T> and typeof(T).

The drawback to using Linq Expressions on native AOT'd apps is that the "compiled" expression is just compiled into an expression tree and the tree is interpreted every time it is executed. So it is a slower than it would be on a normal CoreCLR process where it can be compiled to IL and JIT'd to machine code.

If this is a problem, I would recommend making the constructor of Batch<T>(T item) public so this code can call it. Or the comment says this class should be gotten rid of once OpenTelemetry .NET officially supports not having synchronization. If neither of them can happen, the code can be changed to use pure Reflection when on native AOT, which performs better than interpreted Linq Expressions.

[utpilla]

If this is a problem, I would recommend making the constructor of Batch<T>(T item) public so this code can call it. Or the comment says this class should be gotten rid of once OpenTelemetry .NET officially supports not having synchronization. If neither of them can happen, the code can be changed to use pure Reflection when on native AOT, which performs better than interpreted Linq Expressions.

Thanks @eerhardt for the explanation! We would have to switch to using pure Reflection on Native AOT but that can be address later. For now, this PR looks good.

[eerhardt]

We would have to switch to using pure Reflection on Native AOT

"have to" is too strong of a phrase. The code as is will work on Native AOT.

My recommendation would be to not use private/internal Reflection at all. If this API is supposed to be called outside of the assembly, it should be public. Or if this code is supposed to be removed, we should do the work to remove it.

[utpilla]

"have to" is too strong of a phrase. The code as is will work on Native AOT.

Ah yes! Pardon my English 😀. What I actually wanted to say was we could "consider" switching to pure reflection. We don't have to unless there is a valid complaint/ user ask for it.

My recommendation would be to not use private/internal Reflection at all. If this API is supposed to be called outside of the assembly, it should be public. Or if this code is supposed to be removed, we should do the work to remove it.

Unfortunately, both of those alternatives are probably going to be hard (time-taking) as the private API is coming from the main package which has stricter requirements for making things public. Maybe it's time to revisit this issue in the main repo.

[eerhardt]

One other reason to use pure Reflection (besides potential throughput gains) would be for size. If this is the only code in the app pulling in Linq Expressions, there is potential size savings with trimming Linq Expressions from this code. I can look to see if that's the case for the case I care about.