Recycling things (#75)

* - Restoring the ability to enable recycling

- Defaults to disabling recycling on .NET 6.0
- Updated README to include details about recycling
- Allowing user to specify a `null` recycling time to disable recycling

* Fixing typos

* Retuning README

* More tweaks

Co-authored-by: James Luck <[email protected]>

Author: djluck

README.md

@@ -12,7 +12,7 @@ These metrics are essential for understanding the performance of any non-trivial
 
 ## Using this package
 ### Requirements
-- .NET core 3.1 (runtime version 3.1.11+ is recommended)/ .NET 5.0
+- .NET 5.0+ recommended, .NET core 3.1+ is supported 
 - The [prometheus-net](https://github.com/prometheus-net/prometheus-net) package
 
 ### Install it
@@ -61,7 +61,27 @@ The harder you work the .NET core runtime, the more events it generates. Event g
 - **GC stats with `CaptureLevel.Verbose`**: every 100KB of allocations, an event is emitted. If you are consistently allocating memory at a rate > 1GB/sec, you might like to disable GC stats.
 - **Exception stats with `CaptureLevel.Errors`**: for every exception throw, an event is generated.
 
-There is also a [performance issue present in .NET core 3.1](https://github.com/dotnet/runtime/issues/43985#issuecomment-800629516) that will see CPU consumption grow over time when long-running trace sessions are used. 
+#### Recycling collectors
+There have been long-running [performance issues since .NET core 3.1](https://github.com/dotnet/runtime/issues/43985#issuecomment-800629516) that could see CPU consumption grow over time when long-running trace sessions are used. 
+While many of the performance issues have been addressed now in .NET 6.0, a workaround was identified: stopping and starting (AKA recycling) collectors periodically helped reduce CPU consumption:
+```
+IDisposable collector = DotNetRuntimeStatsBuilder.Default()
+	// Recycles all collectors once every day
+	.RecycleCollectorsEvery(TimeSpan.FromDays(1))
+	.StartCollecting()
+```
+
+While this [has been observed to reduce CPU consumption](https://github.com/djluck/prometheus-net.DotNetRuntime/issues/6#issuecomment-784540220) this technique has been identified as a [possible culprit that can lead
+to application instability](https://github.com/djluck/prometheus-net.DotNetRuntime/issues/72). 
+
+Behaviour on different runtime versions is:
+- .NET core 3.1: recycling verified to cause massive instability, cannot enable recycling.
+- .NET 5.0: recycling verified to be beneficial, recycling every day enabled by default.
+- .NET 6.0+: recycling verified to be less necesarry due to long-standing issues being addressed although [some users report recycling to be beneficial](https://github.com/djluck/prometheus-net.DotNetRuntime/pull/73#issuecomment-1308558226), 
+  disabled by default but recycling can be enabled.
+  
+> TLDR: If you observe increasing CPU over time, try enabling recycling. If you see unexpected crashes after using this application, try disabling recycling.
+
 
 ## Examples
 An example `docker-compose` stack is available in the [`examples/`](examples/) folder. Start it with:

examples/AspNetCoreExample/Startup.cs

@@ -90,7 +90,7 @@ public static IDisposable CreateCollector()
             }
 
             builder 
-#if NET5_0
+#if NET5_0_OR_GREATER
                 .RecycleCollectorsEvery(_options.RecycleEvery)
 #endif
                 .WithErrorHandler(ex => _logger.LogError(ex, "Unexpected exception occurred in prometheus-net.DotNetRuntime"));

src/prometheus-net.DotNetRuntime.Tests/EventListening/EventParserTypes.cs

@@ -68,7 +68,7 @@ public void When_Calling_GetEventInterfacesForCurrentRuntime_On_Net31_Then_Retur
         }
 #endif
 
-#if NET5_0
+#if NET5_0_OR_GREATER
 
         [Test]
         public void When_Calling_GetEventInterfacesForCurrentRuntime_On_Net50_Then_Returns_Interfaces_For_Net50_Runtime_And_Below()

src/prometheus-net.DotNetRuntime.Tests/IntegrationTests/JitCompilerTests.cs

@@ -102,7 +102,7 @@ protected override DotNetRuntimeStatsBuilder.Builder ConfigureBuilder(DotNetRunt
             return toConfigure.WithJitStats(CaptureLevel.Counters, SampleEvery.OneEvent);
         }
 
-#if NET5_0
+#if NET5_0_OR_GREATER
 
         [Test]
         public void When_Running_On_NET50_Then_Counts_Of_Methods_Are_Recorded()

src/prometheus-net.DotNetRuntime/DotNetRuntimeStatsBuilder.cs

@@ -226,7 +226,7 @@ public Builder WithErrorHandler(Action<Exception> handler)
                 return this;
             }
 
-#if NET5_0
+#if NET5_0_OR_GREATER
             /// <summary>
             /// Specifies a custom interval to recycle collectors. Defaults to 1 day.
             /// </summary>
@@ -235,9 +235,9 @@ public Builder WithErrorHandler(Action<Exception> handler)
             /// Recycling the event collectors is a workaround, preventing CPU exhaustion (see https://github.com/dotnet/runtime/issues/43985#issuecomment-793187345 for more info).
             /// During a recycle, existing metrics will not disappear/ reset but will not be updated for a short period (should be at most a couple of seconds). 
             /// </remarks>
-            /// <param name="interval"></param>
+            /// <param name="interval">The interval to recycle at. Set to null to disable recycling.</param>
             /// <returns></returns>
-            public Builder RecycleCollectorsEvery(TimeSpan interval)
+            public Builder RecycleCollectorsEvery(TimeSpan? interval)
             {
 #if DEBUG
                 // In debug mode, allow more aggressive recycling times to verify recycling works correctly

src/prometheus-net.DotNetRuntime/DotNetRuntimeStatsCollector.cs

@@ -202,6 +202,10 @@ public class Options
 
             public TimeSpan? RecycleListenersEvery { get; set; } =
 #if NET5_0
+                // only default to enabled for .NET 5. .NET 6 had/ has issues where recycling collectors could lead to 
+                // problems, see https://github.com/dotnet/runtime/pull/76431
+                // HOWEVER, people have mentioned that recycling is still required under .NET 6.0: https://github.com/dotnet/runtime/pull/76431
+                // As a compromise, we won't enable it by default but will allow people to opt-in
                 TimeSpan.FromDays(1);
 #else
                 null;