Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

Commit

Permalink
zpages: fix Snapshot int->*int64 changes + tests on count
Browse files Browse the repository at this point in the history
The signature for countFormatter's arguments was changed
in PR #948 from int to int64 but unfortunately we forgot
to update the int fields from int to the new type.

Issue #948's recommendation was to use uint64 since that
affords us more time to overflow e.g. even at 25QPs
it'd take a couple of millenia to overflow with uint64.

This change completes the change of types of the int variables
but also adds tests that make use of the template functions
and a snapshot and then compare the output or fail. Previously the
tests complained but since they were HTTP tests not attached
to an output buffer that could be inspected, the bug innocently
crept away in standard output and the tests incorrectly passed as in:
* https://ci.appveyor.com/project/opencensusgoteam/opencensus-go/builds/19498802#L700
* https://travis-ci.org/census-instrumentation/opencensus-go/builds/441324783#L1183

This problem was reported by an OpenCensus-Service user in
* census-instrumentation/opencensus-service#123

Fixes #895
Fixes #951
  • Loading branch information
odeke-em committed Oct 20, 2018
1 parent 1eb9a13 commit ac292c6
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 14 deletions.
3 changes: 1 addition & 2 deletions zpages/formatter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,9 @@ import "testing"

func TestCountFormatter(t *testing.T) {
tests := []struct {
in int64
in uint64
want string
}{
{-1, " "},
{0, " "},
{1, "1"},
{1024, "1024"},
Expand Down
20 changes: 10 additions & 10 deletions zpages/rpcz.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,9 @@ type statSnapshot struct {
// TODO: compute hour/minute values from cumulative
Method string
Received bool
CountMinute int
CountHour int
CountTotal int
CountMinute uint64
CountHour uint64
CountTotal uint64
AvgLatencyMinute time.Duration
AvgLatencyHour time.Duration
AvgLatencyTotal time.Duration
Expand All @@ -185,9 +185,9 @@ type statSnapshot struct {
OutputRateMinute float64
OutputRateHour float64
OutputRateTotal float64
ErrorsMinute int
ErrorsHour int
ErrorsTotal int
ErrorsMinute uint64
ErrorsHour uint64
ErrorsTotal uint64
}

type methodKey struct {
Expand Down Expand Up @@ -267,7 +267,7 @@ func (s snapExporter) ExportView(vd *view.Data) {
}
for _, tag := range row.Tags {
if tag.Key == ocgrpc.KeyClientStatus && tag.Value != "OK" {
s.ErrorsTotal += int(count)
s.ErrorsTotal += uint64(count)
}
}

Expand All @@ -281,7 +281,7 @@ func (s snapExporter) ExportView(vd *view.Data) {
s.InputRateTotal = computeRate(0, sum)

case ocgrpc.ClientSentMessagesPerRPCView:
s.CountTotal = int(count)
s.CountTotal = uint64(count)
s.RPCRateTotal = computeRate(0, count)

case ocgrpc.ClientReceivedMessagesPerRPCView:
Expand All @@ -294,7 +294,7 @@ func (s snapExporter) ExportView(vd *view.Data) {
}
for _, tag := range row.Tags {
if tag.Key == ocgrpc.KeyServerStatus && tag.Value != "OK" {
s.ErrorsTotal += int(count)
s.ErrorsTotal += uint64(count)
}
}

Expand All @@ -305,7 +305,7 @@ func (s snapExporter) ExportView(vd *view.Data) {
s.OutputRateTotal = computeRate(0, sum)

case ocgrpc.ServerReceivedMessagesPerRPCView:
s.CountTotal = int(count)
s.CountTotal = uint64(count)
s.RPCRateTotal = computeRate(0, count)

case ocgrpc.ServerSentMessagesPerRPCView:
Expand Down
2 changes: 1 addition & 1 deletion zpages/rpcz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func TestRpcz(t *testing.T) {
t.Fatal("Expected method stats not recorded")
}

if got, want := snapshot.CountTotal, 1; got != want {
if got, want := snapshot.CountTotal, uint64(1); got != want {
t.Errorf("snapshot.CountTotal = %d; want %d", got, want)
}
}
2 changes: 1 addition & 1 deletion zpages/templates.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func parseTemplate(name string) *template.Template {
return template.Must(template.New(name).Funcs(templateFunctions).Parse(string(text)))
}

func countFormatter(num int64) string {
func countFormatter(num uint64) string {
if num <= 0 {
return " "
}
Expand Down
99 changes: 99 additions & 0 deletions zpages/templates_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
// Copyright 2018, OpenCensus Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package zpages

import (
"bytes"
"html/template"
"testing"
)

const tmplBody = `
<td><b>{{.Method}}</b></td>
<td></td>
<td align="right">{{.CountMinute|count}}</td>
<td align="right">{{.CountHour|count}}</td>
<td align="right">{{.CountTotal|count}}</td><td></td>
<td align="right">{{.AvgLatencyMinute|ms}}</td>
<td align="right">{{.AvgLatencyHour|ms}}</td>
<td align="right">{{.AvgLatencyTotal|ms}}</td><td></td>
<td align="right">{{.RPCRateMinute|rate}}</td>
<td align="right">{{.RPCRateHour|rate}}</td>
<td align="right">{{.RPCRateTotal|rate}}</td><td></td>
<td align="right">{{.InputRateMinute|datarate}}</td>
<td align="right">{{.InputRateHour|datarate}}</td>
<td align="right">{{.InputRateTotal|datarate}}</td><td></td>
<td align="right">{{.OutputRateMinute|datarate}}</td>
<td align="right">{{.OutputRateHour|datarate}}</td>
<td align="right">{{.OutputRateTotal|datarate}}</td><td></td>
<td align="right">{{.ErrorsMinute|count}}</td>
<td align="right">{{.ErrorsHour|count}}</td>
<td align="right">{{.ErrorsTotal|count}}</td><td></td>
`

var tmpl = template.Must(template.New("countTest").Funcs(templateFunctions).Parse(tmplBody))

func TestTemplateFuncs(t *testing.T) {
buf := new(bytes.Buffer)
sshot := &statSnapshot{
Method: "Foo",
CountMinute: 1e9,
CountHour: 5000,
CountTotal: 1e12,
AvgLatencyMinute: 10000,
AvgLatencyHour: 1000,
AvgLatencyTotal: 20000,
RPCRateMinute: 2000,
RPCRateHour: 5000,
RPCRateTotal: 75000,
InputRateMinute: 75000,
InputRateHour: 75000,
InputRateTotal: 75000,
OutputRateMinute: 75000,
OutputRateHour: 75000,
OutputRateTotal: 75000,
ErrorsMinute: 120000000,
ErrorsHour: 75000000,
ErrorsTotal: 7500000,
}
if err := tmpl.Execute(buf, sshot); err != nil {
t.Fatalf("Failed to execute template: %v", err)
}
want := `
<td><b>Foo</b></td>
<td></td>
<td align="right">1.000 G </td>
<td align="right">5000</td>
<td align="right">1.000 T </td><td></td>
<td align="right">0.010</td>
<td align="right">0.001</td>
<td align="right">0.020</td><td></td>
<td align="right">2000.000</td>
<td align="right">5000.000</td>
<td align="right">75000.000</td><td></td>
<td align="right">0.075</td>
<td align="right">0.075</td>
<td align="right">0.075</td><td></td>
<td align="right">0.075</td>
<td align="right">0.075</td>
<td align="right">0.075</td><td></td>
<td align="right">120.000 M </td>
<td align="right">75.000 M </td>
<td align="right">7.500 M </td><td></td>
`
if g, w := buf.String(), want; g != w {
t.Errorf("Output mismatch:\nGot:\n\t%s\nWant:\n\t%s", g, w)
}
}

0 comments on commit ac292c6

Please sign in to comment.