Additional metrics to graph runner

controller/getchangedtargets.go

@@ -41,6 +41,7 @@ type job struct {
 // GetChangedTargets returns the changed targets between two revisions.
 func (c *controller) GetChangedTargets(request *pb.GetChangedTargetsRequest, stream pb.TangoServiceGetChangedTargetsYARPCServer) (retErr error) {
 	scope := c.scope.SubScope("get_changed_targets")
+	scope.Counter("calls").Inc(1)
 	defer func() {
 		if retErr != nil {
 			scope.Counter("failure").Inc(1)
@@ -54,7 +55,6 @@ func (c *controller) GetChangedTargets(request *pb.GetChangedTargetsRequest, str
 		return common.WithReason(failureReasonValidation, common.ErrorTypeUser, err)
 	}
 	scope = scope.Tagged(map[string]string{"repo": common.ToShortRemote(request.GetFirstRevision().GetRemote())})
-	scope.Counter("calls").Inc(1)
 	ctx := stream.Context()
 	start := time.Now()
 	logger := c.logger.With(

controller/getchangedtargetsandedges.go

@@ -41,6 +41,7 @@ func packEdge(src, dep int32) uint64 {
 // GetChangedTargetsAndEdges returns the changed targets and edges between two revisions.
 func (c *controller) GetChangedTargetsAndEdges(request *pb.GetChangedTargetsAndEdgesRequest, stream pb.TangoServiceGetChangedTargetsAndEdgesYARPCServer) (retErr error) {
 	scope := c.scope.SubScope("get_changed_targets_and_edges")
+	scope.Counter("calls").Inc(1)
 	defer func() {
 		if retErr != nil {
 			scope.Counter("failure").Inc(1)
@@ -54,7 +55,6 @@ func (c *controller) GetChangedTargetsAndEdges(request *pb.GetChangedTargetsAndE
 		return common.WithReason(failureReasonValidation, common.ErrorTypeUser, err)
 	}
 	scope = scope.Tagged(map[string]string{"repo": common.ToShortRemote(request.GetFirstRevision().GetRemote())})
-	scope.Counter("calls").Inc(1)
 	ctx := stream.Context()
 	start := time.Now()
 	logger := c.logger.With(

controller/gettargetgraph.go

@@ -32,6 +32,7 @@ import (
 // GetTargetGraph returns the target graph for a given request.
 func (c *controller) GetTargetGraph(request *pb.GetTargetGraphRequest, stream pb.TangoServiceGetTargetGraphYARPCServer) (retErr error) {
 	scope := c.scope.SubScope("get_target_graph")
+	scope.Counter("calls").Inc(1)
 	defer func() {
 		if retErr != nil {
 			scope.Counter("failure").Inc(1)
@@ -46,7 +47,6 @@ func (c *controller) GetTargetGraph(request *pb.GetTargetGraphRequest, stream pb
 		zap.Any("build_description", request.GetBuildDescription()),
 	)
 	scope = scope.Tagged(map[string]string{"repo": common.ToShortRemote(request.GetBuildDescription().GetRemote())})
-	scope.Counter("calls").Inc(1)
 	graphReader, err := c.getGraph(ctx, request.GetBuildDescription(), request.GetOutputConfig(), request.GetRequestOptions(), request.GetBypassCache())
 	if err != nil {
 		return err

core/targethasher/graph.go

@@ -630,10 +630,10 @@ func HashRecursively(ctx context.Context, p HashParam) ([]byte, error) {
 		return []byte{}, nil
 	}
 
-	// Mark node as visited by setting it to an empty slice instead of nil slice which it got by default - to avoid
-	// cycles. It would be reset to a real hash value once dependencies are traversed.
+	// Mark node as visited by setting Hash to an empty slice (non-nil) to break cycles.
+	// HashWithoutDeps is intentionally left as-is: toTarget already computed it, and
+	// HashRecursively reuses it below to avoid calling HashRuleCommon twice per rule.
 	target.Hash = []byte{}
-	target.HashWithoutDeps = []byte{}
 
 	var hash []byte
 	var hashWithoutDeps []byte
@@ -663,11 +663,17 @@ func HashRecursively(ctx context.Context, p HashParam) ([]byte, error) {
 		h.Write([]byte(p.TargetName))
 		hash = h.Sum(nil)
 	default:
-		// Regular rule
+		// Regular rule: hash = sha1(ruleBody || dep1Hash || dep2Hash || ...)
+		// toTarget already called HashRuleCommon and stored the result in target.HashWithoutDeps,
+		// so reuse it here rather than re-running HashRuleCommon over all attributes again.
 		h := newHash()
-		noDepsHasher := newHash()
-		HashRuleCommon(target.Rule, noDepsHasher)
-		hashWithoutDeps = noDepsHasher.Sum(nil)
+		hashWithoutDeps = target.HashWithoutDeps
+		if len(hashWithoutDeps) == 0 {
+			// Fallback for synthetic targets that bypass toTarget (shouldn't happen in practice).
+			noDepsHasher := newHash()
+			HashRuleCommon(target.Rule, noDepsHasher)
+			hashWithoutDeps = noDepsHasher.Sum(nil)
+		}
 		h.Write(hashWithoutDeps)
 		for _, dep := range target.Deps {
 			depParam := p

graphrunner/BUILD.bazel

@@ -15,6 +15,7 @@ go_library(
         "//core/git",
         "//core/targethasher",
         "//core/workspace",
+        "@com_github_uber_go_tally//:tally",
     ],
 )
 

graphrunner/native.go

@@ -16,7 +16,9 @@ package graphrunner
 
 import (
 	"context"
+	"time"
 
+	"github.com/uber-go/tally"
 	"github.com/uber/tango/config"
 	"github.com/uber/tango/core/bazel"
 	"github.com/uber/tango/core/git"
@@ -29,22 +31,29 @@ type nativeGraphRunner struct {
 	git                git.Interface
 	config             config.RepositoryConfig
 	extraExcludedFiles []string
+	scope              tally.Scope
 }
 
 type NativeGraphRunnerParams struct {
 	BazelClient        bazel.Bazel
 	GitClient          git.Interface
 	Config             config.RepositoryConfig
 	ExtraExcludedFiles []string
+	Scope              tally.Scope
 }
 
 // graph runner takes in a bazel query request and computes the graph
 func NewNativeGraphRunner(p NativeGraphRunnerParams) GraphRunner {
+	scope := p.Scope
+	if scope == nil {
+		scope = tally.NoopScope
+	}
 	return &nativeGraphRunner{
 		bazel:              p.BazelClient,
 		git:                p.GitClient,
 		config:             p.Config,
 		extraExcludedFiles: p.ExtraExcludedFiles,
+		scope:              scope.SubScope("graph_runner"),
 	}
 }
 
@@ -57,6 +66,8 @@ func (g *nativeGraphRunner) Compute(ctx context.Context, ws workspace.Workspace)
 		[]string{"--order_output=no", "--proto:locations", "--noproto:default_values"},
 		g.config.BazelExtraArgs...,
 	)
+
+	bazelStart := time.Now()
 	queryResult, err := g.bazel.ExecuteQuery(ctx, &bazel.QueryRequest{
 		Query: query,
 		// --order_output=no will make Bazel execute query faster
@@ -67,23 +78,32 @@ func (g *nativeGraphRunner) Compute(ctx context.Context, ws workspace.Workspace)
 
 		AdditionalArgs: additionalArgs,
 	})
+	g.scope.Timer("bazel_query_duration").Record(time.Since(bazelStart))
 	if err != nil {
 		return targethasher.EmptyResult(), err
 	}
+
+	gitStart := time.Now()
 	knownSourceHashes, err := g.git.FileHashes(ctx, "HEAD")
+	g.scope.Timer("git_file_hashes_duration").Record(time.Since(gitStart))
 	if err != nil {
 		return targethasher.EmptyResult(), err
 	}
+
 	hashConfig := targethasher.HashConfig{
 		KnownSourceHashes: knownSourceHashes,
 		FullHashRepos:     g.config.FullHashRepos,
 		ExcludedRegex:     append(g.config.ExcludedFiles, g.extraExcludedFiles...),
 		UseBzlmod:         g.config.BzlmodEnabled,
 	}
 
+	hashStart := time.Now()
 	res, err := targethasher.FromProto(ctx, queryResult.Result, ws.Path(), hashConfig)
+	g.scope.Timer("target_hash_duration").Record(time.Since(hashStart))
 	if err != nil {
 		return targethasher.EmptyResult(), err
 	}
+
+	g.scope.Gauge("target_count").Update(float64(len(res.Targets)))
 	return res, nil
 }

orchestrator/native_orchestrator.go

@@ -191,6 +191,7 @@ func (b *nativeOrchestrator) GetTargetGraph(ctx context.Context, param GetTarget
 			GitClient:          gitModule,
 			Config:             repoCfg,
 			ExtraExcludedFiles: param.Req.GetRequestOptions().GetExtraExcludeFilesRegex(),
+			Scope:              b.scope,
 		})
 	}
 	result, err := runner.Compute(ctx, ws)