metrics: add metrics for bake command #2610
Conversation
commands/bake.go
Outdated
| var files []string | ||
| if len(o.files) == 0 { | ||
| files = []string{osutil.GetWd()} | ||
| } else { | ||
| files = make([]string, len(o.files)) | ||
| for i, file := range o.files { | ||
| files[i] = osutil.ToAbs(file) | ||
| } | ||
| sort.Strings(files) | ||
| } |
There was a problem hiding this comment.
Not sure that would work for remote bake definitions, maybe using cmdContext would work.
There was a problem hiding this comment.
Can you expand on what you mean by this? I'm not very familiar with remote bake definitions. I'm guessing this is referring to osutil.ToAbs?
There was a problem hiding this comment.
I'm guessing this is referring to
osutil.ToAbs?
Yes I mean osutil.* calls that would not be right for remote bake definition set here:
Line 68 in e2f6808
Line 474 in e2f6808
Stat of these definitions are not on the host but memory:
Lines 186 to 194 in e2f6808
Hence why I think we should use cmdContext and the relative path of files.
There was a problem hiding this comment.
Ok I think I understand now. Do you think that it would be better to just include the url and cmdContext attributes in the hash instead of trying to resolve the path of the files? The intention is just to differentiate different bake commands in different directories on the same machine. I don't think we need to resolve the absolute path to these files.
I'll mock something up and see how it looks.
jsternberg
force-pushed
the
bake-metrics
branch
from
55a2957
to
5207602
Compare
jsternberg
force-pushed
the
bake-metrics
branch
from
5207602
to
01ce010
Compare
There was a problem hiding this comment.
Some lint issues otherwise looks pretty good! https://github.com/docker/buildx/actions/runs/10286248497/job/28466467965?pr=2610#step:5:368
#18 [lint 1/1] RUN --mount=target=/go/src/github.com/docker/buildx --mount=target=/root/.cache,type=cache,id=lint-cache-darwin/amd64 xx-go --wrap && golangci-lint run
#18 85.65 commands/bake.go:649:3: ineffectual assignment to cpy (ineffassign)
#18 85.65 cpy = s
#18 85.65 ^
#18 ERROR: process "/bin/sh -c xx-go --wrap && golangci-lint run" did not complete successfully: exit code: 1
|
Not for this PR but I wonder we could have integration tests pushing metrics to a local endpoint in the sandbox similar to minio integration for s3 remote cache moby/buildkit#3398? |
|
We don't have any integration tests like that but I think we really need that at this point. |
|
Ha I must have been really tired for this one. The |
This adds metrics for the bake command using a different method of calculating the build identifier but with the same attributes otherwise. Signed-off-by: Jonathan A. Sternberg <[email protected]>
jsternberg
force-pushed
the
bake-metrics
branch
from
01ce010
to
58571ff
Compare
|
Labelled as needs follow-up to not forget for itg tests |
This adds metrics for the bake command using a different method of calculating the build identifier but with the same attributes otherwise.