diff --git a/src/python/pants/bin/local_pants_runner.py b/src/python/pants/bin/local_pants_runner.py index 510b3658594..ce295e277b5 100644 --- a/src/python/pants/bin/local_pants_runner.py +++ b/src/python/pants/bin/local_pants_runner.py @@ -88,15 +88,8 @@ def _init_graph_session( dynamic_ui = global_scope.dynamic_ui if global_scope.v2 else False use_colors = global_scope.get("colors", True) - zipkin_trace_v2 = options.for_scope("reporting").zipkin_trace_v2 - # TODO(#8658) This should_report_workunits flag must be set to True for - # StreamingWorkunitHandler to receive WorkUnits. It should eventually - # be merged with the zipkin_trace_v2 flag, since they both involve most - # of the same engine functionality, but for now is separate to avoid - # breaking functionality associated with zipkin tracing while iterating on streaming workunit reporting. stream_workunits = len(options.for_global_scope().streaming_workunits_handlers) != 0 return graph_scheduler_helper.new_session( - zipkin_trace_v2, RunTracker.global_instance().run_id, dynamic_ui=dynamic_ui, use_colors=use_colors, diff --git a/src/python/pants/engine/internals/native.py b/src/python/pants/engine/internals/native.py index ecfc7ad19ac..b3e81839a7c 100644 --- a/src/python/pants/engine/internals/native.py +++ b/src/python/pants/engine/internals/native.py @@ -174,16 +174,9 @@ def new_execution_request(self) -> PyExecutionRequest: return PyExecutionRequest() def new_session( - self, - scheduler, - should_record_zipkin_spans, - dynamic_ui: bool, - build_id, - should_report_workunits: bool, + self, scheduler, dynamic_ui: bool, build_id, should_report_workunits: bool, ) -> PySession: - return PySession( - scheduler, should_record_zipkin_spans, dynamic_ui, build_id, should_report_workunits, - ) + return PySession(scheduler, dynamic_ui, build_id, should_report_workunits,) def new_scheduler( self, diff --git a/src/python/pants/engine/internals/scheduler.py b/src/python/pants/engine/internals/scheduler.py index 1971dd4472a..83bb588fa88 100644 --- a/src/python/pants/engine/internals/scheduler.py +++ b/src/python/pants/engine/internals/scheduler.py @@ -350,17 +350,13 @@ def garbage_collect_store(self): self._native.lib.garbage_collect_store(self._scheduler) def new_session( - self, - zipkin_trace_v2: bool, - build_id, - dynamic_ui: bool = False, - should_report_workunits: bool = False, + self, build_id, dynamic_ui: bool = False, should_report_workunits: bool = False, ) -> "SchedulerSession": """Creates a new SchedulerSession for this Scheduler.""" return SchedulerSession( self, self._native.new_session( - self._scheduler, zipkin_trace_v2, dynamic_ui, build_id, should_report_workunits, + self._scheduler, dynamic_ui, build_id, should_report_workunits, ), ) diff --git a/src/python/pants/engine/internals/scheduler_test_base.py b/src/python/pants/engine/internals/scheduler_test_base.py index 5cc26e9ef0d..59c357063ad 100644 --- a/src/python/pants/engine/internals/scheduler_test_base.py +++ b/src/python/pants/engine/internals/scheduler_test_base.py @@ -76,9 +76,7 @@ def mk_scheduler( include_trace_on_error=include_trace_on_error, ) return scheduler.new_session( - zipkin_trace_v2=False, - build_id="buildid_for_test", - should_report_workunits=should_report_workunits, + build_id="buildid_for_test", should_report_workunits=should_report_workunits, ) def context_with_scheduler(self, scheduler, *args, **kwargs): diff --git a/src/python/pants/init/engine_initializer.py b/src/python/pants/init/engine_initializer.py index 54d603f7a6b..b8be3377cd2 100644 --- a/src/python/pants/init/engine_initializer.py +++ b/src/python/pants/init/engine_initializer.py @@ -174,16 +174,9 @@ class LegacyGraphScheduler: goal_map: Any def new_session( - self, - zipkin_trace_v2, - build_id, - dynamic_ui: bool = False, - use_colors=True, - should_report_workunits=False, + self, build_id, dynamic_ui: bool = False, use_colors=True, should_report_workunits=False, ) -> "LegacyGraphSession": - session = self.scheduler.new_session( - zipkin_trace_v2, build_id, dynamic_ui, should_report_workunits - ) + session = self.scheduler.new_session(build_id, dynamic_ui, should_report_workunits) console = Console(use_colors=use_colors, session=session if dynamic_ui else None,) return LegacyGraphSession(session, console, self.build_file_aliases, self.goal_map) diff --git a/src/python/pants/pantsd/service/scheduler_service.py b/src/python/pants/pantsd/service/scheduler_service.py index 94bfe638199..30d9bc3dd2b 100644 --- a/src/python/pants/pantsd/service/scheduler_service.py +++ b/src/python/pants/pantsd/service/scheduler_service.py @@ -49,9 +49,7 @@ def __init__( self._scheduler = legacy_graph_scheduler.scheduler # This session is only used for checking whether any invalidation globs have been invalidated. # It is not involved with a build itself; just with deciding when we should restart pantsd. - self._scheduler_session = self._scheduler.new_session( - zipkin_trace_v2=False, build_id="scheduler_service_session", - ) + self._scheduler_session = self._scheduler.new_session(build_id="scheduler_service_session",) self._logger = logging.getLogger(__name__) # NB: We declare these as a single field so that they can be changed atomically. diff --git a/src/python/pants/pantsd/service/store_gc_service.py b/src/python/pants/pantsd/service/store_gc_service.py index a3995f434e6..6846d41c47e 100644 --- a/src/python/pants/pantsd/service/store_gc_service.py +++ b/src/python/pants/pantsd/service/store_gc_service.py @@ -23,9 +23,7 @@ def __init__( gc_interval_secs=(4 * 60 * 60), ): super().__init__() - self._scheduler_session = scheduler.new_session( - zipkin_trace_v2=False, build_id="store_gc_service_session" - ) + self._scheduler_session = scheduler.new_session(build_id="store_gc_service_session") self._logger = logging.getLogger(__name__) self._period_secs = period_secs diff --git a/src/python/pants/reporting/reporting.py b/src/python/pants/reporting/reporting.py index 956b1d7aaaf..6471e133233 100644 --- a/src/python/pants/reporting/reporting.py +++ b/src/python/pants/reporting/reporting.py @@ -94,13 +94,6 @@ def register_options(cls, register): default=100.0, help="Rate at which to sample Zipkin traces. Value 0.0 - 100.0.", ) - register( - "--zipkin-trace-v2", - advanced=True, - type=bool, - default=False, - help="If enabled, the zipkin spans are tracked for v2 engine execution progress.", - ) register( "--zipkin-service-name-prefix", advanced=True, diff --git a/src/python/pants/testutil/test_base.py b/src/python/pants/testutil/test_base.py index a9c038064bf..db162c14eba 100644 --- a/src/python/pants/testutil/test_base.py +++ b/src/python/pants/testutil/test_base.py @@ -429,9 +429,7 @@ def _init_engine(self, local_store_dir: Optional[str] = None) -> None: build_configuration=self.build_config(), build_ignore_patterns=None, execution_options=ExecutionOptions.from_bootstrap_options(global_options), - ).new_session( - zipkin_trace_v2=False, build_id="buildid_for_test", should_report_workunits=True - ) + ).new_session(build_id="buildid_for_test", should_report_workunits=True) self._scheduler = graph_session.scheduler_session self._build_graph, self._address_mapper = graph_session.create_build_graph( Specs(address_specs=AddressSpecs([]), filesystem_specs=FilesystemSpecs([])), diff --git a/src/rust/engine/src/externs/interface.rs b/src/rust/engine/src/externs/interface.rs index 0b28cc62c22..3f511dff4e7 100644 --- a/src/rust/engine/src/externs/interface.rs +++ b/src/rust/engine/src/externs/interface.rs @@ -498,14 +498,12 @@ py_class!(class PySession |py| { data session: Session; def __new__(_cls, scheduler_ptr: PyScheduler, - should_record_zipkin_spans: bool, should_render_ui: bool, build_id: String, should_report_workunits: bool ) -> CPyResult { Self::create_instance(py, Session::new( scheduler_ptr.scheduler(py), - should_record_zipkin_spans, should_render_ui, build_id, should_report_workunits, @@ -927,17 +925,11 @@ fn scheduler_metrics( ) -> CPyResult { with_scheduler(py, scheduler_ptr, |scheduler| { with_session(py, session_ptr, |session| { - let mut values = scheduler + let values = scheduler .metrics(session) .into_iter() .map(|(metric, value)| (externs::store_utf8(metric), externs::store_i64(value))) .collect::>(); - if session.should_record_zipkin_spans() { - let workunits = session.workunit_store().get_workunits(); - let mut iter = workunits.iter(); - let value = workunits_to_py_tuple_value(&mut iter, &scheduler.core)?; - values.push((externs::store_utf8("engine_workunits"), value)); - }; externs::store_dict(values).map(|d| d.consume_into_py_object(py)) }) }) diff --git a/src/rust/engine/src/scheduler.rs b/src/rust/engine/src/scheduler.rs index 46d4e94d5db..ef056ad0c05 100644 --- a/src/rust/engine/src/scheduler.rs +++ b/src/rust/engine/src/scheduler.rs @@ -58,8 +58,6 @@ struct InnerSession { // If enabled, the display that will render the progress of the V2 engine. This is only // Some(_) if the --dynamic-ui option is enabled. display: Option>, - // If enabled, Zipkin spans for v2 engine will be collected. - should_record_zipkin_spans: bool, // A place to store info about workunits in rust part workunit_store: WorkunitStore, // The unique id for this Session: used for metrics gathering purposes. @@ -81,7 +79,6 @@ pub struct Session(Arc); impl Session { pub fn new( scheduler: &Scheduler, - should_record_zipkin_spans: bool, should_render_ui: bool, build_id: String, should_report_workunits: bool, @@ -97,7 +94,6 @@ impl Session { preceding_graph_size: scheduler.core.graph.len(), roots: Mutex::new(HashMap::new()), display, - should_record_zipkin_spans, workunit_store, build_id, run_id: Mutex::new(Uuid::new_v4()), @@ -131,10 +127,6 @@ impl Session { self.0.preceding_graph_size } - pub fn should_record_zipkin_spans(&self) -> bool { - self.0.should_record_zipkin_spans - } - pub fn should_report_workunits(&self) -> bool { self.0.should_report_workunits } @@ -185,10 +177,6 @@ impl Session { } } - pub fn should_handle_workunits(&self) -> bool { - self.should_report_workunits() || self.should_record_zipkin_spans() - } - fn maybe_display_initialize(&self, executor: &Executor, sender: &mpsc::Sender) { if let Some(display) = &self.0.display { let mut display = display.lock(); diff --git a/src/rust/engine/workunit_store/src/lib.rs b/src/rust/engine/workunit_store/src/lib.rs index ce13efe96b5..c14d34ab488 100644 --- a/src/rust/engine/workunit_store/src/lib.rs +++ b/src/rust/engine/workunit_store/src/lib.rs @@ -201,21 +201,6 @@ impl WorkunitStore { })) } - pub fn get_workunits(&self) -> Vec { - let mut inner_guard = (*self.inner).lock(); - let inner_store: &mut WorkUnitInnerStore = &mut *inner_guard; - let workunit_records = &inner_store.workunit_records; - inner_store - .completed_ids - .iter() - .flat_map(|id| workunit_records.get(id)) - .flat_map(|workunit| match workunit.state { - WorkunitState::Started { .. } => None, - WorkunitState::Completed { .. } => Some(workunit.clone()), - }) - .collect() - } - /// /// Find the longest running leaf workunits, and render their first visible parents. /// diff --git a/tests/python/pants_test/goal/test_run_tracker_integration.py b/tests/python/pants_test/goal/test_run_tracker_integration.py index 71b02022122..58d81a4771f 100644 --- a/tests/python/pants_test/goal/test_run_tracker_integration.py +++ b/tests/python/pants_test/goal/test_run_tracker_integration.py @@ -22,7 +22,6 @@ def test_stats_local_json_file_v2(self): "test", f"--run-tracker-stats-local-json-file={tmpfile}", "--run-tracker-stats-version=2", - "--reporting-zipkin-trace-v2", "testprojects/src/java/org/pantsbuild/testproject/unicode/main", ] ) diff --git a/tests/python/pants_test/reporting/test_reporting_integration.py b/tests/python/pants_test/reporting/test_reporting_integration.py index fe970e4c9de..d7dd8ccaa55 100644 --- a/tests/python/pants_test/reporting/test_reporting_integration.py +++ b/tests/python/pants_test/reporting/test_reporting_integration.py @@ -9,7 +9,6 @@ import psutil from pants.testutil.pants_run_integration_test import PantsRunIntegrationTest -from pants.util.collections import assert_single_element from pants.util.contextutil import http_server @@ -51,68 +50,6 @@ def test_zipkin_reporter_with_zero_sample_rate(self): num_of_traces = len(ZipkinHandler.traces) self.assertEqual(num_of_traces, 0) - def test_zipkin_reporter_for_v2_engine(self): - ZipkinHandler = zipkin_handler() - with http_server(ZipkinHandler) as port: - endpoint = f"http://localhost:{port}" - command = [ - "-ldebug", - f"--reporting-zipkin-endpoint={endpoint}", - "--reporting-zipkin-trace-v2", - "list", - "examples/src/python/example/hello/greet", - ] - - pants_run = self.run_pants(command) - self.assert_success(pants_run) - - child_processes = self.find_child_processes_that_send_spans(pants_run.stderr_data) - self.assertTrue(child_processes) - - self.wait_spans_to_be_sent(child_processes) - - trace = assert_single_element(ZipkinHandler.traces.values()) - - v2_span_name_part = "snapshot" - self.assertTrue( - any(v2_span_name_part in span["name"] for span in trace), - "There is no span that contains '{}' in it's name. The trace:{}".format( - v2_span_name_part, trace - ), - ) - - def test_zipkin_reports_for_pure_v2_goals(self): - ZipkinHandler = zipkin_handler() - with http_server(ZipkinHandler) as port: - endpoint = f"http://localhost:{port}" - command = [ - "-ldebug", - "--no-v1", - "--v2", - f"--reporting-zipkin-endpoint={endpoint}", - "--reporting-zipkin-trace-v2", - "list", - "3rdparty:", - ] - - pants_run = self.run_pants(command) - self.assert_success(pants_run) - - child_processes = self.find_child_processes_that_send_spans(pants_run.stderr_data) - self.assertTrue(child_processes) - - self.wait_spans_to_be_sent(child_processes) - - trace = assert_single_element(ZipkinHandler.traces.values()) - - v2_span_name_part = "snapshot" - self.assertTrue( - any(v2_span_name_part in span["name"] for span in trace), - "There is no span that contains '{}' in it's name. The trace:{}".format( - v2_span_name_part, trace - ), - ) - @staticmethod def find_spans_by_name_and_service_name(trace, name, service_name): return [