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

Scrape extension pods in seed #428

Merged
merged 3 commits into from
Oct 18, 2021
Merged

Scrape extension pods in seed #428

merged 3 commits into from
Oct 18, 2021

Conversation

timebertt
Copy link
Member

@timebertt timebertt commented Oct 11, 2021 •
edited
Loading

How to categorize this PR?

/area monitoring
/kind enhancement
/platform aws

What this PR does / why we need it:

Make use of the newly introduced seed prometheus and let it scrape the metrics exposed by extension pods running on seeds (see monitoring docs).

Which issue(s) this PR fixes:
Part of #427

Special notes for your reviewer:

  • currently the metrics port is not configurable, so I'm relying on the controller-runtime default (8080)
  • some examples of what we could already observe with these metrics:
    • reconciliation duration per controller:
      image
    • request rate with conflicts:
      image
    • request latency per verb observed from the client-side:
      image

Release note:

Metrics exposed by `provider-aws` pods running on Seeds are now available in the Seed monitoring stack.
Scraping can be enabled/disabled via the `metrics.enabled` value in the `Controller{Registration,Deployment}` (enabled by default).

@timebertt timebertt requested review from a team as code owners October 11, 2021 09:13
@gardener-robot gardener-robot added area/monitoring Monitoring (including availability monitoring and alerting) related kind/enhancement Enhancement, improvement, extension platform/aws Amazon web services platform/infrastructure needs/review Needs review size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) labels Oct 11, 2021
@timebertt
Copy link
Member Author

/invite @wyb1 @istvanballok

@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 Oct 11, 2021
@gardener-robot-ci-1 gardener-robot-ci-1 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 Oct 11, 2021
@timebertt timebertt mentioned this pull request Oct 11, 2021
Open
2 tasks
@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 Oct 11, 2021
@timebertt timebertt mentioned this pull request Oct 13, 2021
Closed
76 tasks
@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 Oct 13, 2021
@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 Oct 13, 2021
@timebertt
Copy link
Member Author

@dkistner @wyb1 @istvanballok thanks for your opinions.
I reworked the PR to include a switch in the chart values. PTAL :)

@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 Oct 14, 2021
@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 Oct 14, 2021
@timebertt
Copy link
Member Author

I pushed another commit simplifying the values structure. PTAL :)

@timebertt timebertt added the merge/squash Should be merged via 'Squash and merge' label Oct 14, 2021
@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 Oct 15, 2021
@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 Oct 15, 2021
dkistner
dkistner previously approved these changes Oct 15, 2021
View reviewed changes
Copy link
Member

@dkistner dkistner 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 Oct 15, 2021
ialidzhikov
ialidzhikov previously approved these changes Oct 15, 2021
View reviewed changes
Copy link
Member

@ialidzhikov ialidzhikov left a comment

Choose a reason for hiding this comment

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

/lgtm
/squash

@dkistner
Copy link
Member

/rebase

@gardener-robot gardener-robot added needs/rebase Needs git rebase and removed reviewed/lgtm Has approval for merging labels Oct 15, 2021
@timebertt timebertt dismissed stale reviews from ialidzhikov and dkistner via 881cb54 October 18, 2021 05:58
@gardener-robot gardener-robot added the needs/review Needs review label Oct 18, 2021
@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 Oct 18, 2021
@timebertt
Copy link
Member Author

Done.
/remove needs/rebase

@gardener-robot gardener-robot removed the needs/rebase Needs git rebase label Oct 18, 2021
@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 Oct 18, 2021
@dkistner dkistner merged commit 88a0cf6 into gardener:master Oct 18, 2021
@timebertt timebertt mentioned this pull request Oct 26, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dkistner dkistner dkistner approved these changes

@ialidzhikov ialidzhikov ialidzhikov left review comments

@istvanballok istvanballok Awaiting requested review from istvanballok

@wyb1 wyb1 Awaiting requested review from wyb1

Assignees
No one assigned
Labels
area/monitoring Monitoring (including availability monitoring and alerting) related kind/enhancement Enhancement, improvement, extension merge/squash Should be merged via 'Squash and merge' needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/review Needs review platform/aws Amazon web services platform/infrastructure size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@timebertt @dkistner @wyb1 @ialidzhikov @gardener-robot-ci-1 @gardener-robot-ci-2 @gardener-robot-ci-3 @gardener-robot