census-instrumentation/opencensus-go

zpages: fix the count formatter argument type and StatsSnapshot field types

odeke-em opened this issue · 0 comments

PR #948 changed the signature of countFormatter's argument from int to int64 however the fields of Snapshot that are passed into count during template execution are still int. This issue was reported by a user of the opencensus-service as per census-instrumentation/opencensus-service#123

zpages: executing template: template: rpcz:31:41: executing "rpcz" at <count>: wrong type for value; expected int64; got int

and in fact the test output also reported the runtime template error in
https://ci.appveyor.com/project/opencensusgoteam/opencensus-go/builds/19498802
screen shot 2018-10-20 at 3 53 53 pm
and
https://travis-ci.org/census-instrumentation/opencensus-go/builds/441324783#L1183
screen shot 2018-10-20 at 3 55 06 pm

The proper way to test those templates is by passing in a buffer and invoking execute then checking the output and errors such as

        if err := tmpl.Execute(buf, data); err != nil {
                t.Fatalf("Failed to execute template: %v", err)
        }

In the issue #895 I had recommended uint64, but we went with int64 which while still large, we have the ability to take advantage of the extra precision as I had described and also ensure graceful wrapping back to 0 on overflow in a couple of millenia

It would also be nice to make the zpages functions testable but also the formatters testable so that we can catch such innocent regressions early enough.