Skip to content

fix(secops-soar): log marketplace diagnostics instead of printing - #334

Merged
dandye merged 2 commits into
google:mainfrom
VISHNU0906:fix/marketplace-print-to-logger
Nov 6, 2026
Merged

dandye merged 2 commits into
google:mainfrom
VISHNU0906:fix/marketplace-print-to-logger

Conversation

@VISHNU0906

Copy link
Copy Markdown
Contributor

Fixes #332

secops-soar marketplace tools (~294 modules) report fetch/action errors
and missing-instance warnings with print(). On the MCP stdio transport
stdout is the JSON-RPC channel, so those writes go into the protocol
stream — same class as #267 / #315 for secops.

This routes every marketplace Error fetching… / Error executing…
print through logger.error, and every Warning: No active integration instance… print through logger.warning, after importing
logger_utils.get_logger the same way the rest of secops-soar does.
The code generator is left alone for a follow-up; this matches the
immediate path the issue offered.

Test

  • Static: no remaining print( under marketplace/
  • Unit: missing-instance and fetch-error paths leave stdout empty and
    hit the logger (tests/unit/test_marketplace_logging.py)
  • Full tests/unit/ suite: 10 passed

Marketplace tools report fetch/action errors and missing-instance
warnings with print(). On the MCP stdio transport that writes into the
JSON-RPC stream (same class as google#267 / google#315).

Mechanical substitution across the 294 marketplace modules: import
logger_utils, replace Error/Warning prints with logger.error /
logger.warning. Generator left unchanged for a follow-up.

Adds unit coverage that asserts no remaining print() calls and that
missing-instance / fetch-error paths leave stdout empty while logging.

Fixes google#332

Signed-off-by: KVR Vishnu <46535652+VISHNU0906@users.noreply.github.com>
@VISHNU0906
VISHNU0906 requested a review from a team November 5, 2026 15:34
@google-cla

google-cla Bot commented Nov 5, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@VISHNU0906

Copy link
Copy Markdown
Contributor Author

@googlebot I signed it!

@dandye

dandye commented Nov 5, 2026 •

Copy link
Copy Markdown
Collaborator

Thanks for cleaning up the print() calls in the marketplace modules to route through logger!

Could you please update the logging calls to use lazy % formatting (e.g., logger.error("Error fetching instance for ActiveDirectory: %s", e)) instead of f-strings?

Per the Google Python Style Guide - Section 3.10.1 (Logging):

"For logging functions that expect a pattern-string (with %-placeholders) as their first argument: Always call them with a string literal (not an f-string!) as their first argument with pattern-parameters as subsequent arguments. Some logging implementations collect the unexpanded pattern-string as a queryable field. It also prevents spending time rendering a message that no logger is configured to output."

Following this established project convention (as done in PR #315 for server/secops investigation management) ensures consistent lazy evaluation across all MCP servers.

Convert marketplace logger.error/warning calls from f-strings to lazy
% style per Google Python Style Guide 3.10.1 and the convention set in
PR google#315. Message text is unchanged; only the formatting style.

Adds a static check that marketplace modules do not use f-string
logger calls, and asserts fetch-error records carry args.

Signed-off-by: KVR Vishnu <46535652+VISHNU0906@users.noreply.github.com>
@VISHNU0906

Copy link
Copy Markdown
Contributor Author

@dandye Updated: marketplace logger.error / logger.warning calls now use lazy % formatting per the Google Python Style Guide 3.10.1 and the same convention as PR #315. See f766daa.

@dandye dandye self-assigned this Nov 6, 2026

@dandye dandye left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for working through this and converting all the marketplace diagnostics over to use lazy % logging with clean unit test coverage! This is a great improvement and prevents stdio corruption on the MCP stream.

Merging now!

@dandye
dandye merged commit 4fdf75a into google:main Nov 6, 2026
6 checks passed
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.

secops-soar: marketplace tools print() errors to stdout, corrupting the stdio JSON-RPC stream

2 participants