From 7ccf9a0776fcbe6e975e94e1d466d37f9f10978d Mon Sep 17 00:00:00 2001 From: Xiaoyang Tan Date: Fri, 24 Jul 2026 21:02:46 -0700 Subject: [PATCH] refactor(controller): fold compare_target_graphs metrics into get_changed_targets compareTargetGraphs is a private sub-step of GetChangedTargets, not its own RPC, so it should not carry a separate op. Route its phase metrics (decode_duration, diff_duration) under opGetChangedTargets, matching the other helpers (cache_read_duration, graph_fetch_duration, target_count). Replace the Begin/Complete lifecycle with a single compare_duration histogram: an error in this step already surfaces on get_changed_targets.finish, so the sub-op finish histogram was double-counting the same failure under a second op name. Total step latency is preserved via compare_duration. BREAKING CHANGE: the controller.compare_target_graphs.* metrics (start, finish, decode_duration, diff_duration) are removed. Use controller.get_changed_targets.{compare_duration,decode_duration,diff_duration} instead. --- controller/getchangedtargets.go | 12 +++++++----- controller/metrics.go | 1 - 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/controller/getchangedtargets.go b/controller/getchangedtargets.go index 19c49d8..8bf224a 100644 --- a/controller/getchangedtargets.go +++ b/controller/getchangedtargets.go @@ -371,9 +371,11 @@ func (c *controller) cacheComparedTargets(logger *zap.Logger, request *pb.GetCha // are re-mapped into a canonical per-call ID namespace so the response metadata // only carries the names actually referenced. See internal/targetdiff for the // classification and distance rules. -func (c *controller) compareTargetGraphs(ctx context.Context, e *metrics.Emitter, logger *zap.Logger, firstGraph, secondGraph []entity.GetTargetGraphResponse, maxDist int32) (_ []entity.GetChangedTargetsResponse, retErr error) { - op := metrics.Begin(e, opCompareTargetGraphs, metrics.SlowDurationBuckets) - defer func() { op.Complete(retErr) }() +func (c *controller) compareTargetGraphs(ctx context.Context, e *metrics.Emitter, logger *zap.Logger, firstGraph, secondGraph []entity.GetTargetGraphResponse, maxDist int32) ([]entity.GetChangedTargetsResponse, error) { + compareStart := time.Now() + defer func() { + e.DurationHistogram(opGetChangedTargets, "compare_duration", metrics.SlowDurationBuckets).RecordDuration(time.Since(compareStart)) + }() logger.Info("compareTargetGraphs: Computing differences between target graphs") // 1) Decode each stream into a semantic graph keyed by canonical target name. @@ -402,7 +404,7 @@ func (c *controller) compareTargetGraphs(ctx context.Context, e *metrics.Emitter } secondTargetsByID = nil secondMetadata = nil - e.DurationHistogram(opCompareTargetGraphs, "decode_duration", metrics.FastDurationBuckets).RecordDuration(time.Since(decodeStart)) + e.DurationHistogram(opGetChangedTargets, "decode_duration", metrics.FastDurationBuckets).RecordDuration(time.Since(decodeStart)) // 2) Compare the two semantic graphs. diffStart := time.Now() @@ -417,7 +419,7 @@ func (c *controller) compareTargetGraphs(ctx context.Context, e *metrics.Emitter // Release the input graphs; only result is needed from here on. before = nil after = nil - e.DurationHistogram(opCompareTargetGraphs, "diff_duration", metrics.FastDurationBuckets).RecordDuration(time.Since(diffStart)) + e.DurationHistogram(opGetChangedTargets, "diff_duration", metrics.FastDurationBuckets).RecordDuration(time.Since(diffStart)) e.ValueHistogram(opGetChangedTargets, "target_count", metrics.LargeCountBuckets).RecordValue(float64(len(result.ChangedTargets))) if ctx.Err() != nil { diff --git a/controller/metrics.go b/controller/metrics.go index 9ce59ef..559e8ee 100644 --- a/controller/metrics.go +++ b/controller/metrics.go @@ -19,5 +19,4 @@ const ( opGetTargetGraph = "get_target_graph" opGetChangedTargets = "get_changed_targets" opGetChangedTargetGraph = "get_changed_target_graph" - opCompareTargetGraphs = "compare_target_graphs" )