From e68f2bfdb3dc43540febb0874bab0bc559c5ec95 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Wed, 5 Oct 2022 11:16:18 -0700 Subject: Don't trace merge jobs that we don't lock We get a trace from every merger (including executors) for every merge job because we start the trace before attempting the lock. So essentially, we get one trace from the merger that runs the job, and one trace from every other merger indicating that it did not run the job. This is perhaps too much detail for us. While it's true that we can see the response times of every system component here, it may be sufficient to have only the response time of the first merger. This will reduce the noise in trace visualizations significantly. Change-Id: I88c56f00c060eae9316473f4a4e222a0db97e510 --- zuul/merger/server.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/zuul/merger/server.py b/zuul/merger/server.py index fec3df5e0..d99e5d3ee 100644 --- a/zuul/merger/server.py +++ b/zuul/merger/server.py @@ -208,22 +208,25 @@ class BaseMergeServer(metaclass=ABCMeta): if not self._merger_running: break - with tracing.startSpanInContext( - merge_request.span_context, "MergerJob", - attributes={"merger": self.hostname}): - self._runMergeJob(merge_request) + self._lockAndRunMergeJob(merge_request) except Exception: self.log.exception("Error in merge thread:") time.sleep(5) self.merger_loop_wake_event.set() + def _lockAndRunMergeJob(self, merge_request): + # Lock and update the merge request + if not self.merger_api.lock(merge_request, blocking=False): + return + with tracing.startSpanInContext( + merge_request.span_context, "MergerJob", + attributes={"merger": self.hostname}): + self._runMergeJob(merge_request) + def _runMergeJob(self, merge_request): log = get_annotated_logger( self.log, merge_request.event_id ) - # Lock and update the merge request - if not self.merger_api.lock(merge_request, blocking=False): - return # Ensure that the request is still in state requested. This method is # called based on cached data and there might be a mismatch between the -- cgit v1.2.1