godev/cmd/worker: skip empty partition in charts
When there is no data to draw, do not include in the charts data.
Previously, this resulted in adding null to chartdata > Programs >
Charts and caused the frontend javascript to throw an exception.
Update charts.ts to handle chart json files that contain null charts.
While we are here,
* remove the exception for cmd/go (which does not have version).
After CL 585198 go toolchain programs will produce counter files
with the program field filled with the go version.
* fix the program version handling since now toolchains will have
version strings that are not semver.
Fixes golang/go#67397
Change-Id: I0c037afbbfd931b70ada310dba9d67b49521ad94
Reviewed-on: https://go-review.googlesource.com/c/telemetry/+/585479
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/godev/cmd/worker/main.go b/godev/cmd/worker/main.go
index 81cb70b..31951a3 100644
--- a/godev/cmd/worker/main.go
+++ b/godev/cmd/worker/main.go
@@ -55,7 +55,7 @@
mux := http.NewServeMux()
mux.Handle("/", cserv)
- mux.Handle("/merge/", handleMerge(ucfg, buckets))
+ mux.Handle("/merge/", handleMerge(buckets))
mux.Handle("/chart/", handleChart(ucfg, buckets))
mux.Handle("/queue-tasks/", handleTasks(cfg))
@@ -135,7 +135,7 @@
}
// TODO: monitor duration and processed data volume.
-func handleMerge(cfg *tconfig.Config, s *storage.API) content.HandlerFunc {
+func handleMerge(s *storage.API) content.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) error {
ctx := r.Context()
date := r.URL.Query().Get("date")
@@ -271,22 +271,23 @@
for _, p := range cfg.Programs {
prog := &program{ID: "charts:" + p.Name, Name: p.Name}
result.Programs = append(result.Programs, prog)
- if p.Name != "cmd/go" {
- prog.Charts = append(prog.Charts,
- partition(d, p.Name, "Version", p.Versions, xs),
- )
+ var charts []*chart
+ if !telemetry.IsToolchainProgram(p.Name) {
+ charts = append(charts, partition(d, p.Name, "Version", p.Versions))
}
- prog.Charts = append(prog.Charts,
- partition(d, p.Name, "GOOS", cfg.GOOS, xs),
- partition(d, p.Name, "GOARCH", cfg.GOARCH, xs),
- partition(d, p.Name, "GoVersion", cfg.GoVersion, xs),
- )
+ charts = append(charts,
+ partition(d, p.Name, "GOOS", cfg.GOOS),
+ partition(d, p.Name, "GOARCH", cfg.GOARCH),
+ partition(d, p.Name, "GoVersion", cfg.GoVersion))
for _, c := range p.Counters {
// TODO: add support for histogram counters by getting the counter type
// from the chart config.
- prog.Charts = append(prog.Charts,
- partition(d, p.Name, c.Name, tconfig.Expand(c.Name), xs),
- )
+ charts = append(charts, partition(d, p.Name, c.Name, tconfig.Expand(c.Name)))
+ }
+ for _, p := range charts {
+ if p != nil {
+ prog.Charts = append(prog.Charts, p)
+ }
}
}
return result
@@ -300,7 +301,7 @@
}
for wk := range dat {
for _, c := range counters {
- prefix, _ := parts(c)
+ prefix, _ := splitCounterName(c)
pk := programKey{program}
gk := graphKey{prefix}
ck := counterKey{c}
@@ -311,7 +312,7 @@
xk := xKey{x}
v := dat[wk][pk][gk][ck][xk]
total := dat[wk][pk][gk][counterKey{gk.prefix}][xk]
- _, bucket := parts(c)
+ _, bucket := splitCounterName(c)
if total == 0 {
d := &datum{
Week: wk.date,
@@ -332,14 +333,16 @@
return count
}
-func partition(dat data, program, name string, counters []string, xs []float64) *chart {
+// partition builds a chart for the program and the counter. It can return nil
+// if there is no data for the counter in dat.
+func partition(dat data, program, counterPrefix string, counters []string) *chart {
count := &chart{
- ID: "charts:" + program + ":" + name,
- Name: name,
+ ID: "charts:" + program + ":" + counterPrefix,
+ Name: counterPrefix,
Type: "partition",
}
pk := programKey{program}
- prefix, _ := parts(name)
+ prefix, _ := splitCounterName(counterPrefix)
gk := graphKey{prefix}
for wk := range dat {
// TODO: when should this be number of reports?
@@ -352,7 +355,8 @@
// major minor versions we've already added to the dataset.
seen := make(map[string]bool)
for _, b := range counters {
- counter := fixCounter(name, b)
+ // TODO(hyangah): let caller normalize names in counters.
+ counter := normalizeCounterName(counterPrefix, b)
if seen[counter] {
continue
}
@@ -360,7 +364,7 @@
ck := counterKey{counter}
// number of reports where count prefix:bucket > 0
n := len(dat[wk][pk][gk][ck])
- _, bucket := parts(counter)
+ _, bucket := splitCounterName(counter)
d := &datum{
Week: wk.date,
Key: bucket,
@@ -394,11 +398,13 @@
type data map[weekKey]map[programKey]map[graphKey]map[counterKey]map[xKey]int64
+// Names of special counters.
+// Unlike other counters, these are constructed from the metadata in the report.
const (
- version = "Version"
- goos = "GOOS"
- goarch = "GOARCH"
- goVersion = "GoVersion"
+ versionCounter = "Version"
+ goosCounter = "GOOS"
+ goarchCounter = "GOARCH"
+ goversionCounter = "GoVersion"
)
// nest groups the report data by week, program, prefix, counter, and x value
@@ -407,12 +413,12 @@
result := make(data)
for _, r := range reports {
for _, p := range r.Programs {
- writeCount(result, r.Week, p.Program, version, p.Version, r.X, 1)
- writeCount(result, r.Week, p.Program, goos, p.GOOS, r.X, 1)
- writeCount(result, r.Week, p.Program, goarch, p.GOARCH, r.X, 1)
- writeCount(result, r.Week, p.Program, goVersion, p.GoVersion, r.X, 1)
+ writeCount(result, r.Week, p.Program, versionCounter, p.Version, r.X, 1)
+ writeCount(result, r.Week, p.Program, goosCounter, p.GOOS, r.X, 1)
+ writeCount(result, r.Week, p.Program, goarchCounter, p.GOARCH, r.X, 1)
+ writeCount(result, r.Week, p.Program, goversionCounter, p.GoVersion, r.X, 1)
for c, value := range p.Counters {
- name, _ := parts(c)
+ name, _ := splitCounterName(c)
writeCount(result, r.Week, p.Program, name, c, r.X, value)
}
}
@@ -436,7 +442,8 @@
if _, ok := result[wk][pk][gk]; !ok {
result[wk][pk][gk] = make(map[counterKey]map[xKey]int64)
}
- counter = fixCounter(prefix, counter)
+ // TODO(hyangah): let caller pass the normalized counter name.
+ counter = normalizeCounterName(prefix, counter)
ck := counterKey{counter}
if _, ok := result[wk][pk][gk][ck]; !ok {
result[wk][pk][gk][ck] = make(map[xKey]int64)
@@ -454,29 +461,38 @@
}
}
-// fixCounter groups counters for version and go version by the major minor
-// version. It also adds a prefix to counters derived from report metadata.
-func fixCounter(prefix, counter string) string {
+// normalizeCounterName normalizes the counter name.
+// More specifically, program version, goos, goarch, and goVersion
+// are not a real counter, but information from the metadata in the report.
+// This function constructs pseudo counter names to handle them
+// like other normal counters in aggregation and chart drawing.
+// To limit the cardinality of version and goVersion, this function
+// uses only major and minor version numbers in the pseudo-counter names.
+// If the counter is a normal counter name, it is returned as is.
+func normalizeCounterName(prefix, counter string) string {
switch prefix {
- case version:
+ case versionCounter:
if counter == "devel" {
return prefix + ":" + counter
}
+ if strings.HasPrefix(counter, "go") {
+ return prefix + ":" + goMajorMinor(counter)
+ }
return prefix + ":" + semver.MajorMinor(counter)
- case goos:
+ case goosCounter:
return prefix + ":" + counter
- case goarch:
+ case goarchCounter:
return prefix + ":" + counter
- case goVersion:
+ case goversionCounter:
return prefix + ":" + goMajorMinor(counter)
}
return counter
}
-// parts gets splits the prefix and bucket parts of a counter name
+// splitCounterName gets splits the prefix and bucket splitCounterName of a counter name
// or a bucket name. For an input with no bucket part prefix and bucket
// are the same.
-func parts(name string) (prefix, bucket string) {
+func splitCounterName(name string) (prefix, bucket string) {
prefix, bucket, found := strings.Cut(name, ":")
if !found {
bucket = prefix
@@ -486,6 +502,8 @@
// goMajorMinor gets the go<Maj>,<Min> version for a given go version.
// For example, go1.20.1 -> go1.20.
+// TODO(hyangah): replace with go/version.Lang (available from go1.22)
+// after our builders stop running go1.21.
func goMajorMinor(v string) string {
v = v[2:]
maj, x, ok := cutInt(v)
@@ -512,7 +530,6 @@
}
return x[:i], x[i:], true
}
-
func fsys(fromOS bool) fs.FS {
var f fs.FS = contentfs.FS
if fromOS {
diff --git a/godev/cmd/worker/main_test.go b/godev/cmd/worker/main_test.go
index 0b64a51..352b7f2 100644
--- a/godev/cmd/worker/main_test.go
+++ b/godev/cmd/worker/main_test.go
@@ -8,6 +8,8 @@
"reflect"
"testing"
+ "github.com/google/go-cmp/cmp"
+ "golang.org/x/telemetry/internal/config"
"golang.org/x/telemetry/internal/telemetry"
)
@@ -123,6 +125,40 @@
X: 0.123456789,
Programs: []*telemetry.ProgramReport{
{
+ Program: "cmd/go",
+ Version: "go1.2.3",
+ GoVersion: "go1.2.3",
+ GOOS: "darwin",
+ GOARCH: "arm64",
+ Counters: map[string]int64{
+ "main": 1,
+ },
+ },
+ {
+ Program: "example.com/mod/pkg",
+ Version: "v2.3.4",
+ GoVersion: "go1.2.3",
+ GOOS: "darwin",
+ GOARCH: "arm64",
+ Counters: map[string]int64{
+ "main": 1,
+ "flag:a": 2,
+ "flag:b": 3,
+ },
+ // TODO: add support for stacks
+ Stacks: map[string]int64{
+ "panic": 4,
+ },
+ },
+ },
+ Config: "v0.0.1",
+ },
+ {
+ Week: "2999-01-01",
+ LastWeek: "2998-01-01",
+ X: 0.123456789,
+ Programs: []*telemetry.ProgramReport{
+ {
Program: "example.com/mod/pkg",
Version: "v1.2.3",
GoVersion: "go1.2.3",
@@ -321,9 +357,150 @@
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
- if got := partition(dat, tt.args.program, tt.args.name, tt.args.buckets, tt.args.xs); !reflect.DeepEqual(got, tt.want) {
+ if got := partition(dat, tt.args.program, tt.args.name, tt.args.buckets); !reflect.DeepEqual(got, tt.want) {
t.Errorf("histogram() = %v, want %v", got, tt.want)
}
})
}
}
+
+func Test_charts(t *testing.T) {
+ dat := nest(reports)
+ cfg := &config.Config{
+ UploadConfig: &telemetry.UploadConfig{
+ GOOS: []string{"darwin"},
+ GOARCH: []string{"amd64"},
+ GoVersion: []string{"go1.2.3"},
+ SampleRate: 1,
+ Programs: []*telemetry.ProgramConfig{
+ {
+ Name: "cmd/go",
+ Versions: []string{"go1.2.3"},
+ Counters: []telemetry.CounterConfig{{
+ Name: "main",
+ }},
+ },
+ {
+ Name: "cmd/compiler",
+ Versions: []string{"go1.2.3"},
+ Counters: []telemetry.CounterConfig{{
+ Name: "count1",
+ }},
+ },
+ {
+ Name: "example.com/mod/pkg",
+ Versions: []string{"v0.15.0"},
+ Counters: []telemetry.CounterConfig{{
+ Name: "count2",
+ }},
+ },
+ },
+ },
+ }
+ want := &chartdata{
+ DateRange: [2]string{"2999-01-01", "2999-01-01"},
+ Programs: []*program{
+ {
+ ID: "charts:cmd/go",
+ Name: "cmd/go",
+ Charts: []*chart{
+ {
+ ID: "charts:cmd/go:GOOS",
+ Name: "GOOS",
+ Type: "partition",
+ Data: []*datum{{
+ Week: "2999-01-01",
+ Key: "darwin",
+ Value: 1,
+ }},
+ },
+ {
+ ID: "charts:cmd/go:GOARCH",
+ Name: "GOARCH",
+ Type: "partition",
+ Data: []*datum{{
+ Week: "2999-01-01",
+ Key: "amd64",
+ Value: 0,
+ }},
+ },
+ {
+ ID: "charts:cmd/go:GoVersion",
+ Name: "GoVersion",
+ Type: "partition",
+ Data: []*datum{{
+ Week: "2999-01-01",
+ Key: "go1.2",
+ Value: 1,
+ }},
+ },
+ {
+ ID: "charts:cmd/go:main",
+ Name: "main",
+ Type: "partition",
+ Data: []*datum{{
+ Week: "2999-01-01",
+ Key: "main",
+ Value: 1,
+ }},
+ },
+ },
+ },
+ {
+ ID: "charts:cmd/compiler",
+ Name: "cmd/compiler",
+ },
+ {
+ ID: "charts:example.com/mod/pkg",
+ Name: "example.com/mod/pkg",
+ Charts: []*chart{
+ {
+ ID: "charts:example.com/mod/pkg:Version",
+ Name: "Version",
+ Type: "partition",
+ Data: []*datum{{
+ Week: "2999-01-01",
+ Key: "v0.15",
+ Value: 0,
+ }},
+ },
+ {
+ ID: "charts:example.com/mod/pkg:GOOS",
+ Name: "GOOS",
+ Type: "partition",
+ Data: []*datum{{
+ Week: "2999-01-01",
+ Key: "darwin",
+ Value: 0.5,
+ }},
+ },
+ {
+ ID: "charts:example.com/mod/pkg:GOARCH",
+ Name: "GOARCH",
+ Type: "partition",
+ Data: []*datum{{
+ Week: "2999-01-01",
+ Key: "amd64",
+ Value: 0.5,
+ }},
+ },
+ {
+ ID: "charts:example.com/mod/pkg:GoVersion",
+ Name: "GoVersion",
+ Type: "partition",
+ Data: []*datum{{
+ Week: "2999-01-01",
+ Key: "go1.2",
+ Value: 1,
+ }},
+ },
+ },
+ },
+ },
+ NumReports: 1,
+ }
+ got := charts(cfg, "2999-01-01", dat, []float64{0.12345})
+ if diff := cmp.Diff(want, got); diff != "" {
+ t.Errorf("charts = %+v\n, (-want +got): %v", got, diff)
+ }
+}
diff --git a/godev/go.mod b/godev/go.mod
index 895a3bc..7b5a3dd 100644
--- a/godev/go.mod
+++ b/godev/go.mod
@@ -1,6 +1,6 @@
module golang.org/x/telemetry/godev
-go 1.20
+go 1.21.0
require (
cloud.google.com/go/cloudtasks v1.12.4
diff --git a/godev/go.sum b/godev/go.sum
index ad83c9c..5d9688b 100644
--- a/godev/go.sum
+++ b/godev/go.sum
@@ -51,6 +51,7 @@
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/google/martian/v3 v3.3.2 h1:IqNFLAmvJOgVlpdEBiQbDc2EwKW77amAycfTuWKdfvw=
+github.com/google/martian/v3 v3.3.2/go.mod h1:oBOf6HBosgwRXnUGWUB05QECsc6uvmMiJ3+6W4l/CUk=
github.com/google/s2a-go v0.1.7 h1:60BLSyTrOV4/haCDW4zb1guZItoSq8foHCXrAnjBo/o=
github.com/google/s2a-go v0.1.7/go.mod h1:50CgR4k1jNlWBu4UfS4AcfhVe1r6pdZPygJ3R8F0Qdw=
github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
diff --git a/internal/content/telemetrygodev/charts.ts b/internal/content/telemetrygodev/charts.ts
index 3fef6b4..0616586 100644
--- a/internal/content/telemetrygodev/charts.ts
+++ b/internal/content/telemetrygodev/charts.ts
@@ -38,6 +38,9 @@
for (const program of Page.Charts.Programs) {
for (const counter of program.Charts) {
+ if (!counter) {
+ continue;
+ }
switch (counter.Type) {
case "partition":
document
