Skip to content

fix: apply redact() to remaining unredacted logger paths#2500

Merged
marklysze merged 2 commits into
ag2ai:mainfrom
amabito:fix/logger-redaction-coverage
Apr 24, 2026
Merged

fix: apply redact() to remaining unredacted logger paths#2500
marklysze merged 2 commits into
ag2ai:mainfrom
amabito:fix/logger-redaction-coverage

Conversation

@amabito
Copy link
Copy Markdown
Contributor

@amabito amabito commented Mar 20, 2026

Why are these changes needed?

PR #2485 added redact() to FileLogger.log_event, log_new_wrapper,
log_new_client, and log_function_use, but several other log methods
were missed:

  • FileLogger.log_chat_completion -- to_dict(request) without _redact()
  • FileLogger.log_new_agent -- to_dict(init_args) without _redact()
  • SqliteLogger.log_chat_completion -- json.dumps(to_dict(request)) without _redact()
  • SqliteLogger.log_event -- json.dumps(kwargs) without _redact()
  • SqliteLogger.log_function_use -- safe_serialize(args/returns) without _redact()

API keys and other sensitive fields in SENSITIVE_KEYS could be written
in plaintext to log files or the SQLite database.

This PR wraps each call site with _redact(), same pattern as #2485.

Related issue number

Fixes missed paths from #2485.

Checks

  • I've included any doc changes needed
  • I've added tests corresponding to the changes introduced
  • I've made sure all auto checks have passed

amabito and others added 2 commits March 20, 2026 12:14
FileLogger.log_chat_completion and log_new_agent, and
SqliteLogger.log_chat_completion, log_event, and log_function_use
were writing request data and event kwargs without passing them
through redact(). API keys and other sensitive fields listed in
SENSITIVE_KEYS could end up in log files or the SQLite database
in plaintext.

Wrap each of these call sites with _redact() (or _redact(to_dict(...))
where to_dict is already used), matching the pattern already applied
to the other log methods by ag2ai#2485.

Add 14 tests covering flat, nested, list-of-dicts, case-variant,
non-dict, and empty-kwargs scenarios for both loggers.
Copy link
Copy Markdown
Collaborator

@marklysze marklysze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding these redacts @amabito!

@marklysze marklysze enabled auto-merge April 24, 2026 01:50
@marklysze marklysze added this pull request to the merge queue Apr 24, 2026
Merged via the queue into ag2ai:main with commit 3879079 Apr 24, 2026
20 checks passed
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
autogen/logger/file_logger.py 79.61% <100.00%> (+5.82%) ⬆️
autogen/logger/sqlite_logger.py 68.00% <100.00%> (+6.25%) ⬆️

... and 57 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants