Skip to content

feat(controller)!: publish metrics for all supported endpoint types #5516

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
Merged
k8s-ci-robot merged 14 commits into from
Jun 13, 2025
Merged

feat(controller)!: publish metrics for all supported endpoint types #5516

k8s-ci-robot merged 14 commits into from
Jun 13, 2025

Conversation

@ivankatliarchuk
Copy link
Member

@ivankatliarchuk ivankatliarchuk commented Jun 10, 2025 •
edited
Loading

What does it do ?

Add more metrics for supported record types. At the moment we only have metrics for A and AAAA records

Motivation

  • enchanted visibility, simplify SRE day-to-day

Based on prometheus best practices https://prometheus.io/docs/practices/naming/

Replace metrics with prefix and instead use metrics patronised with endpoint type labels

Note: I think this should be added to release notes

Metrics no longer exists:

  • verified_a_records
  • verified_aaaa_records
  • a_records {source|registry)
  • aaaa_recoreds (source|registry)

Tested on real cluster

Screenshot 2025-06-12 at 10 22 15

Follow-up

More

  • Yes, this PR title follows Conventional Commits
  • Yes, I added unit tests
  • Yes, I updated end user documentation accordingly

@k8s-ci-robot k8s-ci-robot requested a review from mloiseleur June 10, 2025 07:37
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 10, 2025
@k8s-ci-robot k8s-ci-robot requested a review from szuecs June 10, 2025 07:37
@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jun 10, 2025
@ivankatliarchuk ivankatliarchuk changed the title feat(controller): add more metrics for all supported endpoint types feat(controller): publish metrics for all supported endpoint types Jun 10, 2025
@ivankatliarchuk ivankatliarchuk force-pushed the feat-push-more-metrics branch from 55e3ac3 to 41ba880 Compare June 10, 2025 07:39
@ivankatliarchuk ivankatliarchuk force-pushed the feat-push-more-metrics branch from 41ba880 to b6f04fc Compare June 10, 2025 08:01
@ivankatliarchuk
Copy link
Member Author

ivankatliarchuk commented Jun 10, 2025

Hi @AndrewCharlesHay do you have a capacity to review this PR?

mloiseleur
mloiseleur reviewed Jun 10, 2025
View reviewed changes
mloiseleur
mloiseleur reviewed Jun 10, 2025
View reviewed changes
@ivankatliarchuk ivankatliarchuk marked this pull request as draft June 11, 2025 07:20
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 11, 2025
@AndrewCharlesHay
Copy link
Contributor

AndrewCharlesHay commented Jun 11, 2025

Other than the list order it looks good to me!

@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jun 12, 2025
@ivankatliarchuk
feat(controller): add cardinality and labels for records metrics
755d321 
Signed-off-by: ivan katliarchuk <[email protected]>
@ivankatliarchuk
feat(controller): add cardinality and labels for records metrics
7e189d4 
Signed-off-by: ivan katliarchuk <[email protected]>
@ivankatliarchuk
feat(controller): add cardinality and labels for records metrics
e57243b 
Signed-off-by: ivan katliarchuk <[email protected]>
@ivankatliarchuk
feat(controller): add cardinality and labels for records metrics
79d8741 
Signed-off-by: ivan katliarchuk <[email protected]>
@ivankatliarchuk
Merge branch 'master' into feat-push-more-metrics
55431bc 
* master:
  fix(chart): update schema with latest plugin release (kubernetes-sigs#5510)
  chore(deps): bump the dev-dependencies group with 10 updates (kubernetes-sigs#5519)
  feat: Plan normalizeDNSName convert Unicode to ASCII (kubernetes-sigs#5049)
@ivankatliarchuk ivankatliarchuk marked this pull request as ready for review June 12, 2025 10:52
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 12, 2025
@k8s-ci-robot k8s-ci-robot requested a review from mloiseleur June 12, 2025 10:52
mloiseleur
mloiseleur reviewed Jun 12, 2025
View reviewed changes
@mloiseleur mloiseleur changed the title feat(controller): publish metrics for all supported endpoint types feat(controller)!: publish metrics for all supported endpoint types Jun 12, 2025
@mloiseleur mloiseleur linked an issue Jun 12, 2025 that may be closed by this pull request
Metrics for record creation/deletion #962
Closed
mloiseleur
mloiseleur reviewed Jun 12, 2025
View reviewed changes
mloiseleur
mloiseleur reviewed Jun 12, 2025
View reviewed changes
mloiseleur
mloiseleur reviewed Jun 12, 2025
View reviewed changes
controller/controller.go Outdated
Name: "verified_aaaa_records",
Help: "Number of DNS AAAA-records that exists both in source and registry.",
Name: "verified_records",
Help: "Number of DNS that exists both in source and registry (vector).",
Copy link
Collaborator

@mloiseleur mloiseleur Jun 12, 2025

Choose a reason for hiding this comment

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

Suggested change
Help: "Number of DNS that exists both in source and registry (vector).",
Help: "Number of DNS records that exists both in source and registry (vector).",

mloiseleur
mloiseleur reviewed Jun 12, 2025
View reviewed changes
mloiseleur
mloiseleur reviewed Jun 12, 2025
View reviewed changes
@ivankatliarchuk
Copy link
Member Author

ivankatliarchuk commented Jun 13, 2025

/retest-required

@mloiseleur
Copy link
Collaborator

mloiseleur commented Jun 13, 2025 •
edited
Loading

Screenshot 2025-06-12 at 10 22 15

The record_type looks good.

The other labels (arch,go_version and version) do not seem related to this metrics. Those kinds of labels are more expected on a metric like external_dns_build_info (see here or here for instance).

But maybe it's not related to this PR 🤔

@ivankatliarchuk
Copy link
Member Author

ivankatliarchuk commented Jun 13, 2025

Screenshot 2025-06-12 at 10 22 15

The record_type looks good.

The other labels (arch,go_version and version) do not seem related to this metrics. Those kinds of labels are more expected on a metric like external_dns_build_info (see here or here for instance).

But maybe it's not related to this PR 🤔

Yeah. I'll have a look in follow-up

@ivankatliarchuk
Copy link
Member Author

ivankatliarchuk commented Jun 13, 2025

/test all

@ivankatliarchuk
Merge branch 'master' into feat-push-more-metrics
726c444 
* master:
  chore: Release chart for v0.17.0 (kubernetes-sigs#5479)
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 13, 2025
@ivankatliarchuk
Copy link
Member Author

ivankatliarchuk commented Jun 13, 2025

/test all

@ivankatliarchuk
fix rebase
836f65b 
Signed-off-by: ivan katliarchuk <[email protected]>
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 13, 2025
@ivankatliarchuk
fix rebase
295084d 
Signed-off-by: ivan katliarchuk <[email protected]>
@mloiseleur
Copy link
Collaborator

mloiseleur commented Jun 13, 2025

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 13, 2025
@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Jun 13, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mloiseleur

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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 13, 2025
@k8s-ci-robot k8s-ci-robot merged commit d63bfb3 into kubernetes-sigs:master Jun 13, 2025
14 checks passed
@ivankatliarchuk ivankatliarchuk deleted the feat-push-more-metrics branch June 13, 2025 09:49
@bilenkis bilenkis mentioned this pull request Jul 3, 2025
Closed
@ivankatliarchuk
Copy link
Member Author

ivankatliarchuk commented Jul 4, 2025

Fixed #962

@ivankatliarchuk ivankatliarchuk mentioned this pull request Jul 4, 2025
Closed
@ivankatliarchuk ivankatliarchuk mentioned this pull request Jul 13, 2025
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mloiseleur mloiseleur mloiseleur left review comments

@szuecs szuecs Awaiting requested review from szuecs

Assignees

@mloiseleur mloiseleur

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. docs lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Metrics for record creation/deletion

4 participants

@ivankatliarchuk @AndrewCharlesHay @mloiseleur @k8s-ci-robot