Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory stats are not updated #228

Open
mkrill opened this issue May 2, 2022 · 1 comment
Open

Memory stats are not updated #228

mkrill opened this issue May 2, 2022 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@mkrill
Copy link
Contributor

mkrill commented May 2, 2022

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

if err := runmetrics.Enable(runmetrics.RunMetricOptions{
EnableCPU: true,
EnableMemory: true,
}); err != nil {
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

@bastianccm bastianccm added bug Something isn't working good first issue Good for newcomers and removed good first issue Good for newcomers labels May 5, 2022
@karstenkoehler
Copy link
Contributor

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,
})

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants