Extend image pull metrics #2912

saschagrunert · 2019-10-22T12:24:23Z

We now add a new set of image pull metrics, which collect the transferred
bytes per image name or digest every second during an image pull. We
also add a metric which collects the skipped bytes if an image has
already been downloaded.

This fixes the ignored command line parameters for the metrics, too.

Example output from the metrics server:

Download the image
Remove the image, download it again
Download the already existing image

# HELP container_runtime_crio_image_pulls_by_digest Bytes transferred by CRI-O image pulls by digest
# TYPE container_runtime_crio_image_pulls_by_digest counter
container_runtime_crio_image_pulls_by_digest{digest="sha256:2f1a357cf2898f6700b74d933e1784a348df8f719c4db77203423e63b538de9b",mediatype="application/vnd.docker.container.image.v1+json",name="docker.io/nixos/nix",size="5127"} 10254
container_runtime_crio_image_pulls_by_digest{digest="sha256:480223332b134112f7f7f1292789ae19b695c1329a7180200ae82778fab59e95",mediatype="",name="docker.io/nixos/nix",size="277005"} 554010
container_runtime_crio_image_pulls_by_digest{digest="sha256:74bc3bbbef74304d58600283df51bab6a3dbe9b1ec939bd9f5db7b3c87cfb612",mediatype="",name="docker.io/nixos/nix",size="60336471"} 1.20672942e+08
container_runtime_crio_image_pulls_by_digest{digest="sha256:9d48c3bd43c520dc2784e868a780e976b207cbf493eaff8c6596eb871cbd9609",mediatype="",name="docker.io/nixos/nix",size="2789669"} 5.579338e+06
# HELP container_runtime_crio_image_pulls_by_name Bytes transferred by CRI-O image pulls by name
# TYPE container_runtime_crio_image_pulls_by_name counter
container_runtime_crio_image_pulls_by_name{name="docker.io/nixos/nix",size="63408272"} 1.26816544e+08
# HELP container_runtime_crio_image_pulls_by_name_skipped Bytes skipped by CRI-O image pulls by name
# TYPE container_runtime_crio_image_pulls_by_name_skipped counter
container_runtime_crio_image_pulls_by_name_skipped{name="docker.io/nixos/nix"} 1.91975283e+08

Closes #2757
Related change: containers/image#732
Discussion on SIG Node: https://groups.google.com/forum/#!topic/kubernetes-sig-node/JHEus_TlZzA

saschagrunert · 2019-10-22T13:59:07Z

/test e2e_fedora

saschagrunert · 2019-10-23T09:57:11Z

/hold

needs #2915 and the vendored changes to containers/image

saschagrunert · 2019-11-01T10:33:11Z

Looks good for now, but still needs containers/image#738 to proceed.

saschagrunert · 2019-11-22T21:20:27Z

This is ready for a first review. I think I will add some integration tests for it as well later on.

saschagrunert · 2019-11-22T21:20:40Z

/hold cancel

saschagrunert · 2019-12-19T10:57:12Z

/retest

saschagrunert · 2019-12-19T10:57:25Z

PTAL @mrunalp @haircommander @rhatdan

haircommander · 2019-12-19T15:38:14Z

PTAL @smarterclayton

haircommander · 2020-01-03T14:18:48Z

PTAL @vrothberg @mtrmac

saschagrunert · 2020-01-09T13:41:53Z

Ping anyone wants to review this to unblock 1.17? 😇 @mrunalp @vrothberg @rhatdan @haircommander

server/image_pull.go

mtrmac · 2020-01-14T19:32:59Z

server/image_pull.go

+		wg := &sync.WaitGroup{}
+		progress := make(chan types.ProgressProperties)
+		wg.Add(1)
+		go func() {


(Absolutely non-blocking: As an absolutely worthless personal opinion, I tend to avoid nested functions; moving them out makes the body shorter, and allows writing independent unit tests.

OTOH inlining them reads closer to a straight-line code. Feel completely free to leave this as is, as long as this is an intentional choice.)

I see your point and would generally agree. In this example I'd like to stick to the current approach to make the code easier to understand when having the critical parts close to each other :)

server/image_pull.go

saschagrunert · 2020-01-15T10:29:44Z

Thanks for the review @mtrmac, I applied my review related fixes in a dedicated commit on top of this PR. PTAL

mtrmac

Just one more thing about the goroutine.

server/image_pull.go

We now add a new set of image pull metrics, which collect the transferred bytes per image name or digest every second during an image pull. We also add a metric which collects the skipped bytes if an image has already been downloaded. Signed-off-by: Sascha Grunert <sgrunert@suse.com>

saschagrunert · 2020-01-15T12:06:33Z

I rebased on top of the latest master because this PR was a bit outdated.

mtrmac

Thanks! LGTM.

openshift-ci-robot · 2020-01-15T12:09:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mtrmac, saschagrunert

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [saschagrunert]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2020-01-15T12:09:48Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mtrmac, saschagrunert

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [saschagrunert]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

saschagrunert · 2020-01-15T12:56:55Z

/retest

haircommander · 2020-01-15T14:14:04Z

LGTM

rhatdan · 2020-01-15T18:05:18Z

/lgtm

saschagrunert · 2020-01-15T20:22:47Z

/retest

saschagrunert · 2020-01-16T08:50:04Z

/test integration_rhel

saschagrunert · 2020-01-16T10:31:16Z

/test kata-containers

saschagrunert · 2020-01-16T11:11:35Z

/cherrypick release-1.17

openshift-cherrypick-robot · 2020-01-16T11:11:48Z

@saschagrunert: failed to push cherry-picked changes in GitHub: pushing failed, output: "To https://github.com/openshift-cherrypick-robot/cri-o\n ! [remote rejected] cherry-pick-2912-to-release-1.17 -> cherry-pick-2912-to-release-1.17 (cannot lock ref 'refs/heads/cherry-pick-2912-to-release-1.17': reference already exists)\nerror: failed to push some refs to 'https://openshift-cherrypick-robot:CENSORED@github.com/openshift-cherrypick-robot/cri-o'\n", error: exit status 1

Details

In response to this:

/cherrypick release-1.17

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-cherrypick-robot · 2020-01-16T11:11:49Z

@saschagrunert: new pull request created: #3122

Details

In response to this:

/cherrypick release-1.17

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

saschagrunert requested review from mrunalp and runcom as code owners October 22, 2019 12:24

openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Oct 22, 2019

openshift-ci-robot requested review from sboeuf and vrothberg October 22, 2019 12:24

openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 22, 2019

openshift-ci-robot requested a review from umohnani8 October 22, 2019 12:24

This was referenced Oct 22, 2019

Image pull metrics #2757

Closed

Improve progress reader to report start/end and offset update containers/image#732

Merged

saschagrunert force-pushed the pull-metrics branch from 2b6b3fe to a7ba10d Compare October 23, 2019 09:44

saschagrunert changed the title ~~WIP: Add image pull metrics~~ Extend image pull metrics Oct 23, 2019

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 23, 2019

saschagrunert force-pushed the pull-metrics branch from a7ba10d to 641d35c Compare November 1, 2019 10:29

saschagrunert force-pushed the pull-metrics branch from 641d35c to 01098bd Compare November 1, 2019 10:35

saschagrunert force-pushed the pull-metrics branch from 01098bd to 4f7602d Compare November 22, 2019 21:16

openshift-ci-robot removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Nov 22, 2019

saschagrunert force-pushed the pull-metrics branch 2 times, most recently from b03c25a to 418a9b5 Compare November 23, 2019 02:27

saschagrunert force-pushed the pull-metrics branch from 52a9bcd to 45899b2 Compare December 19, 2019 09:58

mtrmac reviewed Jan 14, 2020

View reviewed changes

mtrmac reviewed Jan 15, 2020

View reviewed changes

server/image_pull.go Outdated Show resolved Hide resolved

server/image_pull.go Outdated Show resolved Hide resolved

saschagrunert force-pushed the pull-metrics branch from c233a3e to aa9293e Compare January 15, 2020 12:04

mtrmac approved these changes Jan 15, 2020

View reviewed changes

openshift-ci-robot assigned rhatdan Jan 15, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 15, 2020

openshift-merge-robot merged commit 7151931 into cri-o:master Jan 16, 2020

saschagrunert deleted the pull-metrics branch January 16, 2020 11:11

openshift-cherrypick-robot mentioned this pull request Jan 16, 2020

[release-1.17] Extend image pull metrics #3122

Merged

simonpasquier mentioned this pull request Jun 8, 2021

container_runtime_crio_image_* metrics may have excessive cardinality #4982

Closed

Extend image pull metrics #2912

Extend image pull metrics #2912

Uh oh!

Conversation

saschagrunert commented Oct 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

saschagrunert commented Oct 22, 2019

Uh oh!

saschagrunert commented Oct 23, 2019

Uh oh!

saschagrunert commented Nov 1, 2019

Uh oh!

saschagrunert commented Nov 22, 2019

Uh oh!

saschagrunert commented Nov 22, 2019

Uh oh!

saschagrunert commented Dec 19, 2019

Uh oh!

saschagrunert commented Dec 19, 2019

Uh oh!

haircommander commented Dec 19, 2019

Uh oh!

haircommander commented Jan 3, 2020

Uh oh!

saschagrunert commented Jan 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mtrmac Jan 14, 2020

Choose a reason for hiding this comment

Uh oh!

saschagrunert Jan 15, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

saschagrunert commented Jan 15, 2020

Uh oh!

mtrmac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

saschagrunert commented Jan 15, 2020

Uh oh!

mtrmac left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci-robot commented Jan 15, 2020

Uh oh!

openshift-ci-robot commented Jan 15, 2020

Uh oh!

saschagrunert commented Jan 15, 2020

Uh oh!

haircommander commented Jan 15, 2020

Uh oh!

rhatdan commented Jan 15, 2020

Uh oh!

saschagrunert commented Jan 15, 2020

Uh oh!

saschagrunert commented Jan 16, 2020

Uh oh!

saschagrunert commented Jan 16, 2020

Uh oh!

saschagrunert commented Jan 16, 2020

Uh oh!

openshift-cherrypick-robot commented Jan 16, 2020

Uh oh!

openshift-cherrypick-robot commented Jan 16, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

saschagrunert commented Oct 22, 2019 •

edited

Loading

saschagrunert commented Jan 9, 2020 •

edited

Loading