golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
121.08k stars 17.36k forks source link

x/telemetry: `gotelemetry view` graphs have data from the future #68206

Open prattmic opened 1 week ago

prattmic commented 1 week ago

On the gotelemetry view web server I see graphs like this:

image

Note "Week 2024-06-30–2024-07-07". Today is 2024-06-26. Showing me what my usage will be in the future is a really cool feature, unfortunately I doubt we have that yet.

From the "Counters" section later down the page, I think this data is coming from this counter file:

addr2line@devel-devel-linux-amd64-2024-06-26.v1.count
Program: cmd/addr2line
Version: devel
GOOS: linux
GOARCH: amd64
GoVersion: devel
TimeBegin: 2024-06-26T00:00:00Z
TimeEnd: 2024-07-02T00:00:00Z
addr2line/invocations 8
The program cmd/addr2line is unregistered. No data from this set would be uploaded to the Go team.

cc @golang/telemetry

hyangah commented 2 days ago

gotelemetry view renders x/telemetry/internal/telemetry.Report data. According to the definition of x/telemetry/internal/telemetry.Report, the Week is the first day the report covers.

type Report struct {
    Week     string  // first day this report covers (YYYY-MM-DD)
    LastWeek string  // Week field from latest previous report uploaded
    ...
}

But, it looks to me that the Uploader sets the TimeEnd (expiryDate) as the Report.Week value. (internal/upload/reports.go)

gotelemetry view's pending telemetry.Report creation also uses TimeEnd to match the uploader's logic.

I am afraid it's too late to fix the Uploader's report generation so it sets Report.Weekwith the first day of the week span. Can we change the definition of the Report.Week to match the current implementation?

findleyr commented 2 days ago

Thanks for investigating. Yes, let's update the definition of Week to match the implementation.

gopherbot commented 2 days ago

Change https://go.dev/cl/595962 mentions this issue: cmd/gotelemetry: fix chart x-axis time interval display