Conversation
|
@v1rtl please make sure to follow AGENTS.md |
|
sorry for that, I'll ammend my commits to follow it, I opened the draft PR just to test things but I'll clean it up before making it non-draft |
Adds the MemStats fields upstream Go defines that TinyGo did not have: Lookups, StackInuse, StackSys, MSpanInuse, MSpanSys, MCacheInuse, MCacheSys, BuckHashSys, OtherSys, NextGC and GCCPUFraction. TinyGo has no runtime structures for these to describe, so they always report 0 rather than carry made up values. They exist so that packages reading MemStats can be compiled. github.com/prometheus/client_golang fails to type check against TinyGo because these fields are missing. See tinygo-org#3879.
Adds runtime.StackRecord and a runtime.ThreadCreateProfile stub that always reports an empty profile, matching the existing SetBlockProfileRate and SetMutexProfileFraction stubs. github.com/prometheus/client_golang calls ThreadCreateProfile to count OS threads. See tinygo-org#3879.
539ba65 to
ef4d9e8
Compare
|
should be clean now, marking as ready for review |
|
Thanks for working on this @v1rtl. I ran it on my local machine and took a good look. The following is edited from an automated review:
What do you think? |
Fixes the compile failure in #3879.
github.com/prometheus/client_golang does not build under TinyGo because of missing
runtimeAPI surface. It produces 24 type check errors, all of them undefined fields or functions. This adds that surface.The failure is at package level in
prometheus, so it cannot be avoided by skipping the Go collector. Any import of the package fails.MemStats
Adds the 11 fields upstream defines that TinyGo lacked:
Lookups,StackInuse,StackSys,MSpanInuse,MSpanSys,MCacheInuse,MCacheSys,BuckHashSys,OtherSys,NextGC,GCCPUFraction.TinyGo has no runtime structures for these to describe, so they always report 0 rather than carry made up values.
Lookupsis unused upstream too. Existing fields are untouched and still populated as before.Fields follow upstream's section grouping. This adds a
Garbage collector statisticsheading and movesNumGCunder it, since it was under off heap statistics.ThreadCreateProfile
A stub reporting an empty profile, plus the
StackRecordtype it needs. Matches the existingSetBlockProfileRateandSetMutexProfileFractionstubs indebug.go. Prometheus callsThreadCreateProfile(nil)to count OS threads.Open questions
Left as a draft because three things want a maintainer opinion.
NextGCfor the blocks collector andStackInuseif goroutine stacks were accounted separately are both possible with more work. I kept the whole set at 0 so the change stays small.ThreadCreateProfileshould return(0, true)or(1, true). I chose(0, true)as a consistent empty profile, matchingNumCgoCallreturning 0. Returning 1 is arguably more truthful since a thread does exist, but then the record contents would be wrong. This decides whether Prometheus reportsgo_threadsas 0 or 1.ReadMemStatsshould set the new fields explicitly. It does not zero*mon entry, so a reusedMemStatskeeps stale values in them. This predates the PR but the new fields widen it.Testing
Compiled the #3879 reproducer against this branch. All 24 errors are gone and it runs.
ReadMemStatsstill reports real values for existing fields, new ones read 0.Reproducer: