Skip to content

feat: Improve codejail darklaunch logging #36671

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.

Sign up for GitHub

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

Merged
timmc-edx merged 3 commits into from
May 6, 2025
Merged

feat: Improve codejail darklaunch logging #36671

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4098378
feat: Improve codejail darklaunch logging
May 6, 2025
57da45e
fixup! Set course and slug in tests; use repr in logging them
May 6, 2025
aeab88d
fixup! Add docstring for report_arm
May 6, 2025
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
30 changes: 19 additions & 11 deletions xmodule/capa/safe_exec/safe_exec.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ def safe_exec(
local_exc_unexpected = None if isinstance(exception, SafeExecException) else exception

report_darklaunch_results(
slug=slug,
limit_overrides_context=limit_overrides_context, slug=slug,
globals_local=globals_dict, emsg_local=emsg, unexpected_exc_local=local_exc_unexpected,
globals_remote=darklaunch_globals, emsg_remote=remote_emsg, unexpected_exc_remote=remote_exception,
)
Expand Down Expand Up @@ -347,14 +347,14 @@ def normalize_error_message(emsg):


def report_darklaunch_results(
*, slug,
*, limit_overrides_context, slug,
globals_local, emsg_local, unexpected_exc_local,
globals_remote, emsg_remote, unexpected_exc_remote,
):
"""Send telemetry for results of darklaunch."""
can_compare_output = True

def report_arm(arm, globals_dict, emsg, unexpected_exception):
def report_arm(arm, emsg, unexpected_exception):
Comment thread
robrap marked this conversation as resolved.
nonlocal can_compare_output
if unexpected_exception:
# .. custom_attribute_name: codejail.darklaunch.status.{local,remote}
Expand All @@ -373,25 +373,33 @@ def report_arm(arm, globals_dict, emsg, unexpected_exception):
set_custom_attribute(f'codejail.darklaunch.status.{arm}', 'ok' if emsg is None else 'safe_error')
set_custom_attribute(f'codejail.darklaunch.exception.{arm}', None)

# Logs include full globals and emsg
log.info(
f"Codejail darklaunch {arm} results for slug={slug}: globals={globals_dict!r}, "
f"emsg={emsg!r}, exception={unexpected_exception!r}"
)

report_arm('local', globals_local, emsg_local, unexpected_exc_local)
report_arm('remote', globals_remote, emsg_remote, unexpected_exc_remote)
report_arm('local', emsg_local, unexpected_exc_local)
report_arm('remote', emsg_remote, unexpected_exc_remote)

# If the arms can't be compared (unexpected errors), stop early -- the rest
# is about output comparison.
if not can_compare_output:
set_custom_attribute('codejail.darklaunch.globals_match', 'N/A')
set_custom_attribute('codejail.darklaunch.emsg_match', 'N/A')
log.info(
"Codejail darklaunch had unexpected exception for "
f"course={limit_overrides_context}, slug={slug}:\n"
Comment thread
timmc-edx marked this conversation as resolved.
Outdated
f"Local exception: {unexpected_exc_local!r}\n"
f"Remote exception: {unexpected_exc_remote!r}"
)
return

globals_match = globals_local == globals_remote
emsg_match = normalize_error_message(emsg_local) == normalize_error_message(emsg_remote)

if not globals_match or not emsg_match:
log.info(
f"Codejail darklaunch had mismatch for course={limit_overrides_context}, slug={slug}:\n"
f"{emsg_match=}, {globals_match=}\n"
f"Local: globals={globals_local!r}, emsg={emsg_local!r}\n"
f"Remote: globals={globals_remote!r}, emsg={emsg_remote!r}"
)

# .. custom_attribute_name: codejail.darklaunch.globals_match
# .. custom_attribute_description: True if local and remote globals_dict
# values match, False otherwise. 'N/A' when either arm raised an
Expand Down
40 changes: 12 additions & 28 deletions xmodule/capa/safe_exec/tests/test_safe_exec.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,12 +256,10 @@ def remote_exec(data):
],
expect_log_info_calls=[
call(
"Codejail darklaunch local results for slug=None: globals={'overwrite': 'mock local'}, "
"emsg=None, exception=None"
),
call(
"Codejail darklaunch remote results for slug=None: globals={'overwrite': 'mock remote'}, "
"emsg=None, exception=None"
"Codejail darklaunch had mismatch for course=None, slug=None:\n"
"emsg_match=True, globals_match=False\n"
"Local: globals={'overwrite': 'mock local'}, emsg=None\n"
"Remote: globals={'overwrite': 'mock remote'}, emsg=None"
),
],
# Should only see behavior of local exec
Expand Down Expand Up @@ -296,12 +294,9 @@ def remote_exec(data):
],
expect_log_info_calls=[
call(
"Codejail darklaunch local results for slug=None: globals={}, "
"emsg='unexpected', exception=BaseException('unexpected')"
),
call(
"Codejail darklaunch remote results for slug=None: globals={}, "
"emsg=None, exception=None"
"Codejail darklaunch had unexpected exception for course=None, slug=None:\n"
"Local exception: BaseException('unexpected')\n"
"Remote exception: None"
),
],
expect_globals_contains={},
Expand Down Expand Up @@ -332,12 +327,10 @@ def remote_exec(data):
],
expect_log_info_calls=[
call(
"Codejail darklaunch local results for slug=None: globals={}, "
"emsg='oops', exception=None"
),
call(
"Codejail darklaunch remote results for slug=None: globals={}, "
"emsg='OH NO', exception=None"
"Codejail darklaunch had mismatch for course=None, slug=None:\n"
"emsg_match=False, globals_match=True\n"
"Local: globals={}, emsg='oops'\n"
"Remote: globals={}, emsg='OH NO'"
),
],
expect_globals_contains={},
Expand Down Expand Up @@ -365,16 +358,7 @@ def remote_exec(data):
call('codejail.darklaunch.globals_match', True),
call('codejail.darklaunch.emsg_match', True), # even though not exact match
],
expect_log_info_calls=[
call(
"Codejail darklaunch local results for slug=None: globals={}, "
"emsg='stack trace involving /tmp/codejail-1234567/whatever.py', exception=None"
),
call(
"Codejail darklaunch remote results for slug=None: globals={}, "
"emsg='stack trace involving /tmp/codejail-abcd_EFG/whatever.py', exception=None"
),
],
expect_log_info_calls=[],
expect_globals_contains={},
)
assert isinstance(results['raised'], SafeExecException)
Expand Down