[Instrumentation.Runtime] Change API for GC Heap Size #495
Conversation
Codecov Report
@@ Coverage Diff @@
## main #495 +/- ##
=====================================
Coverage 0.00% 0.00%
=====================================
Files 167 167
Lines 5084 5098 +14
=====================================
- Misses 5084 5098 +14
|
…ernal API is available
…orkaround a known issue in .NET 6.
error SA1025: Code should not contain multiple whitespace characters in a row error SA1108: Block statements should not contain embedded comments
utpilla
added
the
comp:instrumentation.runtime
| () => | ||
| // GC.GetGCMemoryInfo().GenerationInfo[i].SizeAfterBytes is better but it has a bug in .NET 6. See context in https://github.com/open-telemetry/opentelemetry-dotnet-contrib/issues/496 | ||
| Func<int, ulong> getGenerationSize = null; | ||
| bool isCodeRunningOnBuggyRuntimeVersion = Environment.Version.Major == 6; |
There was a problem hiding this comment.
I understand that this code is guarded by #if NET6_0_OR_GREATER, I wonder if we should change == 6 to <= 6 (although both would give the exact same results) just to express the intention.
There was a problem hiding this comment.
My intention was to "check to avoid the buggy API on certain Runtime version" (only 6 and forward has this new API and only 6 has bug, thus the variable name), but not to "use GC.GetGenerationSize whenever possible".
In fact, if I go with the intention of the latter, I'd think whether we could add the metric for more Runtime versions whenever GC.GetGenerationSize is available. However, I don't think it's a good idea, because it would be a change of the spec and this API is not an official API.
|
Please add changelog entry |
Added. |
|
Looks good with this change:
|
Changes
Use internal method
GC.GetGenerationSizefor GC Heap Size..NET 6.0 has a bug in computing those generation sizes for the
GC.GetGCMemoryInfo()API and it was fixed in .NET 7.Note that the heap sizes are not retrieved in one transactional query with
GC.GetGCMemoryInfo(). It'd be better to change it back toGC.GetGCMemoryInfo().GenerationInfo[i].SizeAfterBytesfor .NET 7 when appropriate.For significant contributions please make sure you have completed the following items: