From 7f1b33f0c89e24e50f13e2ad9636ce284daa4c65 Mon Sep 17 00:00:00 2001 From: Ethan Lee Date: Thu, 18 Dec 2025 03:32:22 +0000 Subject: internal/dcensus: add metrics for codewiki link usage - Count link usage, target url and referrer page. - To avoid increasing dependency size, use indirection to call recordCodeWikiMetrics. Example: Run `go run ./cmd/frontend/main.go -dev -direct_proxy` and access pkgsite via localhost:8080. After clicking on the link, observe the relevant metric logged in localhost:8081/statz. Run `go test -v -run TestCmdPkgsiteDeps` to ensure that cmd/pkgsite dependencies remain stable. Change-Id: Ib312584162b81deac4c22b4ed923ff783133e11e Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/731020 Reviewed-by: Jonathan Amsterdam kokoro-CI: kokoro Auto-Submit: Ethan Lee LUCI-TryBot-Result: Go LUCI --- cmd/frontend/main.go | 26 +++++----- cmd/internal/pkgsite/server.go | 21 ++++---- cmd/pkgsite/main.go | 1 + internal/dcensus/dcensus.go | 24 +++++++++ internal/frontend/links.go | 19 ++++--- internal/frontend/links_test.go | 2 +- internal/frontend/server.go | 111 +++++++++++++++++++++++----------------- internal/frontend/unit.go | 2 +- 8 files changed, 129 insertions(+), 77 deletions(-) diff --git a/cmd/frontend/main.go b/cmd/frontend/main.go index c3541cd6..7ed399fe 100644 --- a/cmd/frontend/main.go +++ b/cmd/frontend/main.go @@ -152,18 +152,19 @@ func main() { TaskIDChangeInterval: config.TaskIDChangeIntervalFrontend, } server, err := frontend.NewServer(frontend.ServerConfig{ - Config: cfg, - FetchServer: fetchServer, - DataSourceGetter: dsg, - Queue: fetchQueue, - TemplateFS: template.TrustedFSFromTrustedSource(staticSource), - StaticFS: os.DirFS(*staticFlag), - ThirdPartyFS: os.DirFS(*thirdPartyPath), - DevMode: *devMode, - LocalMode: *localMode, - Reporter: reporter, - VulndbClient: vc, - HTTPClient: &http.Client{Transport: new(ochttp.Transport)}, + Config: cfg, + FetchServer: fetchServer, + DataSourceGetter: dsg, + Queue: fetchQueue, + TemplateFS: template.TrustedFSFromTrustedSource(staticSource), + StaticFS: os.DirFS(*staticFlag), + ThirdPartyFS: os.DirFS(*thirdPartyPath), + DevMode: *devMode, + LocalMode: *localMode, + Reporter: reporter, + VulndbClient: vc, + HTTPClient: &http.Client{Transport: new(ochttp.Transport)}, + RecordCodeWikiMetrics: dcensus.RecordClick, }) if err != nil { log.Fatalf(ctx, "frontend.NewServer: %v", err) @@ -192,6 +193,7 @@ func main() { middleware.CacheErrorCount, middleware.CacheLatency, middleware.QuotaResultCount, + dcensus.CodeWikiClickCountView, ) if err := dcensus.Init(cfg, views...); err != nil { log.Fatal(ctx, err) diff --git a/cmd/internal/pkgsite/server.go b/cmd/internal/pkgsite/server.go index 22d9e8f3..4dcf7e80 100644 --- a/cmd/internal/pkgsite/server.go +++ b/cmd/internal/pkgsite/server.go @@ -32,16 +32,17 @@ import ( // ServerConfig provides configuration for BuildServer. type ServerConfig struct { - Paths []string - GOPATHMode bool - UseCache bool - CacheDir string - UseListedMods bool - UseLocalStdlib bool - DevMode bool - DevModeStaticDir string - GoRepoPath string - GoDocMode bool + Paths []string + GOPATHMode bool + UseCache bool + CacheDir string + UseListedMods bool + UseLocalStdlib bool + DevMode bool + DevModeStaticDir string + GoRepoPath string + GoDocMode bool + RecordCodeWikiMetrics frontend.RecordClickFunc Proxy *proxy.Client // client, or nil; controlled by the -proxy flag } diff --git a/cmd/pkgsite/main.go b/cmd/pkgsite/main.go index cd0a3b49..49b99a59 100644 --- a/cmd/pkgsite/main.go +++ b/cmd/pkgsite/main.go @@ -100,6 +100,7 @@ func main() { serverCfg.UseLocalStdlib = true serverCfg.GoRepoPath = *goRepoPath serverCfg.Paths = collectPaths(flag.Args()) + serverCfg.RecordCodeWikiMetrics = nil if serverCfg.UseCache || *useProxy { fmt.Fprintf(os.Stderr, "BYPASSING LICENSE CHECKING: MAY DISPLAY NON-REDISTRIBUTABLE INFORMATION\n") diff --git a/internal/dcensus/dcensus.go b/internal/dcensus/dcensus.go index 1447a0d4..2f6ac7a5 100644 --- a/internal/dcensus/dcensus.go +++ b/internal/dcensus/dcensus.go @@ -57,6 +57,14 @@ func NewRouter(tagger RouteTagger) *Router { } } +func RecordClick(ctx context.Context, url string, target string) { + mutators := []tag.Mutator{ + tag.Upsert(KeyTargetURL, url), + tag.Upsert(KeyReferrer, target), + } + stats.RecordWithTags(ctx, mutators, CodeWikiClickCount.M(1)) +} + // Handle registers handler with the given route. It has the same routing // semantics as http.ServeMux. func (r *Router) Handle(route string, handler http.Handler) { @@ -241,6 +249,22 @@ var ( } ) +var ( + // KeyReferrer is a tag key for the referrer URL. + KeyReferrer = tag.MustNewKey("referrer") + // KeyTargetURL is a tag key for the target URL. + KeyTargetURL = tag.MustNewKey("target_url") + + CodeWikiClickCount = stats.Int64("go-discovery/frontend_codewiki_clicks", "Codewiki link clicks", stats.UnitDimensionless) + CodeWikiClickCountView = &view.View{ + Name: "go-discovery/frontend/codewiki_clicks", + Description: "Count of Codewiki link clicks", + TagKeys: []tag.Key{KeyReferrer, KeyTargetURL}, + Measure: CodeWikiClickCount, + Aggregation: view.Count(), + } +) + // RecordWithTag is a convenience function for recording a single measurement with a single tag. func RecordWithTag(ctx context.Context, key tag.Key, val string, m stats.Measurement) { stats.RecordWithTags(ctx, []tag.Mutator{tag.Upsert(key, val)}, m) diff --git a/internal/frontend/links.go b/internal/frontend/links.go index 2edc6804..c4cf8281 100644 --- a/internal/frontend/links.go +++ b/internal/frontend/links.go @@ -8,7 +8,6 @@ import ( "context" "encoding/json" "errors" - "fmt" "net/http" "net/url" "strings" @@ -24,6 +23,8 @@ const ( depsDevBase = "https://deps.dev" // depsDevTimeout is the time budget for making requests to deps.dev. depsDevTimeout = 250 * time.Millisecond + + codeWikiPrefix = "/codewiki?url=" ) var ( @@ -107,16 +108,16 @@ func fetchDepsDevURL(ctx context.Context, client *http.Client, modulePath, versi // codeWikiURLGenerator returns a function that will return a URL for the given // module version on codewiki. If the URL can't be generated within // codeWikiTimeout then the empty string is returned instead. -func codeWikiURLGenerator(ctx context.Context, client *http.Client, um *internal.UnitMeta) func() string { +func codeWikiURLGenerator(ctx context.Context, client *http.Client, um *internal.UnitMeta, recordClick bool) func() string { fetch := func(ctx context.Context, client *http.Client) (string, error) { - return fetchCodeWikiURL(ctx, client, um.ModulePath) + return fetchCodeWikiURL(ctx, client, um.ModulePath, recordClick) } return newURLGenerator(ctx, client, "codewiki.google", codeWikiTimeout, fetch) } // fetchCodeWikiURL makes a request to codewiki to check whether the given // path is known there, and if so it returns the link to that page. -func fetchCodeWikiURL(ctx context.Context, client *http.Client, path string) (string, error) { +func fetchCodeWikiURL(ctx context.Context, client *http.Client, path string, recordClick bool) (string, error) { if strings.HasPrefix(path, "golang.org/x/") { path = strings.Replace(path, "golang.org/x/", "github.com/golang/", 1) } @@ -133,8 +134,12 @@ func fetchCodeWikiURL(ctx context.Context, client *http.Client, path string) (st return "", err } defer resp.Body.Close() - if resp.StatusCode == http.StatusOK { - return fmt.Sprintf("%s%s", codeWikiURLBase, path), nil + if resp.StatusCode != http.StatusOK { + return "", errors.New(resp.Status) + } + res := codeWikiURLBase + path + if recordClick { + res = codeWikiPrefix + res } - return "", errors.New(resp.Status) + return res, nil } diff --git a/internal/frontend/links_test.go b/internal/frontend/links_test.go index b5417e9d..e2e83009 100644 --- a/internal/frontend/links_test.go +++ b/internal/frontend/links_test.go @@ -76,7 +76,7 @@ func TestCodeWikiURLGenerator(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { um := &internal.UnitMeta{ModuleInfo: internal.ModuleInfo{ModulePath: tc.modulePath}} - url := codeWikiURLGenerator(context.Background(), server.Client(), um)() + url := codeWikiURLGenerator(context.Background(), server.Client(), um, false)() if url != tc.want { t.Errorf("codeWikiURLGenerator(ctx, client, %q) = %q, want %q, got %q", tc.path, url, tc.want, url) } diff --git a/internal/frontend/server.go b/internal/frontend/server.go index e15f9298..9648c5e7 100644 --- a/internal/frontend/server.go +++ b/internal/frontend/server.go @@ -42,25 +42,26 @@ import ( type Server struct { fetchServer FetchServerInterface // getDataSource should never be called from a handler. It is called only in Server.errorHandler. - getDataSource func(context.Context) internal.DataSource - queue queue.Queue - templateFS template.TrustedFS - staticFS fs.FS - thirdPartyFS fs.FS - devMode bool - goDocMode bool // running to serve documentation for 'go doc' - localMode bool // running locally (i.e. ./cmd/pkgsite) - localModules []LocalModule // locally hosted modules; empty in production - errorPage []byte - appVersionLabel string - googleTagManagerID string - serveStats bool - reporter derrors.Reporter - fileMux *http.ServeMux - vulnClient *vuln.Client - versionID string - instanceID string - HTTPClient *http.Client + getDataSource func(context.Context) internal.DataSource + queue queue.Queue + templateFS template.TrustedFS + staticFS fs.FS + thirdPartyFS fs.FS + devMode bool + goDocMode bool // running to serve documentation for 'go doc' + localMode bool // running locally (i.e. ./cmd/pkgsite) + localModules []LocalModule // locally hosted modules; empty in production + errorPage []byte + appVersionLabel string + googleTagManagerID string + serveStats bool + reporter derrors.Reporter + fileMux *http.ServeMux + vulnClient *vuln.Client + versionID string + instanceID string + HTTPClient *http.Client + recordCodeWikiMetrics RecordClickFunc mu sync.Mutex // Protects all fields below templates map[string]*template.Template @@ -77,6 +78,8 @@ type FetchServerInterface interface { ds internal.PostgresDB, fullPath, modulePath, requestedVersion string) (err error) } +type RecordClickFunc func(ctx context.Context, url string, target string) + // ServerConfig contains everything needed by a Server. type ServerConfig struct { Config *config.Config @@ -84,18 +87,19 @@ type ServerConfig struct { FetchServer FetchServerInterface // DataSourceGetter should return a DataSource on each call. // It should be goroutine-safe. - DataSourceGetter func(context.Context) internal.DataSource - Queue queue.Queue - TemplateFS template.TrustedFS // for loading templates safely - StaticFS fs.FS // for static/ directory - ThirdPartyFS fs.FS // for third_party/ directory - DevMode bool - LocalMode bool - GoDocMode bool - LocalModules []LocalModule - Reporter derrors.Reporter - VulndbClient *vuln.Client - HTTPClient *http.Client + DataSourceGetter func(context.Context) internal.DataSource + Queue queue.Queue + TemplateFS template.TrustedFS // for loading templates safely + StaticFS fs.FS // for static/ directory + ThirdPartyFS fs.FS // for third_party/ directory + DevMode bool + LocalMode bool + GoDocMode bool + LocalModules []LocalModule + Reporter derrors.Reporter + VulndbClient *vuln.Client + HTTPClient *http.Client + RecordCodeWikiMetrics RecordClickFunc } // NewServer creates a new Server for the given database and template directory. @@ -107,21 +111,22 @@ func NewServer(scfg ServerConfig) (_ *Server, err error) { } dochtml.LoadTemplates(scfg.TemplateFS) s := &Server{ - fetchServer: scfg.FetchServer, - getDataSource: scfg.DataSourceGetter, - queue: scfg.Queue, - templateFS: scfg.TemplateFS, - staticFS: scfg.StaticFS, - thirdPartyFS: scfg.ThirdPartyFS, - devMode: scfg.DevMode, - localMode: scfg.LocalMode, - goDocMode: scfg.GoDocMode, - localModules: scfg.LocalModules, - templates: ts, - reporter: scfg.Reporter, - fileMux: http.NewServeMux(), - vulnClient: scfg.VulndbClient, - HTTPClient: scfg.HTTPClient, + fetchServer: scfg.FetchServer, + getDataSource: scfg.DataSourceGetter, + queue: scfg.Queue, + templateFS: scfg.TemplateFS, + staticFS: scfg.StaticFS, + thirdPartyFS: scfg.ThirdPartyFS, + devMode: scfg.DevMode, + localMode: scfg.LocalMode, + goDocMode: scfg.GoDocMode, + localModules: scfg.LocalModules, + templates: ts, + reporter: scfg.Reporter, + fileMux: http.NewServeMux(), + vulnClient: scfg.VulndbClient, + HTTPClient: scfg.HTTPClient, + recordCodeWikiMetrics: scfg.RecordCodeWikiMetrics, } if s.HTTPClient == nil { s.HTTPClient = http.DefaultClient @@ -141,6 +146,19 @@ func NewServer(scfg ServerConfig) (_ *Server, err error) { return s, nil } +func (s *Server) handleCodeWikiRedirect(w http.ResponseWriter, r *http.Request) { + url := r.FormValue("url") + if url == "" { + http.Error(w, "missing url", http.StatusBadRequest) + return + } + ctx := r.Context() + if s.recordCodeWikiMetrics != nil { + s.recordCodeWikiMetrics(ctx, url, r.Header.Get("Referer")) + } + http.Redirect(w, r, url, http.StatusFound) +} + // A Cacher is used to create request caches for http handlers. type Cacher interface { // Cache returns a new middleware that caches every request. @@ -211,6 +229,7 @@ func (s *Server) Install(handle func(string, http.Handler), cacher Cacher, authV // (This is what golang.org/C does.) http.Redirect(w, r, "/cmd/cgo", http.StatusMovedPermanently) })) + handle("GET /codewiki", http.HandlerFunc(s.handleCodeWikiRedirect)) handle("GET /golang.org/x", s.staticPageHandler("subrepo", "Sub-repositories")) handle("GET /files/", http.StripPrefix("/files", s.fileMux)) handle("GET /vuln/", vulnHandler) diff --git a/internal/frontend/unit.go b/internal/frontend/unit.go index 55471797..f1f04d95 100644 --- a/internal/frontend/unit.go +++ b/internal/frontend/unit.go @@ -147,7 +147,7 @@ func (s *Server) serveUnitPage(ctx context.Context, w http.ResponseWriter, r *ht makeCodeWikiURL := func() string { return "" } if !s.goDocMode { makeDepsDevURL = depsDevURLGenerator(ctx, s.HTTPClient, um) - makeCodeWikiURL = codeWikiURLGenerator(ctx, s.HTTPClient, um) + makeCodeWikiURL = codeWikiURLGenerator(ctx, s.HTTPClient, um, s.recordCodeWikiMetrics != nil) } // Use GOOS and GOARCH query parameters to create a build context, which -- cgit v1.3