refactor: replace log.Printf calls with Reporter interface by anderruiz · Pull Request #175 · DataDog/datadog-sbom-generator

anderruiz · 2026-06-18T09:14:56Z

Summary

Replaces all 11 log.Printf/log.Println calls in pkg/ with the existing reporter.Reporter interface methods (Warnf, Infof, Errorf, Verbosef)
Threads Reporter through to scanner, extractor matchers (Ruby, Python, JavaScript), and reachability layers where it wasn't previously available
Library consumers can now use VoidReporter to fully silence all log output, eliminating noise in their service's logs

Motivation

Downstream consumers using this library (e.g. sci) were seeing log.Printf output from the library appearing as noise in their service logs. The library already had the right abstraction (reporter.Reporter + VoidReporter) but 11 call sites bypassed it.

Changes

pkg/scanner/datadog_sbom_generator.go — use Reporter already in scope
pkg/scanner/vulnerability_result.go — add Reporter parameter to exportMetadata
pkg/extractor/ruby/match-gemspec.go, match-gemfile.go — thread Reporter through enrichment functions
pkg/extractor/python/pyproject_package_collector.go — add reporter field to struct
pkg/extractor/javascript/match-package-json.go — add reporter field to packageJSONDependencyMap
pkg/reachability/codefile/java.go — add reporter field to ReachabilityJava, update NewJavaReachableDetector signature

Test plan

All existing tests pass unchanged
Zero log.Print* calls remain in pkg/ source files
Linter clean

🤖 Generated with Claude Code

Thread the Reporter to exportMetadata and replace direct log.Printf/Println calls in DoScan and exportMetadata with appropriate Reporter methods (Warnf, Errorf). This ensures library consumers using VoidReporter get complete silence. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

Thread the Reporter from ScanContext to internal helper functions in Ruby (gemspec/gemfile enrichPackagesWithLocation), Python (pyprojectPackageCollector), and JavaScript (packageJSONDependencyMap.UnmarshalJSON) matchers. Update the pyproject test to use ExtractFromFileWithContext with a real Reporter instead of intercepting log.Writer(). Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

Add reporter field to ReachabilityJava struct and thread it from PerformReachabilityAnalysis. Remove redundant log in readFileContent since the error is already returned and handled by the caller. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7f164bbfbd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-06-18T09:19:17Z

 	if unmarshallErr != nil {
-		log.Printf("could not unmarshal %s, received error of %s", packageJSONContent, unmarshallErr)
+		if depMap.reporter != nil {
+			depMap.reporter.Errorf("could not unmarshal %s, received error of %s", packageJSONContent, unmarshallErr)


Use a warning for recoverable package.json parse failures

When a dependency section is valid JSON but not an object (for example "dependencies": []), this json.Unmarshal fails but UnmarshalJSON still returns nil and continues matching as before. Calling Errorf here marks the reporter as errored, and cmd/datadog-sbom-generator/main.go:67 turns any such mark into exit code 127 after an otherwise successful scan, so a recoverable matcher warning now fails the CLI; use a non-error reporting method for this path.

Useful? React with 👍 / 👎.

Fixed. Changed Errorf to Warnf since this parse failure is recoverable — the matcher continues normally, so marking the scan as errored (and causing exit code 127 in the CLI) was incorrect.

…ailures - Errorf marks the scan as failed (exit code 127 in CLI); this parse failure is recoverable and should not fail the overall scan

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 317b5c7928

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-06-18T10:04:18Z


-		log.Printf(
-			"Multiple pyproject version ranges for dependency %q from %s collapse to the same unversioned PURL; CycloneDX output will keep the earliest source declaration. Saw ranges %q and %q\n",
+		c.reporter.Warnf(


Guard the reporter before warning

When PyProjectTOMLExtractor.Extract is used directly or via ExtractFromFileWithContext with a zero ScanContext/nil Reporter, a pyproject that declares two different ranges for the same dependency now panics here while trying to emit the conflict warning. This path previously used log.Printf and still returned packages, so library callers that only populate the context fields they need can regress from a successful parse to a nil-pointer crash; default to a VoidReporter or nil-check before calling Warnf.

Useful? React with 👍 / 👎.

I guess it shouldn't happen? should we be more defensive?

Let's fix it

Fixed. Added reporter.Effective(r) helper that returns VoidReporter when nil, applied at construction time in createPackageJSONFile and pyprojectPackageCollector — the reporter field is now guaranteed non-nil, removing the need for nil-guards at call sites.

piloulacdog · 2026-06-18T10:06:27Z

 				val, err := vuln.ReachableSymbolLocations.MarshalToJSONString()
 				if err != nil {
-					log.Printf("failed to marshal reachable symbol locations into a JSON string: %s\n", err)
+					r.Errorf("failed to marshal reachable symbol locations into a JSON string: %s", err)


as we do continue in the case of an error, should we instead return as r.Warnf here?

Sound good, you know better this logic

piloulacdog · 2026-06-18T10:07:08Z


-		log.Printf(
-			"Multiple pyproject version ranges for dependency %q from %s collapse to the same unversioned PURL; CycloneDX output will keep the earliest source declaration. Saw ranges %q and %q\n",
+		c.reporter.Warnf(


I guess it shouldn't happen? should we be more defensive?

- Add reporter.Effective(r) helper that returns VoidReporter when r is nil - Use it in createPackageJSONFile and pyprojectPackageCollector constructors so the reporter field is never nil after construction - Remove nil-guard at call site in UnmarshalJSON (now unnecessary)

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6246301f24

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-06-18T13:09:19Z

 		// the source of truth
 		if !ok {
-			log.Printf("Skipping package %q from Gemfile as it does not exist in the Gemfile.lock\n", gem.name)
+			r.Verbosef("Skipping package %q from Gemfile as it does not exist in the Gemfile.lock", gem.name)


Default the Gemfile matcher reporter before logging

When GemfileMatcher.Match is invoked directly or via ExtractFromFileWithContext with a zero ScanContext, context.Reporter is nil. For an out-of-sync Gemfile that declares a gem absent from Gemfile.lock (the skip branch this code is handling), this new r.Verbosef call panics instead of returning the remaining packages as the previous log.Printf path did; normalize the reporter with reporter.Effective before logging.

Useful? React with 👍 / 👎.

Fixed. Applied reporter.Effective(context.Reporter) at both call sites so enrichPackagesWithLocation always receives a non-nil reporter.

chatgpt-codex-connector · 2026-06-18T13:09:19Z

 		// the source of truth
 		if !ok {
-			log.Printf("Skipping package %q from gemspec as it does not exist in the Gemfile.lock\n", gem.name)
+			r.Verbosef("Skipping package %q from gemspec as it does not exist in the Gemfile.lock", gem.name)


Default the gemspec matcher reporter before logging

When GemspecFileMatcher.Match is used by a library caller with a zero ScanContext, r is nil here. A gemspec dependency that is not present in Gemfile.lock now takes this skip path and panics while logging, whereas the previous log.Printf path simply skipped it and preserved the parsed packages; use reporter.Effective or a nil guard before calling Verbosef.

Useful? React with 👍 / 👎.

Fixed. Applied reporter.Effective(context.Reporter) at the call site so enrichPackagesWithLocation always receives a non-nil reporter.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b78799de2a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-06-18T13:46:27Z

 	return &ReachabilityJava{
 		tsParser:               tsParser,
 		tsQueriesPerSymbolType: tsQueriesPerSymbolType,
+		reporter:               r,


Default the Java detector reporter

When NewJavaReachableDetector is called with a nil reporter (for example, a library caller preserving the old no-output behavior) and Detect receives an advisory whose symbol Type is not in symbolTypeToTSQuery, the new r.reporter.Warnf path dereferences nil and panics instead of skipping the unsupported symbol as the previous log.Printf did. Store reporter.Effective(r) here so nil remains a silent reporter.

Useful? React with 👍 / 👎.

Fixed. Applied reporter.Effective(r) in NewJavaReachableDetector so a nil reporter is silently replaced with VoidReporter rather than panicking on first use.

…nil panics

anderruiz and others added 4 commits June 18, 2026 11:00

style: fix import ordering in match-gemfile.go

7f164bb

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

anderruiz requested a review from a team as a code owner June 18, 2026 09:14

This comment has been minimized.

Sign in to view

chatgpt-codex-connector Bot reviewed Jun 18, 2026

View reviewed changes

fix: use Warnf instead of Errorf for recoverable package.json parse f…

317b5c7

…ailures - Errorf marks the scan as failed (exit code 127 in CLI); this parse failure is recoverable and should not fail the overall scan

chatgpt-codex-connector Bot reviewed Jun 18, 2026

View reviewed changes

piloulacdog approved these changes Jun 18, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Jun 18, 2026

View reviewed changes

anderruiz added 2 commits June 18, 2026 15:37

style: add blank line before return in reporter.Effective

a099059

fix: apply reporter.Effective in Ruby matchers to prevent nil panics

b78799d

chatgpt-codex-connector Bot reviewed Jun 18, 2026

View reviewed changes

fix: apply reporter.Effective in NewJavaReachableDetector to prevent …

0bda872

…nil panics

anderruiz merged commit 79103b7 into main Jun 18, 2026
11 checks passed

anderruiz deleted the ander/configurable-log-output branch June 18, 2026 14:01

Uh oh!

Conversation

anderruiz commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Motivation

Changes

Test plan

Uh oh!

This comment has been minimized.

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

anderruiz commented Jun 18, 2026 •

edited

Loading