chore: add prefixes to log messages

[knqyf263]

Description

Adding prefixes for each artifact.

Before

2024-10-01T15:43:48+04:00       INFO    [db] Need to update DB
2024-10-01T15:43:48+04:00       INFO    Downloading vulnerability DB...
2024-10-01T15:43:48+04:00       INFO    Downloading  artifact...        repo="ghcr.io/aquasecurity/trivy-db:2"
53.85 MiB / 53.85 MiB [---------------------------------------------------------------------------------------------------------------] 100.00% 23.26 MiB p/s 2.5s
2024-10-01T15:43:51+04:00       INFO    Artifact successfully downloaded        repo="ghcr.io/aquasecurity/trivy-db:2"

After

2024-10-01T18:16:51+04:00       INFO    [vulndb] Need to update DB
2024-10-01T18:16:51+04:00       INFO    [vulndb] Downloading vulnerability DB...
2024-10-01T18:16:51+04:00       INFO    [vulndb] Downloading artifact...        repo="ghcr.io/aquasecurity/trivy-db:2"
53.87 MiB / 53.87 MiB [---------------------------------------------------------------------------------------------------------------] 100.00% 28.86 MiB p/s 2.1s
2024-10-01T18:16:55+04:00       INFO    [vulndb] Artifact successfully downloaded       repo="ghcr.io/aquasecurity/trivy-db:2"

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).

[nikpivkin]

Here it is logged without the prefix

[DmitriyLewen]

LGTM

Left one comment

[knqyf263]

Here it is logged without the prefix

@nikpivkin It is called via NewVersionInfo, which doesn't have a context as the argument. That's why I didn't change it, but I eventually used context.TODO() so we can fix it later.
6d648a8

[DmitriyLewen]

LGTM

[nikpivkin]

@knqyf263 Is there an advantage of using context for logging over using a logger with a prefix as a field? Then there would be no need for context.

[nikpivkin]

LGTM

[knqyf263]

@knqyf263 Is there an advantage of using context for logging over using a logger with a prefix as a field? Then there would be no need for context.

It seems more intuitive and preferable to define a new logger rather than a context if the prefix is closed within a specific package. In this case, however, the prefix needed to be propagated over packages (over context).

Other approaches are:

1. Re-define a new logger in each package

diff --git a/pkg/commands/artifact/run.go b/pkg/commands/artifact/run.go
index eeca48436..d85f54226 100644
--- a/pkg/commands/artifact/run.go
+++ b/pkg/commands/artifact/run.go
@@ -618,8 +618,8 @@ func (r *runner) scan(ctx context.Context, opts flag.Options, initializeScanner
 }

 func initMisconfScannerOption(ctx context.Context, opts flag.Options) (misconf.ScannerOption, error) {
-       ctx = log.WithContextPrefix(ctx, log.PrefixMisconfiguration)
-       log.InfoContext(ctx, "Misconfiguration scanning is enabled")
+       logger := log.WithPrefix(log.PrefixMisconfiguration)
+       logger.Info("Misconfiguration scanning is enabled")

        var downloadedPolicyPaths []string
        var disableEmbedded bool
@@ -630,7 +630,7 @@ func initMisconfScannerOption(ctx context.Context, opts flag.Options) (misconf.S
                        log.ErrorContext(ctx, "Falling back to embedded checks", log.Err(err))
                }
        } else {
-               log.DebugContext(ctx, "Checks successfully loaded from disk")
+               logger.Debug("Checks successfully loaded from disk")
                disableEmbedded = true
        }

diff --git a/pkg/commands/operation/operation.go b/pkg/commands/operation/operation.go
index ac52eee7f..e4f0feba2 100644
--- a/pkg/commands/operation/operation.go
+++ b/pkg/commands/operation/operation.go
@@ -82,6 +82,7 @@ func InitBuiltinChecks(ctx context.Context, cacheDir string, quiet, skipUpdate b
        mu.Lock()
        defer mu.Unlock()

+       logger := log.WithPrefix(log.PrefixMisconfiguration)
        client, err := policy.NewClient(cacheDir, quiet, checkBundleRepository)
        if err != nil {
                return nil, xerrors.Errorf("check client error: %w", err)
@@ -96,8 +97,8 @@ func InitBuiltinChecks(ctx context.Context, cacheDir string, quiet, skipUpdate b
        }

        if needsUpdate {
-               log.InfoContext(ctx, "Need to update the built-in checks")
-               log.InfoContext(ctx, "Downloading the built-in checks...")
+               logger.Info("Need to update the built-in checks")
+               logger.Info("Downloading the built-in checks...")
                if err = client.DownloadBuiltinChecks(ctx, registryOpts); err != nil {
                        return nil, xerrors.Errorf("failed to download built-in policies: %w", err)
                }

2. Pass a logger

+func InitBuiltinChecks(ctx context.Context, logger *log.Logger, cacheDir string, quiet, skipUpdate bool, checkBundleRepository string, registryOpts ftypes.RegistryOptions) ([]string, error) {
        mu.Lock()
        defer mu.Unlock()

@@ -96,8 +96,8 @@ func InitBuiltinChecks(ctx context.Context, cacheDir string, quiet, skipUpdate b
        }

        if needsUpdate {
-               log.InfoContext(ctx, "Need to update the built-in checks")
-               log.InfoContext(ctx, "Downloading the built-in checks...")
+               logger.Info("Need to update the built-in checks")
+               logger.Info("Downloading the built-in checks...")

Using a context looks better. Does it answer your question?

[nikpivkin]

@knqyf263 Yeah, thanks.

[knqyf263]

@aquasecurity/trivy I believe that context.Context should be used as little as possible because it is difficult to see what values are in it, but I thought that context.Context might be suitable (in terms of its name) when a common "context" (in this case a logging prefix) needs to be shared across multiple packages. However, I'm always open to better ideas!