-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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.
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
Changes to add count of A records of source and registry as metrics #2422
Changes to add count of A records of source and registry as metrics #2422
Conversation
Changes to add A records of source and registry as metrics. The existing metrics - external_dns_registry_endpoints_total & external_dns_source_endpoints_total gives a cumulative count of records all records (A, SRV, CNAME, TXT). We have a metric - external_dns_controller_verified_records which gives the intersection of A records between source & regsitry. However, in order to determined how many records are yet to be synced to registry, we need count of total A records in registry & source.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adityask2, Raffo The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Was there some reason this didn't use a vector label for the type, eg |
@timbunce No specific reason. The current name appeared to convey the required info. |
Thanks. It does. My concern is that it doesn't follow best practice in metric naming. That'll become more of a concern if/when metrics for more answer types are desired (eg CNAME, AAAA, etc). Adding two more metrics for each new type places extra load on the metrics system. It's also much harder to query, e.g. "sum across record types excluding AAAA" would simply be I may be adding metrics myself before long. In which case I'd add new vector based metrics and mark these as deprecated. That way anyone using them can migrate to the new ones in their own time. Thanks again @adityask2. |
Changes to count of add A records of registry and source as gauge metric
external_dns_registry_a_records, external_dns_source_a_records
Description
The existing metrics -
external_dns_registry_endpoints_total & external_dns_source_endpoints_total
gives a cumulative count of records all records (A, SRV, CNAME, TXT).We have a metric - external_dns_controller_verified_records which gives the intersection of A records between source & regsitry. However, in order to determined how many records are yet to be synced to registry, we need count of total A records in registry & source.
Testing - Tested the changes.
Checklist