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.

Sign up for GitHub

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

perf(log): encode objects only when logged #481

Merged
merged 2 commits into from
Dec 2, 2024
Merged

perf(log): encode objects only when logged #481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6310844
fix: log performance
JeyJeyGao Nov 29, 2024
da2c926
Merge branch 'main' into fix/error_message_performance
JeyJeyGao Dec 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions notation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@
}
// 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 @@ -508,7 +508,7 @@
}
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 511 in notation.go

View check run for this annotation

Codecov / codecov/patch

notation.go#L511 

Added line #L511 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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,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)
JeyJeyGao marked this conversation as resolved.
Show resolved Hide resolved
}
for _, serverResult := range certResult.ServerResults {
if serverResult.Error != nil {
Expand All @@ -821,10 +821,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)
JeyJeyGao marked this conversation as resolved.
Show resolved Hide resolved
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)
JeyJeyGao marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand All @@ -840,7 +840,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)
JeyJeyGao marked this conversation as resolved.
Show resolved Hide resolved
}
}
if revokedFound {
Expand Down
Loading