Remove reflector metrics since they are causing a memory leak #74636

logicalhan · 2019-02-27T01:54:50Z

What type of PR is this?

/kind bug

What this PR does / why we need it:

Currently, our reflector metrics are a memory leak. While this has always been true (for these metrics), this has been exacerbated by the increased watches introduced by #64752. For now, let's just remove these metrics since (I think) no one wants a memory leak. This should be safe because it would not have been possible to have set up reliable monitoring around these metrics since the label names are unstable (as documented).

Which issue(s) this PR fixes:

Fixes #73587

Does this PR introduce a user-facing change?:

This PR removes the following metrics:

  reflector_items_per_list
  reflector_items_per_watch
  reflector_last_resource_version
  reflector_list_duration_seconds
  reflector_lists_total
  reflector_short_watches_total
  reflector_watch_duration_seconds
  reflector_watches_total

While this is a backwards-incompatible change, it would have been impossible to setup reliable monitoring around these metrics since the labels were not stable.

/sig api-machinery instrumentation
/cc @wojtek-t @lavalamp @liggitt

smarterclayton · 2019-02-27T02:10:23Z

I agree, we need to be more judicious because these are generally low value relative to other metrics and are not targeted like our other metrics.

liggitt · 2019-02-27T02:55:14Z

I'm in favor as well. If there are targeted metrics we want for fixed informers, we could add them in the future, but the impact of these (e.g. #73587) far outweighs any benefit.

cc @kubernetes/sig-instrumentation-pr-reviews

danielqsj · 2019-02-27T03:48:43Z

/cc @brancz

logicalhan · 2019-02-27T04:54:04Z

/test pull-kubernetes-bazel-test pull-kubernetes-integration

lavalamp · 2019-02-27T05:51:13Z

staging/src/k8s.io/client-go/tools/cache/reflector.go

 	defer r.lastSyncResourceVersionMutex.Unlock()
 	r.lastSyncResourceVersion = v
-
-	rv, err := strconv.Atoi(v)


Wow, this removed code is even worse than expected. Good call.

lavalamp · 2019-02-27T05:51:36Z

/approve
/lgtm

k8s-ci-robot · 2019-02-27T05:52:02Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lavalamp, logicalhan

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:

~~pkg/OWNERS~~ [lavalamp]
~~staging/src/k8s.io/client-go/tools/cache/OWNERS~~ [lavalamp]

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

logicalhan · 2019-02-27T06:12:24Z

/test pull-kubernetes-integration

wojtek-t · 2019-02-27T07:20:01Z

/lgtm
/retest

Thanks!

brancz · 2019-02-27T10:16:36Z

This is the second time these have caused a leak, so I welcome this 👍

This was removed upstream because of memory leak: xref: kubernetes/kubernetes#74636 xref: kubernetes/kubernetes#75700

Slutzky · 2019-11-19T16:54:31Z

I'm using promtheus-operator verison 5.3.8 and promethues 2.14.0 and still can see in the http://localhost:9090/status the following :

Name	Count
reflector_list_duration_seconds	701745
reflector_watch_duration_seconds	701745
reflector_items_per_watch	701745
reflector_items_per_list	701745
http_server_requests_seconds_bucket	559314
hikaricp_connections_usage_seconds_bucket	312846
hikaricp_connections_creation_seconds_bucket	312846
hikaricp_connections_acquire_seconds_bucket	312846
reflector_short_watches_total	233915
reflector_items_per_list_count	233915

I thought these metrics were excluded in all versions after 2.12.6 or am I wrong? I also tried to exclude these metrics by adding, but it seems to be not working ...

additionalScrapeConfigs:

  - job_name: reflector_watches_total
    metric_relabel_configs:
    - source_labels: [__name__]
      regex: 'reflector_watches_total'
      action: drop

  - job_name: reflector_items_per_list
    metric_relabel_configs:
    - source_labels: [__name__]
      regex: 'reflector_items_per_list'
      action: drop

  - job_name: reflector_items_per_watch
    metric_relabel_configs:
    - source_labels: [__name__]
      regex: 'reflector_items_per_watch'
      action: drop

  - job_name: reflector_list_duration_seconds
    metric_relabel_configs:
    - source_labels: [__name__]
      regex: 'reflector_list_duration_seconds'
      action: drop

  - job_name: reflector_items_per_watch_sum
    metric_relabel_configs:
    - source_labels: [__name__]
      regex: 'reflector_items_per_watch_sum'
      action: drop

brancz · 2019-11-21T18:04:29Z

It depends on your kubernetes version whether you may still have these, and metric relabeling would need to be done in the scrape config/ServiceMonitor that configures the target that these come from.

Slutzky · 2019-11-22T08:58:28Z

hi @brancz , I'm using version v1.12.3 on k8s

brancz · 2019-11-22T16:42:18Z

Yes in 1.12 those are still present, so you need to drop the metrics on the component ServiceMonitors that expose them.

The reflector metrics were removed in kubernetes#74636. This code is not used anymore.

Reflector metrics were removed in Kubernetes due to them causing memory leaks: kubernetes/kubernetes#74636 Signed-off-by: João Vilaça <[email protected]>

k8s-ci-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Feb 27, 2019

k8s-ci-robot requested review from lavalamp, liggitt and wojtek-t February 27, 2019 01:54

k8s-ci-robot requested a review from brancz February 27, 2019 03:48

Remove reflector metrics as they currently cause a memory leak

ca096f8

logicalhan force-pushed the reflector-metrics branch from 72707b9 to ca096f8 Compare February 27, 2019 04:29

lavalamp reviewed Feb 27, 2019

View reviewed changes

k8s-ci-robot assigned lavalamp Feb 27, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 27, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 27, 2019

k8s-ci-robot assigned wojtek-t Feb 27, 2019

wojtek-t added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Feb 27, 2019

k8s-ci-robot merged commit a514fa0 into kubernetes:master Feb 27, 2019

mfojtik pushed a commit to mfojtik/openshift-apiserver that referenced this pull request Sep 2, 2019

INTERESTING: remove reflector prometheus metrics

474e443

This was removed upstream because of memory leak: xref: kubernetes/kubernetes#74636 xref: kubernetes/kubernetes#75700

mfojtik pushed a commit to mfojtik/openshift-apiserver that referenced this pull request Sep 2, 2019

INTERESTING: remove reflector prometheus metrics

7773fb6

This was removed upstream because of memory leak: xref: kubernetes/kubernetes#74636 xref: kubernetes/kubernetes#75700

mfojtik pushed a commit to mfojtik/openshift-apiserver that referenced this pull request Sep 2, 2019

INTERESTING: remove reflector prometheus metrics

8fed209

This was removed upstream because of memory leak: xref: kubernetes/kubernetes#74636 xref: kubernetes/kubernetes#75700

mfojtik pushed a commit to mfojtik/openshift-apiserver that referenced this pull request Sep 2, 2019

INTERESTING: remove reflector prometheus metrics

081e507

This was removed upstream because of memory leak: xref: kubernetes/kubernetes#74636 xref: kubernetes/kubernetes#75700

mfojtik pushed a commit to mfojtik/openshift-apiserver that referenced this pull request Sep 2, 2019

INTERESTING: remove reflector prometheus metrics

2a947b6

This was removed upstream because of memory leak: xref: kubernetes/kubernetes#74636 xref: kubernetes/kubernetes#75700

mfojtik pushed a commit to mfojtik/openshift-apiserver that referenced this pull request Sep 5, 2019

INTERESTING: remove reflector prometheus metrics

51cf3ea

This was removed upstream because of memory leak: xref: kubernetes/kubernetes#74636 xref: kubernetes/kubernetes#75700

mfojtik pushed a commit to mfojtik/openshift-apiserver that referenced this pull request Sep 5, 2019

INTERESTING: remove reflector prometheus metrics

c82eb27

This was removed upstream because of memory leak: xref: kubernetes/kubernetes#74636 xref: kubernetes/kubernetes#75700

logicalhan mentioned this pull request Sep 5, 2019

Cleanup staticcheck for package cache #82325

Merged

mfojtik pushed a commit to mfojtik/openshift-apiserver that referenced this pull request Sep 11, 2019

INTERESTING: remove reflector prometheus metrics

a5915da

This was removed upstream because of memory leak: xref: kubernetes/kubernetes#74636 xref: kubernetes/kubernetes#75700

mfojtik pushed a commit to mfojtik/openshift-apiserver that referenced this pull request Sep 23, 2019

INTERESTING: remove reflector prometheus metrics

c582598

This was removed upstream because of memory leak: xref: kubernetes/kubernetes#74636 xref: kubernetes/kubernetes#75700

mfojtik pushed a commit to mfojtik/openshift-apiserver that referenced this pull request Sep 23, 2019

INTERESTING: remove reflector prometheus metrics

3fe0d14

This was removed upstream because of memory leak: xref: kubernetes/kubernetes#74636 xref: kubernetes/kubernetes#75700

mfojtik pushed a commit to mfojtik/openshift-apiserver that referenced this pull request Sep 23, 2019

INTERESTING: remove reflector prometheus metrics

c2120e1

This was removed upstream because of memory leak: xref: kubernetes/kubernetes#74636 xref: kubernetes/kubernetes#75700

mfojtik pushed a commit to mfojtik/openshift-apiserver that referenced this pull request Sep 24, 2019

INTERESTING: remove reflector prometheus metrics

5ce1819

This was removed upstream because of memory leak: xref: kubernetes/kubernetes#74636 xref: kubernetes/kubernetes#75700

mfojtik pushed a commit to mfojtik/openshift-apiserver that referenced this pull request Sep 25, 2019

INTERESTING: remove reflector prometheus metrics

52cdea8

This was removed upstream because of memory leak: xref: kubernetes/kubernetes#74636 xref: kubernetes/kubernetes#75700

logicalhan mentioned this pull request Oct 25, 2019

Allow show hidden metrics in kube-apiserver #84292

Merged

mm4tt added a commit to mm4tt/kubernetes that referenced this pull request Dec 10, 2019

Remove not used code for old reflector metrics

434d462

The reflector metrics were removed in kubernetes#74636. This code is not used anymore.

charith-elastic mentioned this pull request May 4, 2020

The reflector metrics is empty. kubernetes-sigs/controller-runtime#817

Closed

skonto mentioned this pull request Feb 12, 2021

Reflector metrics are not available knative/pkg#2019

Closed

zqzten mentioned this pull request May 18, 2022

✨ Enable metrics of clientgo leader election kubernetes-sigs/controller-runtime#1901

Merged

zqzten mentioned this pull request Jun 28, 2022

🐛 Remove no-op clientgo reflector metrics kubernetes-sigs/controller-runtime#1946

Merged

chenk008 mentioned this pull request Feb 5, 2024

client-go: Add metrics into informer kubernetes/enhancements#4346

Open

7 tasks

machadovilaca added a commit to machadovilaca/kubevirt that referenced this pull request Apr 8, 2024

Remove reflector metrics

53fd1da

Reflector metrics were removed in Kubernetes due to them causing memory leaks: kubernetes/kubernetes#74636 Signed-off-by: João Vilaça <[email protected]>

machadovilaca mentioned this pull request Apr 8, 2024

Refactor common component metrics kubevirt/kubevirt#11184

Merged

8 tasks

machadovilaca added a commit to machadovilaca/kubevirt that referenced this pull request Apr 8, 2024

Remove reflector metrics

b66f627

Reflector metrics were removed in Kubernetes due to them causing memory leaks: kubernetes/kubernetes#74636 Signed-off-by: João Vilaça <[email protected]>

vicentefb mentioned this pull request Dec 3, 2024

Missing agones_k8s_client_cache_* metrics googleforgames/agones#3540

Closed

Remove reflector metrics since they are causing a memory leak #74636

Remove reflector metrics since they are causing a memory leak #74636

Conversation

logicalhan commented Feb 27, 2019

Uh oh!

smarterclayton commented Feb 27, 2019

Uh oh!

liggitt commented Feb 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danielqsj commented Feb 27, 2019

Uh oh!

logicalhan commented Feb 27, 2019

Uh oh!

lavalamp Feb 27, 2019

Choose a reason for hiding this comment

Uh oh!

lavalamp commented Feb 27, 2019

Uh oh!

k8s-ci-robot commented Feb 27, 2019

Uh oh!

logicalhan commented Feb 27, 2019

Uh oh!

wojtek-t commented Feb 27, 2019

Uh oh!

brancz commented Feb 27, 2019

Uh oh!

Slutzky commented Nov 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brancz commented Nov 21, 2019

Uh oh!

Slutzky commented Nov 22, 2019

Uh oh!

brancz commented Nov 22, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

liggitt commented Feb 27, 2019 •

edited

Loading

Slutzky commented Nov 19, 2019 •

edited

Loading