Skip to content

Commit 3dcfd08

Browse files
committed
Python: Add telemetry for parser usage
Adds statistics on how many files were extracted using the old parser and using the tree-sitter parser. Because parsing is done in parallel across many workers, I opted not to consolidate these statistics for the entire run. Instead, we emit the statistics for each worker and then need to aggregate themselves after the telemetry has been ingested. (In practice the number of workers is ~16 at most, so is unlikely to be an issue.) In terms of implementation, I opted to simply extend the existing `DiagnosticsWriter` object (instantiatied once per worker) with methods for counting the number of parsed files, and then thread this object through to `modules.py` where the magic happens. Finally, this also required instantiating such an object in cases where we call directly into the extractor for debugging purposes (e.g. dumping the AST or CFG). Note that in these cases we do not actually print any diagnostics, so it's harmless to create these objects. As for tests, we add a new separate CLI integration test that checks the behaviour against a database that contains two files -- one that can be parsed with the old parser and one that requires the new one. The existing diagnostics test is modified slightly so that it ignores these statistics (as we cannot guarantee their exact form due to worker nondeterminism).
1 parent 8116060 commit 3dcfd08

14 files changed

Lines changed: 205 additions & 10 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
x = 1
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
match 1:
2+
case 1:
3+
pass
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
#!/bin/bash
2+
3+
set -Eeuo pipefail # see https://vaneyckt.io/posts/safer_bash_scripts_with_set_euxo_pipefail/
4+
5+
set -x
6+
7+
CODEQL=${CODEQL:-codeql}
8+
9+
SCRIPTDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
10+
cd "$SCRIPTDIR"
11+
12+
rm -rf db
13+
14+
$CODEQL database create db --language python --source-root repo_dir/
15+
python3 test_parser_telemetry.py db
16+
17+
rm -rf db
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import glob
2+
import json
3+
import os
4+
import sys
5+
6+
7+
database = sys.argv[1]
8+
diagnostics = []
9+
diagnostic_dir = os.path.join(database, "diagnostic", "extractors", "python")
10+
for path in glob.glob(os.path.join(diagnostic_dir, "*.jsonl")):
11+
with open(path) as diagnostic_file:
12+
diagnostics.extend(json.loads(line) for line in diagnostic_file)
13+
parser_statistics = [
14+
diagnostic
15+
for diagnostic in diagnostics
16+
if diagnostic["source"]["id"] == "py/extractor/parser-statistics"
17+
]
18+
actual = (
19+
sum(diagnostic["attributes"]["old_parser_file_count"] for diagnostic in parser_statistics),
20+
sum(
21+
diagnostic["attributes"]["tree_sitter_parser_file_count"]
22+
for diagnostic in parser_statistics
23+
),
24+
)
25+
assert actual == (1, 1), actual

‎python/extractor/cli-integration-test/writing-diagnostics/test_diagnostics_output.py‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,11 @@
44
import diagnostics_test_utils
55

66
test_db = "db"
7-
diagnostics_test_utils.check_diagnostics(".", test_db, skip_attributes=True)
7+
diagnostics_test_utils.check_diagnostics(
8+
".",
9+
test_db,
10+
skip_attributes=True,
11+
replacements={
12+
r'"py/extractor/parser-statistics"': '"cli/py/extractor/parser-statistics"'
13+
},
14+
)

‎python/extractor/semmle/extractors/module_printer.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ class ModulePrinter(object):
66

77
name = "module printer"
88

9-
def __init__(self, options, trap_folder, src_archive, renamer, logger):
9+
def __init__(self, options, trap_folder, src_archive, renamer, logger, diagnostics_writer):
1010
self.logger = logger
11-
self.py_extractor = PythonExtractor(options, trap_folder, src_archive, logger)
11+
self.py_extractor = PythonExtractor(options, trap_folder, src_archive, logger, diagnostics_writer)
1212

1313
def process(self, unit):
1414
imports = ()

‎python/extractor/semmle/extractors/py_extractor.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ def __init__(self, options, trap_folder, src_archive, logger: Logger, diagnostic
1616
self.module_extractor = extractor.Extractor.from_options(options, trap_folder, src_archive, logger, diagnostics_writer)
1717
self.finder = finder.Finder.from_options_and_env(options, logger)
1818
self.importer = imports.importer_from_options(options, self.finder, logger)
19+
self.diagnostics_writer = diagnostics_writer
1920

2021
def _get_module_and_imports(self, unit):
2122
if not isinstance(unit, util.FileExtractable):
@@ -24,7 +25,7 @@ def _get_module_and_imports(self, unit):
2425
module = self.finder.from_extractable(unit)
2526
if module is None:
2627
return None, ()
27-
py_module = module.load(self.logger)
28+
py_module = module.load(self.logger, self.diagnostics_writer)
2829
if py_module is None:
2930
return None, ()
3031
imports = set(mod.get_extractable() for mod in self.importer.get_imports(module, py_module))

‎python/extractor/semmle/logging.py‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,14 @@ def extractor_telemetry_message():
367367
.telemetry()
368368
)
369369

370+
def parser_statistics_telemetry_message(old_parser_file_count, tree_sitter_parser_file_count):
371+
return (DiagnosticMessage(Source("py/extractor/parser-statistics", "Python parser statistics"), Severity.NOTE)
372+
.markdown("Internal parser telemetry for the Python extractor.\n\nNo action needed.")
373+
.attribute("old_parser_file_count", old_parser_file_count)
374+
.attribute("tree_sitter_parser_file_count", tree_sitter_parser_file_count)
375+
.telemetry()
376+
)
377+
370378
def get_stack_trace_lines():
371379
"""Creates a stack trace for inclusion into the `attributes` part of a diagnostic message.
372380
Limits the size of the stack trace to 5000 characters, so as to not make the SARIF file overly big.

‎python/extractor/semmle/python/finder.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@ def all_sub_modules(self):
6565
def get_extractable(self):
6666
return FileExtractable(self.path)
6767

68-
def load(self, logger=None):
69-
return PythonSourceModule(self.name, self.path, logger=logger)
68+
def load(self, logger, diagnostics_writer):
69+
return PythonSourceModule(self.name, self.path, logger=logger, diagnostics_writer=diagnostics_writer)
7070

7171
def __str__(self):
7272
return "Python module at %s" % self.path

‎python/extractor/semmle/python/modules.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ class PythonSourceModule(object):
1818

1919
kind = None
2020

21-
def __init__(self, name, path, logger, bytes_source = None):
21+
def __init__(self, name, path, logger, diagnostics_writer, bytes_source = None):
2222
assert isinstance(path, str), path
2323
self.name = name # May be None
2424
self.path = path
@@ -34,6 +34,7 @@ def __init__(self, name, path, logger, bytes_source = None):
3434
self._line_types = None
3535
self._comments = None
3636
self._tokens = None
37+
self.diagnostics_writer = diagnostics_writer
3738
self.logger = logger
3839
with timers["decode"]:
3940
self.encoding, self.bytes_source = semmle.python.parser.tokenizer.encoding_from_source(bytes_source)
@@ -113,6 +114,7 @@ def old_py_ast(self):
113114
self.logger.debug("Trying old parser on %s", self.path)
114115
self._py_ast = semmle.python.parser.parse(self.tokens, self.logger)
115116
self.logger.debug("Old parser successful on %s", self.path)
117+
self.diagnostics_writer.record_old_parser()
116118
else:
117119
self.logger.debug("Found (during old_py_ast) parse tree for %s in cache", self.path)
118120
return self._py_ast
@@ -147,6 +149,7 @@ def py_ast(self):
147149
self.logger.debug("Trying tsg-python on %s", self.path)
148150
self._py_ast = semmle.python.parser.tsg_parser.parse(self.path, self.logger)
149151
self.logger.debug("tsg-python successful on %s", self.path)
152+
self.diagnostics_writer.record_tree_sitter_parser()
150153
else:
151154
self.logger.debug("Found (during py_ast) parse tree for %s in cache", self.path)
152155
return self._py_ast

0 commit comments

Comments
 (0)