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.

Sign up for GitHub

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

Jump to bottom

Expose monitoring configuration for Cilium #14

Merged
merged 1 commit into from
Sep 7, 2020
Merged

Expose monitoring configuration for Cilium #14

merged 1 commit into from
Sep 7, 2020

Conversation

zanetworker
Copy link
Contributor

@zanetworker zanetworker commented Aug 17, 2020 •
edited
Loading

How to categorize this PR?

/kind enhancement
/priority normal
/area monitoring
/area networking

What this PR does / why we need it:
This PR exposes metrics and dashboards for Cilium Agent / Hubble and Cilium Operator.

Special notes for your reviewer:

Release note:

Monitoring is now enabled by default which would expose metrics/dashboards for the agent/hubble and operator components.

@zanetworker zanetworker requested review from a team as code owners August 17, 2020 07:51
@gardener-robot gardener-robot added area/monitoring Monitoring (including availability monitoring and alerting) related area/networking Networking related kind/enhancement Enhancement, improvement, extension priority/normal labels Aug 17, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 17, 2020
@zanetworker
Copy link
Contributor Author

zanetworker commented Aug 17, 2020 •
edited
Loading

/invite @wyb1 @rfranzke

@gardener-robot gardener-robot requested a review from wyb1 August 17, 2020 07:51
@gardener-robot gardener-robot added the needs/review Needs review label Aug 17, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Aug 17, 2020
vpnachev
vpnachev reviewed Aug 17, 2020
View reviewed changes
charts/images.yaml Outdated Show resolved Hide resolved
pkg/controller/actuator.go Outdated Show resolved Hide resolved
pkg/controller/actuator_reconcile.go Outdated Show resolved Hide resolved
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 17, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 17, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 18, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 18, 2020
wyb1
wyb1 suggested changes Aug 18, 2020
View reviewed changes
Copy link

@wyb1 wyb1 left a comment

Choose a reason for hiding this comment

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

Thanks @zanetworker for the PR!
Cilium produces quite a few metrics (>2000 timeseries for my shoot that does nothing). For larger shoots (more nodes) I would assume this would increase as well. I would recommend that we whitelist these metrics (only keep metrics that we need in dashboards) or try to reduce the cardinality.

charts/internal/cilium-monitoring/templates/cilium-monitoring-config.yaml Outdated Show resolved Hide resolved
charts/internal/cilium-monitoring/cilium-dashboard.json Outdated Show resolved Hide resolved
charts/internal/cilium-monitoring/templates/cilium-monitoring-config.yaml Outdated Show resolved Hide resolved
charts/internal/cilium-monitoring/templates/cilium-monitoring-config.yaml Show resolved Hide resolved
charts/internal/cilium-monitoring/templates/cilium-monitoring-config.yaml Outdated Show resolved Hide resolved
charts/internal/cilium-monitoring/templates/cilium-monitoring-config.yaml Outdated Show resolved Hide resolved
charts/internal/cilium-monitoring/templates/cilium-monitoring-config.yaml Show resolved Hide resolved
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 21, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 21, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 24, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 24, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 24, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 24, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 26, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 26, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Sep 1, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Sep 1, 2020
@zanetworker zanetworker requested a review from wyb1 September 1, 2020 14:31
wyb1
wyb1 suggested changes Sep 3, 2020
View reviewed changes
charts/internal/cilium-monitoring/cilium-agent-metrics-dashboard.json Outdated Show resolved Hide resolved
charts/internal/cilium-monitoring/cilium-agent-metrics-dashboard.json Outdated Show resolved Hide resolved
charts/internal/cilium-monitoring/cilium-agent-metrics-dashboard.json Outdated Show resolved Hide resolved
charts/internal/cilium-monitoring/cilium-agent-metrics-dashboard.json Outdated Show resolved Hide resolved
charts/internal/cilium-monitoring/cilium-agent-metrics-dashboard.json Outdated Show resolved Hide resolved
charts/internal/cilium-monitoring/cilium-agent-metrics-dashboard.json Outdated Show resolved Hide resolved
charts/internal/cilium-monitoring/cilium-agent-metrics-dashboard.json Outdated Show resolved Hide resolved
charts/internal/cilium-monitoring/cilium-agent-metrics-dashboard.json Show resolved Hide resolved
charts/internal/cilium-monitoring/cilium-agent-metrics-dashboard.json Outdated Show resolved Hide resolved
charts/internal/cilium-monitoring/cilium-operator-metrics-dashboard.json Outdated Show resolved Hide resolved
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Sep 3, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Sep 3, 2020
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Sep 4, 2020
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Sep 4, 2020
@zanetworker zanetworker requested a review from wyb1 September 4, 2020 13:26
wyb1
wyb1 reviewed Sep 7, 2020
View reviewed changes
Copy link

@wyb1 wyb1 left a comment

Choose a reason for hiding this comment

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

Just some minor nits I still found. Once these are fixed it looks good!

charts/internal/cilium-monitoring/cilium-agent-metrics-dashboard.json Outdated Show resolved Hide resolved
charts/internal/cilium-monitoring/hubble-metrics-dashboard.json Outdated Show resolved Hide resolved
charts/internal/cilium-monitoring/hubble-metrics-dashboard.json Outdated Show resolved Hide resolved
charts/internal/cilium-monitoring/hubble-metrics-dashboard.json Outdated Show resolved Hide resolved
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Sep 7, 2020
@zanetworker zanetworker requested a review from wyb1 September 7, 2020 10:46
@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Sep 7, 2020
wyb1
wyb1 approved these changes Sep 7, 2020
View reviewed changes
Copy link

@wyb1 wyb1 left a comment

Choose a reason for hiding this comment

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

/lgtm

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/review Needs review labels Sep 7, 2020
DockToFuture
DockToFuture approved these changes Sep 7, 2020
View reviewed changes
Copy link
Member

@DockToFuture DockToFuture left a comment

Choose a reason for hiding this comment

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

/lgtm

@DockToFuture DockToFuture merged commit fad7959 into gardener:master Sep 7, 2020
@gardener-robot gardener-robot added priority/3 Priority (lower number equals higher priority) and removed priority/3 Priority (lower number equals higher priority) labels Mar 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ccwienk ccwienk ccwienk left review comments

@vpnachev vpnachev vpnachev left review comments

@DockToFuture DockToFuture DockToFuture approved these changes

@wyb1 wyb1 wyb1 approved these changes

@rfranzke rfranzke Awaiting requested review from rfranzke

Assignees
No one assigned
Labels
area/monitoring Monitoring (including availability monitoring and alerting) related area/networking Networking related kind/enhancement Enhancement, improvement, extension needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) reviewed/lgtm Has approval for merging
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@zanetworker @wyb1 @vpnachev @ccwienk @DockToFuture @gardener-robot-ci-1 @gardener-robot-ci-2 @gardener-robot-ci-3 @gardener-robot