Skip to content

Commit

Permalink
perf(log): encode objects only when logged (#481)
Browse files Browse the repository at this point in the history
Fix:
- replaced `.String()` with the `%v` format to avoid rendering the
string before actually logging it.

Resolves #480

Signed-off-by: Junjie Gao <[email protected]>
Signed-off-by: Patrick Zheng <[email protected]>
  • Loading branch information
JeyJeyGao authored and Two-Hearts committed Dec 9, 2024
1 parent e7005a6 commit 5b21f2f
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 6 deletions.
4 changes: 2 additions & 2 deletions notation.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func Sign(ctx context.Context, signer Signer, repo registry.Repository, signOpts
}
// artifactRef is a tag
logger.Warnf("Always sign the artifact using digest(`@sha256:...`) rather than a tag(`:%s`) because tags are mutable and a tag reference can point to a different artifact than the one signed", artifactRef)
logger.Infof("Resolved artifact tag `%s` to digest `%s` before signing", artifactRef, targetDesc.Digest.String())
logger.Infof("Resolved artifact tag `%s` to digest `%v` before signing", artifactRef, targetDesc.Digest)
}
descToSign, err := addUserMetadataToDescriptor(ctx, targetDesc, signOpts.UserMetadata)
if err != nil {
Expand Down Expand Up @@ -378,7 +378,7 @@ func Verify(ctx context.Context, verifier Verifier, repo registry.Repository, ve
}
if ref.ValidateReferenceAsDigest() != nil {
// artifactRef is not a digest reference
logger.Infof("Resolved artifact tag `%s` to digest `%s` before verification", ref.Reference, artifactDescriptor.Digest.String())
logger.Infof("Resolved artifact tag `%s` to digest `%v` before verification", ref.Reference, artifactDescriptor.Digest)

Check warning on line 381 in notation.go

View check run for this annotation

Codecov / codecov/patch

notation.go#L381

Added line #L381 was not covered by tests
logger.Warn("The resolved digest may not point to the same signed artifact, since tags are mutable")
} else if ref.Reference != artifactDescriptor.Digest.String() {
return ocispec.Descriptor{}, nil, ErrorSignatureRetrievalFailed{Msg: fmt.Sprintf("user input digest %s does not match the resolved digest %s", ref.Reference, artifactDescriptor.Digest.String())}
Expand Down
8 changes: 4 additions & 4 deletions verifier/verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,7 @@ func revocationFinalResult(certResults []*revocationresult.CertRevocationResult,
certResult := certResults[i]
if certResult.RevocationMethod == revocationresult.RevocationMethodOCSPFallbackCRL {
// log the fallback warning
logger.Warnf("OCSP check failed with unknown error and fallback to CRL check for certificate #%d in chain with subject %v", (i + 1), cert.Subject.String())
logger.Warnf("OCSP check failed with unknown error and fallback to CRL check for certificate #%d in chain with subject %v", (i + 1), cert.Subject)
}
for _, serverResult := range certResult.ServerResults {
if serverResult.Error != nil {
Expand All @@ -687,10 +687,10 @@ func revocationFinalResult(certResults []*revocationresult.CertRevocationResult,
// when the final revocation method is OCSPFallbackCRL,
// the OCSP server results should not be logged as an error
// since the CRL revocation check can succeed.
logger.Debugf("Certificate #%d in chain with subject %v encountered an error for revocation method %s at URL %q: %v", (i + 1), cert.Subject.String(), revocationresult.RevocationMethodOCSP, serverResult.Server, serverResult.Error)
logger.Debugf("Certificate #%d in chain with subject %v encountered an error for revocation method %s at URL %q: %v", (i + 1), cert.Subject, revocationresult.RevocationMethodOCSP, serverResult.Server, serverResult.Error)
continue
}
logger.Errorf("Certificate #%d in chain with subject %v encountered an error for revocation method %s at URL %q: %v", (i + 1), cert.Subject.String(), serverResult.RevocationMethod, serverResult.Server, serverResult.Error)
logger.Errorf("Certificate #%d in chain with subject %v encountered an error for revocation method %s at URL %q: %v", (i + 1), cert.Subject, serverResult.RevocationMethod, serverResult.Server, serverResult.Error)
}
}

Expand All @@ -706,7 +706,7 @@ func revocationFinalResult(certResults []*revocationresult.CertRevocationResult,
}

if i < len(certResults)-1 && certResult.Result == revocationresult.ResultNonRevokable {
logger.Warnf("Certificate #%d in the chain with subject %v neither has an OCSP nor a CRL revocation method.", (i + 1), cert.Subject.String())
logger.Warnf("Certificate #%d in the chain with subject %v neither has an OCSP nor a CRL revocation method.", (i + 1), cert.Subject)
}
}
if revokedFound {
Expand Down

0 comments on commit 5b21f2f

Please sign in to comment.