Skip to content

Commit

Permalink
fix: resolve duplicated debug log for oras cp (#1139)
Browse files Browse the repository at this point in the history
Signed-off-by: Billy Zha <[email protected]>
  • Loading branch information
qweeah authored Oct 13, 2023
1 parent f1ef429 commit 6901c47
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 8 deletions.
11 changes: 7 additions & 4 deletions internal/trace/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,14 @@ import (
"sync/atomic"
)

// requestCount records the number of logged request-response pairs and will
// be used as the unique id for the next pair.
var requestCount uint64

// Transport is an http.RoundTripper that keeps track of the in-flight
// request and add hooks to report HTTP tracing events.
type Transport struct {
http.RoundTripper
count uint64
}

// NewTransport creates and returns a new instance of Transport
Expand All @@ -38,13 +41,13 @@ func NewTransport(base http.RoundTripper) *Transport {

// RoundTrip calls base roundtrip while keeping track of the current request.
func (t *Transport) RoundTrip(req *http.Request) (resp *http.Response, err error) {
number := atomic.AddUint64(&t.count, 1) - 1
id := atomic.AddUint64(&requestCount, 1) - 1
ctx := req.Context()
e := Logger(ctx)

// log the request
e.Debugf("Request #%d\n> Request URL: %q\n> Request method: %q\n> Request headers:\n%s",
number, req.URL, req.Method, logHeader(req.Header))
id, req.URL, req.Method, logHeader(req.Header))

// log the response
resp, err = t.RoundTripper.RoundTrip(req)
Expand All @@ -54,7 +57,7 @@ func (t *Transport) RoundTrip(req *http.Request) (resp *http.Response, err error
e.Errorf("No response obtained for request %s %q", req.Method, req.URL)
} else {
e.Debugf("Response #%d\n< Response Status: %q\n< Response headers:\n%s",
number, resp.Status, logHeader(resp.Header))
id, resp.Status, logHeader(resp.Header))
}
return resp, err
}
Expand Down
16 changes: 12 additions & 4 deletions test/e2e/suite/command/cp.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"strings"

. "github.com/onsi/ginkgo/v2"
"github.com/onsi/gomega"
. "github.com/onsi/gomega"
"github.com/onsi/gomega/gbytes"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
Expand All @@ -44,9 +43,9 @@ var _ = Describe("ORAS beginners:", func() {
When("running cp command", func() {
It("should show help doc with feature flags", func() {
out := ORAS("cp", "--help").MatchKeyWords("Copy", ExampleDesc).Exec()
gomega.Expect(out).Should(gbytes.Say("--from-distribution-spec string\\s+%s", regexp.QuoteMeta(feature.Preview.Mark)))
gomega.Expect(out).Should(gbytes.Say("-r, --recursive\\s+%s", regexp.QuoteMeta(feature.Preview.Mark)))
gomega.Expect(out).Should(gbytes.Say("--to-distribution-spec string\\s+%s", regexp.QuoteMeta(feature.Preview.Mark)))
Expect(out).Should(gbytes.Say("--from-distribution-spec string\\s+%s", regexp.QuoteMeta(feature.Preview.Mark)))
Expect(out).Should(gbytes.Say("-r, --recursive\\s+%s", regexp.QuoteMeta(feature.Preview.Mark)))
Expect(out).Should(gbytes.Say("--to-distribution-spec string\\s+%s", regexp.QuoteMeta(feature.Preview.Mark)))
})

It("should fail when no reference provided", func() {
Expand Down Expand Up @@ -80,6 +79,15 @@ var _ = Describe("1.1 registry users:", func() {
CompareRef(src, dst)
})

It("should copy and show logs with deduplicated id", func() {
src := RegistryRef(ZOTHost, ArtifactRepo, blob.Tag)
dst := RegistryRef(ZOTHost, cpTestRepo("debug-log"), "copied")
session := ORAS("cp", src, dst, "-d").Exec()
Expect(session.Err).To(gbytes.Say("Request #0"))
Expect(session.Err).NotTo(gbytes.Say("Request #0"))
CompareRef(src, dst)
})

It("should copy an artifact with config", func() {
src := RegistryRef(ZOTHost, ArtifactRepo, config.Tag)
dst := RegistryRef(ZOTHost, cpTestRepo("artifact-with-config"), "copied")
Expand Down

0 comments on commit 6901c47

Please sign in to comment.