diff --git a/src/python/pants/reporting/reporting.py b/src/python/pants/reporting/reporting.py index 76f07e84e87..245f4eef4cc 100644 --- a/src/python/pants/reporting/reporting.py +++ b/src/python/pants/reporting/reporting.py @@ -51,15 +51,15 @@ def register_options(cls, register): help='The full HTTP URL of a zipkin server to which traces should be posted. ' 'No traces will be made if this is not set.') register('--zipkin-trace-id', advanced=True, default=None, - help='The overall 64 or 128-bit ID of the trace. ' - 'Set if Pants trace should be a part of larger trace ' - 'for systems that invoke Pants. If zipkin-trace-id ' - 'and zipkin-parent-id are not set, a trace_id value is randomly generated for a ' - 'Zipkin trace') + help='The overall 64 or 128-bit ID of the trace (the format is 16-character or ' + '32-character hex string). Set if the Pants trace should be a part of a larger ' + 'trace for systems that invoke Pants. If flags zipkin-trace-id and ' + 'zipkin-parent-id are not set, a trace_id value is randomly generated ' + 'for a Zipkin trace.') register('--zipkin-parent-id', advanced=True, default=None, - help='The 64-bit ID for a parent span that invokes Pants. ' - 'zipkin-trace-id and zipkin-parent-id must both either be set or not set ' - 'when run Pants command') + help='The 64-bit ID for a parent span that invokes Pants (the format is 16-character ' + 'hex string). Flags zipkin-trace-id and zipkin-parent-id must both either be set ' + 'or not set when running a Pants command.') register('--zipkin-sample-rate', advanced=True, default=100.0, help='Rate at which to sample Zipkin traces. Value 0.0 - 100.0.') @@ -112,6 +112,16 @@ def initialize(self, run_tracker, all_options, start_time=None): raise ValueError( "Flags zipkin-trace-id and zipkin-parent-id must both either be set or not set." ) + if trace_id and (len(trace_id) != 16 and len(trace_id) != 32 or not is_hex_string(trace_id)): + raise ValueError( + "Value of the flag zipkin-trace-id must be a 16-character or 32-character hex string. " + + "Got {}.".format(trace_id) + ) + if parent_id and (len(parent_id) != 16 or not is_hex_string(parent_id)): + raise ValueError( + "Value of the flag zipkin-parent-id must be a 16-character hex string. " + + "Got {}.".format(parent_id) + ) if zipkin_endpoint is not None: zipkin_reporter_settings = ZipkinReporter.Settings(log_level=Report.INFO) @@ -195,3 +205,12 @@ def update_reporting(self, global_options, is_quiet, run_tracker): invalidation_report.set_filename(outfile) return invalidation_report + + +def is_hex_string(id_value): + return all(is_hex_ch(ch) for ch in id_value) + + +def is_hex_ch(ch): + num = ord(ch) + return ord('0') <= num <= ord('9') or ord('a') <= num <= ord('f') or ord('A') <= num <= ord('F') diff --git a/tests/python/pants_test/reporting/test_reporting.py b/tests/python/pants_test/reporting/test_reporting.py index 3df1144481c..b221ed0a97a 100644 --- a/tests/python/pants_test/reporting/test_reporting.py +++ b/tests/python/pants_test/reporting/test_reporting.py @@ -94,3 +94,87 @@ def test_raise_if_no_parent_id_and_zipkin_endpoint_set(self): "Flags zipkin-trace-id and zipkin-parent-id must both either be set or not set." in str(result.exception) ) + + def test_raise_if_parent_id_is_of_wrong_len_format(self): + parent_id = 'ff' + options = {'reporting': { + 'zipkin_trace_id': self.trace_id, + 'zipkin_parent_id': parent_id, + 'zipkin_endpoint': self.zipkin_endpoint + }} + context = self.context(for_subsystems=[RunTracker, Reporting], options=options) + + run_tracker = RunTracker.global_instance() + reporting = Reporting.global_instance() + + with self.assertRaises(ValueError) as result: + reporting.initialize(run_tracker, context.options) + + self.assertTrue( + "Value of the flag zipkin-parent-id must be a 16-character hex string. " + + "Got {}.".format(parent_id) + in str(result.exception) + ) + + def test_raise_if_trace_id_is_of_wrong_len_format(self): + trace_id = 'aa' + options = {'reporting': { + 'zipkin_trace_id': trace_id, + 'zipkin_parent_id': self.parent_id, + 'zipkin_endpoint': self.zipkin_endpoint + }} + context = self.context(for_subsystems=[RunTracker, Reporting], options=options) + + run_tracker = RunTracker.global_instance() + reporting = Reporting.global_instance() + + with self.assertRaises(ValueError) as result: + reporting.initialize(run_tracker, context.options) + + self.assertTrue( + "Value of the flag zipkin-trace-id must be a 16-character or 32-character hex string. " + + "Got {}.".format(trace_id) + in str(result.exception) + ) + + def test_raise_if_parent_id_is_of_wrong_ch_format(self): + parent_id = 'gggggggggggggggg' + options = {'reporting': { + 'zipkin_trace_id': self.trace_id, + 'zipkin_parent_id': parent_id, + 'zipkin_endpoint': self.zipkin_endpoint + }} + context = self.context(for_subsystems=[RunTracker, Reporting], options=options) + + run_tracker = RunTracker.global_instance() + reporting = Reporting.global_instance() + + with self.assertRaises(ValueError) as result: + reporting.initialize(run_tracker, context.options) + + self.assertTrue( + "Value of the flag zipkin-parent-id must be a 16-character hex string. " + + "Got {}.".format(parent_id) + in str(result.exception) + ) + + def test_raise_if_trace_id_is_of_wrong_ch_format(self): + trace_id = 'gggggggggggggggg' + options = {'reporting': { + 'zipkin_trace_id': trace_id, + 'zipkin_parent_id': self.parent_id, + 'zipkin_endpoint': self.zipkin_endpoint + }} + context = self.context(for_subsystems=[RunTracker, Reporting], options=options) + + run_tracker = RunTracker.global_instance() + reporting = Reporting.global_instance() + + with self.assertRaises(ValueError) as result: + reporting.initialize(run_tracker, context.options) + + self.assertTrue( + "Value of the flag zipkin-trace-id must be a 16-character or 32-character hex string. " + + "Got {}.".format(trace_id) + in str(result.exception) + )