-
Notifications
You must be signed in to change notification settings - Fork 597
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.
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
Reduce log spam on unknown relationship type #1797
Conversation
An unknown relationship type doesn't mean things are incorrect; it probably means that the version of syft that generated the SBOM is ahead of the version of syft that's parsing it. Signed-off-by: Will Murphy <[email protected]>
Knowing that the event-type was unexpected seems like useful information while debugging. Signed-off-by: Will Murphy <[email protected]>
Benchmark Test ResultsBenchmark results from the latest changes vs base branch
|
Discussed this with @kzantow and @wagoodman, and it might be better to still warn about this, but warn only once, rather than once per lost relationship. I will send a revision. |
Signed-off-by: Will Murphy <[email protected]>
Signed-off-by: Will Murphy <[email protected]>
Signed-off-by: Will Murphy <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉 nicely done! -- just a small formatting suggestion
errorCounts[e.Error()] = errorCounts[e.Error()] + 1 | ||
} | ||
for msg, count := range errorCounts { | ||
errorMessages = append(errorMessages, fmt.Sprintf("%q occurred %d time(s)", msg, count)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion:
if count > 1 {
errorMessages = append(errorMessages, fmt.Sprintf("%s (occurred %d times)", msg, count))
} else {
errorMessages = append(errorMessages, msg)
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion - I think I'll leave it as is for now, since I like that the whole block of de-duplicated errors in the logs all look the same, but I'd be open to changing this in a follow-up if it seems important.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks great 👍
Rather than log a warning for every instance of an unknown relationship type, or similar error, log a count of how many times each of these errors is raised. --------- Signed-off-by: Will Murphy <[email protected]>
* main: fix: Reduce log spam on unknown relationship type (#1797) chore(deps): update bootstrap tools to latest versions (#1807) chore(deps): bump golang.org/x/net from 0.9.0 to 0.10.0 (#1802) chore(deps): bump github.com/docker/docker (#1795) chore(deps): bump github.com/google/go-containerregistry (#1796) chore(deps): update bootstrap tools to latest versions (#1792) Print package list when extra packages found (#1791) chore(deps): update bootstrap tools to latest versions (#1786) chore(deps): bump golang.org/x/term from 0.7.0 to 0.8.0 (#1787) Update the CPE generation for spring-security-core (#1789) chore: do not HTML escape PackageURLs (#1782) chore: do not include kernel module cataloger by default (#1784) Signed-off-by: Christopher Phillips <[email protected]>
* main: (32 commits) chore(deps): bump github.com/google/go-containerregistry (#1823) chore(deps): bump github.com/sirupsen/logrus from 1.9.0 to 1.9.1 (#1822) chore(deps): bump github.com/docker/docker (#1824) fix: update field plurality of 8.0.0 schema before release (#1820) fix: update cataloger to check for expressions before split (#1819) feat: update syft license concept to complex struct (#1743) fix: cyclonedx depends-on relationship inverted (#1816) fix: retain sbom cataloger relationships (#1509) feat: warn if parsing newer SBOM (#1810) feat: Add R cataloger (#1790) update cosign to v2 release (different go module) (#1805) fix: Reduce log spam on unknown relationship type (#1797) chore(deps): update bootstrap tools to latest versions (#1807) chore(deps): bump golang.org/x/net from 0.9.0 to 0.10.0 (#1802) chore(deps): bump github.com/docker/docker (#1795) chore(deps): bump github.com/google/go-containerregistry (#1796) chore(deps): update bootstrap tools to latest versions (#1792) Print package list when extra packages found (#1791) chore(deps): update bootstrap tools to latest versions (#1786) chore(deps): bump golang.org/x/term from 0.7.0 to 0.8.0 (#1787) ... Signed-off-by: Christopher Phillips <[email protected]>
Rather than log a warning for every instance of an unknown relationship type, or similar error, log a count of how many times each of these errors is raised. --------- Signed-off-by: Will Murphy <[email protected]>
An unknown relationship type doesn't mean things are incorrect; it probably means that the version of syft that generated the SBOM is ahead of the version of syft that's parsing it.
Addresses #1812, but won't fix it until the version of syft that's imported by grype is updated, at https://github.com/anchore/grype/blob/75e7ef43cd03baba4c76af3751274c7aa7452f36/go.mod#L56.