Skip to content
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

refactor(sbom): add sbom prefix + filepaths for decode log messages #7074

Merged
merged 6 commits into from
Jul 3, 2024

Conversation

DmitriyLewen
Copy link
Contributor

@DmitriyLewen DmitriyLewen commented Jul 2, 2024

Description

Add file_path + sbom prefix for log messages in decode.go.

before:

2024-07-02T16:06:42+06:00       WARN    Ignore the OS package as no OS is detected.

after:

2024-07-02T14:44:45+06:00       WARN    [sbom] Ignore the OS package as no OS is detected.      file_path="var/lib/db/sbom/apk-tools-2.14.4-r0.spdx.json"

Related discussions

Checklist

  • I've read the guidelines for contributing to this repository.
  • I've followed the conventions in the PR title.
  • I've added tests that prove my fix is effective or that my feature works.
  • I've updated the documentation with the relevant information (if needed).
  • I've added usage information (if the PR introduces new options)
  • I've included a "before" and "after" example to the description (if the PR is a user interface change).

@DmitriyLewen DmitriyLewen self-assigned this Jul 2, 2024
@DmitriyLewen DmitriyLewen requested a review from knqyf263 as a code owner July 2, 2024 05:34
@DmitriyLewen DmitriyLewen marked this pull request as draft July 2, 2024 05:38
@DmitriyLewen DmitriyLewen changed the title refactor(sbom): use logger with sbom prefix refactor(sbom): add sbom prefix + filepaths for decode log messages Jul 2, 2024
@DmitriyLewen DmitriyLewen marked this pull request as ready for review July 2, 2024 06:57
Comment on lines 762 to 763
ctx := log.WithContextAttrs(context.Background(), log.String("filepath", tt.inputFile))
err = sbomio.NewDecoder(cdx.BOM).Decode(ctx, &got)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we need to pass attributes in tests.

Suggested change
ctx := log.WithContextAttrs(context.Background(), log.String("filepath", tt.inputFile))
err = sbomio.NewDecoder(cdx.BOM).Decode(ctx, &got)
err = sbomio.NewDecoder(cdx.BOM).Decode(context.Background(), &got)

Copy link
Contributor Author

@DmitriyLewen DmitriyLewen Jul 2, 2024

Choose a reason for hiding this comment

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

removed e0b36ca

Comment on lines 362 to 363
ctx := log.WithContextAttrs(context.Background(), log.String("filepath", tt.inputFile))
err = sbomio.NewDecoder(v.BOM).Decode(ctx, &got)
Copy link
Collaborator

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor Author

@DmitriyLewen DmitriyLewen Jul 2, 2024

Choose a reason for hiding this comment

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

removed in e0b36ca

// Format auto-detection
format, err := sbom.DetectFormat(input.Content)
if err != nil {
return nil, xerrors.Errorf("failed to detect SBOM format: %w", err)
}

bom, err := sbom.Decode(input.Content, format)
ctx = log.WithContextAttrs(ctx, log.String("file", input.FilePath))
Copy link
Collaborator

Choose a reason for hiding this comment

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

What if adding log.FilePath(filePath string) in pkg/log and using it so we can use the consistent key name?

func FilePath(filePath string) slog.Attr {
    return String("file_path", filePath)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added (5f3f694 + ae1e4d8)

Copy link
Collaborator

Choose a reason for hiding this comment

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

added (5f3f694 + ae1e4d8)

I think they should be another PR. And merge the change into this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. The number of filepaths used was more than i expected.
I created #7080

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks. Merged #7080.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated this PR

@DmitriyLewen DmitriyLewen changed the title refactor(sbom): add sbom prefix + filepaths for decode log messages refactor(log): add log.FilePath() function Jul 2, 2024
@DmitriyLewen DmitriyLewen changed the title refactor(log): add log.FilePath() function refactor: add log.FilePath() function for logger Jul 2, 2024
@DmitriyLewen DmitriyLewen changed the title refactor: add log.FilePath() function for logger refactor(sbom): add sbom prefix + filepaths for decode log messages Jul 2, 2024
@DmitriyLewen DmitriyLewen force-pushed the fix-sbom/add-sbom-prefixes branch from ae1e4d8 to cf30a86 Compare July 2, 2024 10:13
@knqyf263 knqyf263 added this pull request to the merge queue Jul 3, 2024
Merged via the queue into aquasecurity:main with commit 266d9b1 Jul 3, 2024
13 checks passed
@DmitriyLewen DmitriyLewen deleted the fix-sbom/add-sbom-prefixes branch July 3, 2024 05:19
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