Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 27 additions & 8 deletions src/python/pants/reporting/reporting.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.')

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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')
84 changes: 84 additions & 0 deletions tests/python/pants_test/reporting/test_reporting.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
)