Skip to content

Commit cfe3a03

Browse files
pmaipmai
committed
Refactor issue reporting, add location reporting
This currently uses file locations with the row indicating the message index, as the framework does not yet have support for more fitting location reporting for binary and time-based formats. Signed-off-by: Pierre R. Mai <pmai@pmsf.de>
1 parent 68d7e14 commit cfe3a03

File tree

1 file changed

+154
-96
lines changed

1 file changed

+154
-96
lines changed

‎qc_ositrace/checks/osirules/osirules_checker.py‎

Lines changed: 154 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,33 @@ def record_message_ids(
9898
record_message_ids(item, id_message_map)
9999

100100

101+
def register_issue(
102+
result: Result,
103+
message: google.protobuf.message.Message,
104+
index: int,
105+
time: float | None,
106+
rule_uid: str,
107+
level: IssueSeverity,
108+
description: str,
109+
) -> None:
110+
time_str = f"{time}" if time is not None else "unknown"
111+
issue_id = result.register_issue(
112+
checker_bundle_name=constants.BUNDLE_NAME,
113+
checker_id=osirules_constants.CHECKER_ID,
114+
description=f"Message {index} at {time_str}: {description}",
115+
level=level,
116+
rule_uid=rule_uid,
117+
)
118+
result.add_file_location(
119+
checker_bundle_name=constants.BUNDLE_NAME,
120+
checker_id=osirules_constants.CHECKER_ID,
121+
issue_id=issue_id,
122+
row=index,
123+
column=0,
124+
description=f"Message {index} at {time_str}: {description}",
125+
)
126+
127+
101128
def evaluate_rule_condition(
102129
message: google.protobuf.message.Message, field_name: str, rule: dict
103130
) -> bool:
@@ -134,7 +161,7 @@ def check_message_against_rules(
134161
rule_map: dict,
135162
id_message_map: dict,
136163
index: int,
137-
time: float,
164+
time: float | None,
138165
result: Result,
139166
) -> None:
140167
field_rules = rule_map.get(message.DESCRIPTOR.full_name, {})
@@ -144,12 +171,14 @@ def check_message_against_rules(
144171
has_field = message.HasField(field_name)
145172
for rule_uid, rule in rules:
146173
if "is_set" in rule and not has_field:
147-
result.register_issue(
148-
checker_bundle_name=constants.BUNDLE_NAME,
149-
checker_id=osirules_constants.CHECKER_ID,
150-
description=f"Message {index} at {time}: Field '{field_name}' is not set in message '{message.DESCRIPTOR.full_name}'.",
151-
level=IssueSeverity.ERROR,
152-
rule_uid=rule_uid,
174+
register_issue(
175+
result,
176+
message,
177+
index,
178+
time,
179+
rule_uid,
180+
IssueSeverity.ERROR,
181+
description=f"Field '{field_name}' is not set in message '{message.DESCRIPTOR.full_name}' but should be set.",
153182
)
154183
if (
155184
"check_if" in rule
@@ -168,119 +197,143 @@ def check_message_against_rules(
168197
)
169198
and not has_field
170199
):
171-
result.register_issue(
172-
checker_bundle_name=constants.BUNDLE_NAME,
173-
checker_id=osirules_constants.CHECKER_ID,
174-
description=f"Message {index} at {time}: Field '{field_name}' is not set in message '{message.DESCRIPTOR.full_name}'.",
175-
level=IssueSeverity.ERROR,
176-
rule_uid=rule_uid,
200+
register_issue(
201+
result,
202+
message,
203+
index,
204+
time,
205+
rule_uid,
206+
IssueSeverity.ERROR,
207+
description=f"Field '{field_name}' is not set in message '{message.DESCRIPTOR.full_name}' but should be set according to 'check_if' rule.",
177208
)
178209

179210
# Process other rules for each set field
180211
for field, value in message.ListFields():
181212
for rule_uid, rule in field_rules.get(field.name, []):
182213
if "is_greater_than" in rule and not value > rule["is_greater_than"]:
183-
result.register_issue(
184-
checker_bundle_name=constants.BUNDLE_NAME,
185-
checker_id=osirules_constants.CHECKER_ID,
186-
description=f"Message {index} at {time}: Field '{field.name}' value {value} in message '{message.DESCRIPTOR.full_name}' is not greater than {rule['is_greater_than']}.",
187-
level=IssueSeverity.ERROR,
188-
rule_uid=rule_uid,
214+
register_issue(
215+
result,
216+
message,
217+
index,
218+
time,
219+
rule_uid,
220+
IssueSeverity.ERROR,
221+
description=f"Field '{field.name}' value {value} in message '{message.DESCRIPTOR.full_name}' is not greater than {rule['is_greater_than']}.",
189222
)
190223
if (
191224
"is_greater_than_or_equal_to" in rule
192225
and not value >= rule["is_greater_than_or_equal_to"]
193226
):
194-
result.register_issue(
195-
checker_bundle_name=constants.BUNDLE_NAME,
196-
checker_id=osirules_constants.CHECKER_ID,
197-
description=f"Message {index} at {time}: Field '{field.name}' value {value} in message '{message.DESCRIPTOR.full_name}' is not greater or equal to {rule['is_greater_than_or_equal_to']}.",
198-
level=IssueSeverity.ERROR,
199-
rule_uid=rule_uid,
227+
register_issue(
228+
result,
229+
message,
230+
index,
231+
time,
232+
rule_uid,
233+
IssueSeverity.ERROR,
234+
description=f"Field '{field.name}' value {value} in message '{message.DESCRIPTOR.full_name}' is not greater or equal to {rule['is_greater_than_or_equal_to']}.",
200235
)
201236
if "is_less_than" in rule and not value < rule["is_less_than"]:
202-
result.register_issue(
203-
checker_bundle_name=constants.BUNDLE_NAME,
204-
checker_id=osirules_constants.CHECKER_ID,
205-
description=f"Message {index} at {time}: Field '{field.name}' value {value} in message '{message.DESCRIPTOR.full_name}' is not less than {rule['is_less_than']}.",
206-
level=IssueSeverity.ERROR,
207-
rule_uid=rule_uid,
237+
register_issue(
238+
result,
239+
message,
240+
index,
241+
time,
242+
rule_uid,
243+
IssueSeverity.ERROR,
244+
description=f"Field '{field.name}' value {value} in message '{message.DESCRIPTOR.full_name}' is not less than {rule['is_less_than']}.",
208245
)
209246
if (
210247
"is_less_than_or_equal_to" in rule
211248
and not value <= rule["is_less_than_or_equal_to"]
212249
):
213-
result.register_issue(
214-
checker_bundle_name=constants.BUNDLE_NAME,
215-
checker_id=osirules_constants.CHECKER_ID,
216-
description=f"Message {index} at {time}: Field '{field.name}' value {value} in message '{message.DESCRIPTOR.full_name}' is not less or equal to {rule['is_less_than_or_equal_to']}.",
217-
level=IssueSeverity.ERROR,
218-
rule_uid=rule_uid,
250+
register_issue(
251+
result,
252+
message,
253+
index,
254+
time,
255+
rule_uid,
256+
IssueSeverity.ERROR,
257+
description=f"Field '{field.name}' value {value} in message '{message.DESCRIPTOR.full_name}' is not less or equal to {rule['is_less_than_or_equal_to']}.",
219258
)
220259
if "is_equal_to" in rule and not value == rule["is_equal_to"]:
221-
result.register_issue(
222-
checker_bundle_name=constants.BUNDLE_NAME,
223-
checker_id=osirules_constants.CHECKER_ID,
224-
description=f"Message {index} at {time}: Field '{field.name}' value {value} in message '{message.DESCRIPTOR.full_name}' is not equal to {rule['is_equal_to']}.",
225-
level=IssueSeverity.ERROR,
226-
rule_uid=rule_uid,
260+
register_issue(
261+
result,
262+
message,
263+
index,
264+
time,
265+
rule_uid,
266+
IssueSeverity.ERROR,
267+
description=f"Field '{field.name}' value {value} in message '{message.DESCRIPTOR.full_name}' is not equal to {rule['is_equal_to']}.",
227268
)
228269
if "is_different_to" in rule and not value != rule["is_different_to"]:
229-
result.register_issue(
230-
checker_bundle_name=constants.BUNDLE_NAME,
231-
checker_id=osirules_constants.CHECKER_ID,
232-
description=f"Message {index} at {time}: Field '{field.name}' value {value} in message '{message.DESCRIPTOR.full_name}' is not different from {rule['is_different_to']}.",
233-
level=IssueSeverity.ERROR,
234-
rule_uid=rule_uid,
270+
register_issue(
271+
result,
272+
message,
273+
index,
274+
time,
275+
rule_uid,
276+
IssueSeverity.ERROR,
277+
description=f"Field '{field.name}' value {value} in message '{message.DESCRIPTOR.full_name}' is not different from {rule['is_different_to']}.",
235278
)
236279
if "is_iso_country_code" in rule:
237280
if value > 999 or value < 0:
238-
result.register_issue(
239-
checker_bundle_name=constants.BUNDLE_NAME,
240-
checker_id=osirules_constants.CHECKER_ID,
241-
description=f"Message {index} at {time}: Field '{field.name}' value {value} in message '{message.DESCRIPTOR.full_name}' is not a valid numeric ISO country code (must be between 000 and 999).",
242-
level=IssueSeverity.ERROR,
243-
rule_uid=rule_uid,
281+
register_issue(
282+
result,
283+
message,
284+
index,
285+
time,
286+
rule_uid,
287+
IssueSeverity.ERROR,
288+
description=f"Field '{field.name}' value {value} in message '{message.DESCRIPTOR.full_name}' is not a valid numeric ISO country code (must be between 000 and 999).",
244289
)
245290
if iso3166.countries.get(value, None) is None:
246-
result.register_issue(
247-
checker_bundle_name=constants.BUNDLE_NAME,
248-
checker_id=osirules_constants.CHECKER_ID,
249-
description=f"Message {index} at {time}: Field '{field.name}' value {value} in message '{message.DESCRIPTOR.full_name}' is not a valid numeric ISO country code (not found in ISO 3166).",
250-
level=IssueSeverity.WARNING,
251-
rule_uid=rule_uid,
291+
register_issue(
292+
result,
293+
message,
294+
index,
295+
time,
296+
rule_uid,
297+
IssueSeverity.ERROR,
298+
description=f"Field '{field.name}' value {value} in message '{message.DESCRIPTOR.full_name}' is not a valid numeric ISO country code (not found in ISO 3166).",
252299
)
253300
if "is_globally_unique" in rule:
254301
if value.value in id_message_map:
255302
existing_message = id_message_map[value.value]
256303
if existing_message != message:
257-
result.register_issue(
258-
checker_bundle_name=constants.BUNDLE_NAME,
259-
checker_id=osirules_constants.CHECKER_ID,
260-
description=f"Message {index} at {time}: Field '{field.name}' value {value.value} in message '{message.DESCRIPTOR.full_name}' is not globally unique, already used by different message '{existing_message.DESCRIPTOR.full_name}'.",
261-
level=IssueSeverity.ERROR,
262-
rule_uid=rule_uid,
304+
register_issue(
305+
result,
306+
message,
307+
index,
308+
time,
309+
rule_uid,
310+
IssueSeverity.ERROR,
311+
description=f"Field '{field.name}' value {value.value} in message '{message.DESCRIPTOR.full_name}' is not globally unique, already used by different message '{existing_message.DESCRIPTOR.full_name}'.",
263312
)
264313
if "refers_to" in rule:
265314
referred_message = id_message_map.get(value.value, None)
266315
if referred_message is None:
267-
result.register_issue(
268-
checker_bundle_name=constants.BUNDLE_NAME,
269-
checker_id=osirules_constants.CHECKER_ID,
270-
description=f"Message {index} at {time}: Field '{field.name}' value {value.value} in message '{message.DESCRIPTOR.full_name}' does not refer to any existing message.",
271-
level=IssueSeverity.ERROR,
272-
rule_uid=rule_uid,
316+
register_issue(
317+
result,
318+
message,
319+
index,
320+
time,
321+
rule_uid,
322+
IssueSeverity.ERROR,
323+
description=f"Field '{field.name}' value {value.value} in message '{message.DESCRIPTOR.full_name}' does not refer to any existing message.",
273324
)
274325
else:
275326
# Check if referred message matches the expected type
276327
expected_type = f"""osi3.{rule['refers_to'].strip("'")}"""
277328
if referred_message.DESCRIPTOR.full_name != expected_type:
278-
result.register_issue(
279-
checker_bundle_name=constants.BUNDLE_NAME,
280-
checker_id=osirules_constants.CHECKER_ID,
281-
description=f"Message {index} at {time}: Field '{field.name}' value {value.value} in message '{message.DESCRIPTOR.full_name}' refers to message '{referred_message.DESCRIPTOR.full_name}', which does not match the expected type '{expected_type}'.",
282-
level=IssueSeverity.ERROR,
283-
rule_uid=rule_uid,
329+
register_issue(
330+
result,
331+
message,
332+
index,
333+
time,
334+
rule_uid,
335+
IssueSeverity.ERROR,
336+
description=f"Field '{field.name}' value {value.value} in message '{message.DESCRIPTOR.full_name}' refers to message '{referred_message.DESCRIPTOR.full_name}', which does not match the expected type '{expected_type}'.",
284337
)
285338

286339
# Recursively check nested messages
@@ -385,13 +438,20 @@ def run_checks(config: Configuration, result: Result) -> None:
385438
logging.info("Executing osirules automatic checks")
386439

387440
for index, message in enumerate(trace):
441+
time = (
442+
message.timestamp.seconds + message.timestamp.nanos * 1e-9
443+
if hasattr(message, "timestamp") and message.HasField("timestamp")
444+
else None
445+
)
388446
if not message.HasField("version"):
389-
issue_id = result.register_issue(
390-
checker_bundle_name=constants.BUNDLE_NAME,
391-
checker_id=osirules_constants.CHECKER_ID,
392-
description=f"Message {index}: Version field is not set in top-level message.",
393-
level=IssueSeverity.ERROR,
394-
rule_uid=version_rule_uid,
447+
register_issue(
448+
result,
449+
message,
450+
index,
451+
time,
452+
version_rule_uid,
453+
IssueSeverity.ERROR,
454+
description=f"Version field is not set in top-level message.",
395455
)
396456
elif (
397457
expected_version is not None
@@ -402,20 +462,18 @@ def run_checks(config: Configuration, result: Result) -> None:
402462
)
403463
!= expected_version
404464
):
405-
issue_id = result.register_issue(
406-
checker_bundle_name=constants.BUNDLE_NAME,
407-
checker_id=osirules_constants.CHECKER_ID,
408-
description=f"Message {index}: Version field value {message.version.version_major}.{message.version.version_minor}.{message.version.version_patch} is not the expected version {'.'.join([str(s) for s in expected_version])}.",
409-
level=IssueSeverity.ERROR,
410-
rule_uid=exp_version_rule_uid,
465+
register_issue(
466+
result,
467+
message,
468+
index,
469+
time,
470+
exp_version_rule_uid,
471+
IssueSeverity.ERROR,
472+
description=f"Version field value {message.version.version_major}.{message.version.version_minor}.{message.version.version_patch} is not the expected version {'.'.join([str(s) for s in expected_version])}.",
411473
)
474+
412475
id_message_map = {}
413476
record_message_ids(message, id_message_map)
414-
time = (
415-
message.timestamp.seconds + message.timestamp.nanos * 1e-9
416-
if hasattr(message, "timestamp") and message.HasField("timestamp")
417-
else 0.0
418-
)
419477
check_message_against_rules(
420478
message, rule_uid_map, id_message_map, index, time, result
421479
)

0 commit comments

Comments
 (0)