Skip to content

Commit

Permalink
Fix the UMA metrics fetcher (#940)
Browse files Browse the repository at this point in the history 
Currently, it only gets the latest metrics. However, we should be sending the date

[Example](https://github.com/GoogleChrome/chromium-dashboard/blob/19f27ae20f81eaa2106360dd8e2e66ee82547b60/internals/fetchmetrics.py#L92) of doing it in ChromeStatus.
  • Loading branch information
@jcscottiii
jcscottiii authored Nov 25, 2024
1 parent a063efa commit b680de0
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 13 deletions.
2 changes: 1 addition & 1 deletion util/go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ go 1.22.7
toolchain go1.23.2

require (
cloud.google.com/go v0.116.0
cloud.google.com/go/spanner v1.70.0
github.com/GoogleChrome/webstatus.dev/lib v0.0.0-20241028191306-75f646a1ea74
github.com/GoogleChrome/webstatus.dev/lib/gen v0.0.0-20241028191306-75f646a1ea74
Expand All @@ -16,7 +17,6 @@ require (

require (
cel.dev/expr v0.18.0 // indirect
cloud.google.com/go v0.116.0 // indirect
cloud.google.com/go/auth v0.9.9 // indirect
cloud.google.com/go/auth/oauth2adapt v0.2.4 // indirect
cloud.google.com/go/cloudtasks v1.13.2 // indirect
Expand Down
16 changes: 12 additions & 4 deletions workflows/steps/services/uma_export/workflow/http_metrics_fetcher.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"net/url"
"time"

"cloud.google.com/go/civil"
"github.com/GoogleChrome/webstatus.dev/lib/metricdatatypes"
)

Expand Down Expand Up @@ -62,8 +63,8 @@ type HTTPMetricsFetcher struct {
}

func (f HTTPMetricsFetcher) Fetch(ctx context.Context,
queryName metricdatatypes.UMAExportQuery) (io.ReadCloser, error) {
queryURL := f.queryURL(queryName)
queryName metricdatatypes.UMAExportQuery, date civil.Date) (io.ReadCloser, error) {
queryURL := f.queryURL(queryName, date)

token, err := f.tokenGen.Generate(ctx, queryURL)
if err != nil {
Expand Down Expand Up @@ -102,6 +103,13 @@ func (f HTTPMetricsFetcher) Fetch(ctx context.Context,
return resp.Body, nil
}

func (f HTTPMetricsFetcher) queryURL(queryName metricdatatypes.UMAExportQuery) string {
return f.baseURL.JoinPath(string(queryName)).String()
func (f HTTPMetricsFetcher) queryURL(queryName metricdatatypes.UMAExportQuery, date civil.Date) string {
u := f.baseURL.JoinPath(string(queryName))
q := u.Query()
// Format the date into YYYYMMDDD
// More information in https://go.dev/src/time/format.go
q.Add("date", date.In(time.UTC).Format("20060102"))
u.RawQuery = q.Encode()

return u.String()
}
10 changes: 8 additions & 2 deletions workflows/steps/services/uma_export/workflow/http_metrics_fetcher_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,16 @@ import (
"strings"
"testing"

"cloud.google.com/go/civil"
"github.com/GoogleChrome/webstatus.dev/lib/metricdatatypes"
)

func TestHTTPMetricsFetcher_Fetch(t *testing.T) {
sampleDate := civil.Date{Year: 2024, Month: 9, Day: 18}
tests := []struct {
name string
queryName metricdatatypes.UMAExportQuery
date civil.Date
expectedURL string
token string
tokenErr error
Expand All @@ -40,6 +43,7 @@ func TestHTTPMetricsFetcher_Fetch(t *testing.T) {
{
name: "success",
queryName: metricdatatypes.WebDXFeaturesQuery,
date: sampleDate,
token: "test-token",
httpStatus: http.StatusOK,
responseBody: `{
Expand All @@ -50,13 +54,14 @@ func TestHTTPMetricsFetcher_Fetch(t *testing.T) {
"queryName": "WebFeatureObserverDailyMetrics",
"rows": []
}`,
expectedURL: "https://uma-export.appspot.com/webstatus/usecounter.webdxfeatures",
expectedURL: "https://uma-export.appspot.com/webstatus/usecounter.webdxfeatures?date=20240918",
err: nil,
tokenErr: nil,
},
{
name: "error generating token",
queryName: metricdatatypes.WebDXFeaturesQuery,
date: sampleDate,
tokenErr: errors.New("some error"),
httpStatus: http.StatusOK,
expectedURL: "",
Expand All @@ -69,6 +74,7 @@ func TestHTTPMetricsFetcher_Fetch(t *testing.T) {
{
name: "error unexpected status code",
queryName: metricdatatypes.WebDXFeaturesQuery,
date: sampleDate,
token: "test-token",
httpStatus: http.StatusInternalServerError,
expectedURL: "",
Expand Down Expand Up @@ -108,7 +114,7 @@ func TestHTTPMetricsFetcher_Fetch(t *testing.T) {
Transport: mockTransport,
}

got, err := fetcher.Fetch(context.Background(), tc.queryName)
got, err := fetcher.Fetch(context.Background(), tc.queryName, sampleDate)
if !errors.Is(err, tc.err) {
t.Errorf("Fetch() error = %s, expected %s", err, tc.err)

Expand Down
4 changes: 2 additions & 2 deletions workflows/steps/services/uma_export/workflow/job_processor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ type MetricStorer interface {
}

type MetricFetecher interface {
Fetch(context.Context, metricdatatypes.UMAExportQuery) (io.ReadCloser, error)
Fetch(context.Context, metricdatatypes.UMAExportQuery, civil.Date) (io.ReadCloser, error)
}

type MetricParser interface {
Expand Down Expand Up @@ -90,7 +90,7 @@ func (p UMAExportJobProcessor) Process(ctx context.Context, job JobArguments) er
slog.InfoContext(ctx, "No capstone entry found. Will fetch", "date", job.day)

// Step 2. Fetch results
rawData, err := p.metricFetcher.Fetch(ctx, job.queryName)
rawData, err := p.metricFetcher.Fetch(ctx, job.queryName, job.day)
if err != nil {
slog.ErrorContext(ctx, "unable to fetch metrics", "error", err)

Expand Down
18 changes: 14 additions & 4 deletions workflows/steps/services/uma_export/workflow/job_processor_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,13 +179,21 @@ func TestUMAExportJobProcessor_Process(t *testing.T) {
}

mockFetcher := &mockMetricFetcher{
fetchFunc: func(_ context.Context, _ metricdatatypes.UMAExportQuery) (io.ReadCloser, error) {
fetchFunc: func(_ context.Context, t *testing.T,
query metricdatatypes.UMAExportQuery, date civil.Date) (io.ReadCloser, error) {
if query != sampleQuery {
t.Errorf("received bad query.\nexpected: %s\nreceived: %s\n", sampleQuery, query)
}
if sampleDate.Compare(date) != 0 {
t.Errorf("received bad query.\nexpected: %s\nreceived: %s\n", sampleDate, date)
}
if tt.fetchErr != nil {
return nil, tt.fetchErr
}

return io.NopCloser(strings.NewReader("mock data")), nil
},
t: t,
}

mockParser := &mockMetricParser{
Expand Down Expand Up @@ -249,13 +257,15 @@ func (m *mockMetricStorer) SaveCapstone(
}

type mockMetricFetcher struct {
fetchFunc func(ctx context.Context, queryName metricdatatypes.UMAExportQuery) (io.ReadCloser, error)
fetchFunc func(ctx context.Context, t *testing.T, queryName metricdatatypes.UMAExportQuery,
date civil.Date) (io.ReadCloser, error)
t *testing.T
}

func (m *mockMetricFetcher) Fetch(
ctx context.Context, queryName metricdatatypes.UMAExportQuery) (io.ReadCloser, error) {
ctx context.Context, queryName metricdatatypes.UMAExportQuery, date civil.Date) (io.ReadCloser, error) {
if m.fetchFunc != nil {
return m.fetchFunc(ctx, queryName)
return m.fetchFunc(ctx, m.t, queryName, date)
}

return nil, errMetricsResponseNil
Expand Down

0 comments on commit b680de0

Please sign in to comment.