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.

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

add an option to show JSON cache metrics #4464

Merged
merged 3 commits into from
Jan 8, 2024

Conversation

azr
Copy link
Contributor

@azr azr commented Dec 5, 2023

Hello there,

this adds a new json-cache-metrics parameter to write to an os.File. Handy in deamonless mode, mode with which it is a bit hard to scrape metrics the good way.

At first it was just writting to stderr like the rest, after everything is done. But while seeking the end of log file to just get the last line works, it turned out a bit brittle at scale (in our case).

So I made it possible to pick a file just like for trace. (But also kept the std(out/err) option, nice locally)

LMK what you think.

Closes #4445

Copy link
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, from the initial issue I thought you meant something on the daemon side. I think this is fine though, maybe "debug" prefix for the buildctl flag? @AkihiroSuda wdyt?

@AkihiroSuda
Copy link
Member

I think this is fine though, maybe "debug" prefix for the buildctl flag? @AkihiroSuda wdyt?

👍
eventually we may drop the prefix though

@azr
Copy link
Contributor Author

azr commented Dec 6, 2023

Owkay, switching for debug-json-cache-metrics !

@azr azr force-pushed the azr/cache-metrics branch 3 times, most recently from 34836ad to 71f1a05 Compare December 6, 2023 08:44
Signed-off-by: Adrien Delorme <azr@users.noreply.github.com>
Signed-off-by: Adrien Delorme <azr@users.noreply.github.com>
Signed-off-by: Adrien Delorme <azr@users.noreply.github.com>
@azr
Copy link
Contributor Author

azr commented Jan 8, 2024

Thanks for the green checkmark ! Is there anything I can do from here ?

@tonistiigi tonistiigi merged commit 8a7e5c7 into moby:master Jan 8, 2024
60 checks passed
@azr azr deleted the azr/cache-metrics branch January 9, 2024 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: cache hit / miss metric
3 participants