flamingo icon indicating copy to clipboard operation
flamingo copied to clipboard

Memory stats are not updated

Open mkrill opened this issue 2 years ago • 1 comments

Hello Flamingo team,

due to an issue in https://github.com/census-instrumentation/opencensus-go, the memory stats do not seem to be updated for opencensus. In line https://github.com/i-love-flamingo/flamingo/blob/fa5bd34bea32f0a38948f2c7875aed733c62e7d4/framework/opencensus/module.go#L155-L158 the RunMetricOptions are instantiated without setting UseDerivedCumulative. Consequently, in line https://github.com/census-instrumentation/opencensus-go/blob/052120675fac2ace91dc2c01e5f63c3e6ec62f04/plugin/runmetrics/producer.go#L133 only producer.deprecatedMemStats is instantiated and not producer.memStats. But, the latter seems to be required to read current memory metrics => see https://github.com/census-instrumentation/opencensus-go/blob/052120675fac2ace91dc2c01e5f63c3e6ec62f04/plugin/runmetrics/producer.go#L169 A possible workaround might be, to set UseDerivedCumulative in framework/opencensus/module.go

mkrill avatar May 02 '22 09:05 mkrill

Hi @mkrill, thanks for your report. This certainly looks like a bug in the opencensus implementation.

With the UseDerivedCumulative flag it would provide the same metrics overall, but changes the type of some existing metrics from gauge to counter. As indicated in the discussion for the PR in opencensus this will fail for some collector backends.

We will consider this bug when switching from opencensus to opentelemetry, which is planned for the near future. See details in #179.

As you suggested previously, you can enable the UseDerivedCumulative flag in one of your own modules. For that, ensure that your module Depends() on the opencensus.Module, so that your module is initialized after the flamingo opencensus module. After that you can override the runmetrics with your own options like this:

runmetrics.Enable(runmetrics.RunMetricOptions{
	EnableCPU:            true,
	EnableMemory:         true,
	UseDerivedCumulative: true,
})

karstenkoehler avatar May 06 '22 06:05 karstenkoehler