Add checks if values of flags zipkin-trace-id and zipkin-parent-id are valid

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,10 @@ 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')

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)
+    )