-
Notifications
You must be signed in to change notification settings - Fork 37
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
add metrics for DNS events #707
Conversation
bc8cbfd
to
4151f2e
Compare
ping @mikenairn @eguzki @maleck13 could I get a review here, when you have a chance? |
1c04299
to
f366404
Compare
} | ||
|
||
func (b *BaseReconciler) DeleteResource(ctx context.Context, obj client.Object, options ...client.DeleteOption) error { | ||
logger, _ := logr.FromContext(ctx) | ||
logger.Info("delete object", "kind", strings.Replace(fmt.Sprintf("%T", obj), "*", "", 1), "name", obj.GetName(), "namespace", obj.GetNamespace()) | ||
return b.Client().Delete(ctx, obj, options...) | ||
if obj.GetDeletionTimestamp() != nil { |
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.
Added a check here, to not reissue delete requests on items already deleting.
Since it was deemed that this was necessary for DNSPolicy, would the same not be true for all policies? Presumably this is intended to be used along some alert or dashboard that signals if CRUD operations are happening too frequently? |
I think this was deemed necessary for DNS Policy as it could result in excessive traffic to the DNS Provider API, which is not a problem with the other policies? |
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.
THis looks a much simpler solution
/lgtm
Emit metrics for:
ping @mikenairn @eguzki
closes #453