diff --git a/injection/sharedmain/main.go b/injection/sharedmain/main.go index 25562e965e..2703e4b7f4 100644 --- a/injection/sharedmain/main.go +++ b/injection/sharedmain/main.go @@ -289,9 +289,6 @@ func MainWithConfig(ctx context.Context, component string, cfg *rest.Config, cto // and pass them in. var wh *webhook.Webhook if len(webhooks) > 0 { - // Register webhook metrics - webhook.RegisterMetrics() - wh, err = webhook.New(ctx, webhooks) if err != nil { logger.Fatalw("Failed to create webhook", zap.Error(err)) diff --git a/webhook/stats_reporter.go b/webhook/stats_reporter.go index 1fe30e8af1..415eb82dfa 100644 --- a/webhook/stats_reporter.go +++ b/webhook/stats_reporter.go @@ -71,13 +71,26 @@ type StatsReporter interface { ReportConversionRequest(request *apixv1.ConversionRequest, response *apixv1.ConversionResponse, d time.Duration) error } +type options struct { + resourceNamespace bool +} + +type Option func(opts *options) + +func WithoutResourceNamespace() Option { + return func(opts *options) { + opts.resourceNamespace = false + } +} + // reporter implements StatsReporter interface type reporter struct { - ctx context.Context + ctx context.Context + opts options } // NewStatsReporter creates a reporter for webhook metrics -func NewStatsReporter() (StatsReporter, error) { +func NewStatsReporter(opts ...Option) (StatsReporter, error) { ctx, err := tag.New( context.Background(), ) @@ -85,13 +98,17 @@ func NewStatsReporter() (StatsReporter, error) { return nil, err } - return &reporter{ctx: ctx}, nil + options := options{resourceNamespace: true} + for _, opt := range opts { + opt(&options) + } + + return &reporter{ctx: ctx, opts: options}, nil } // Captures req count metric, recording the count and the duration func (r *reporter) ReportAdmissionRequest(req *admissionv1.AdmissionRequest, resp *admissionv1.AdmissionResponse, d time.Duration) error { - ctx, err := tag.New( - r.ctx, + mutators := []tag.Mutator{ tag.Insert(requestOperationKey, string(req.Operation)), tag.Insert(kindGroupKey, req.Kind.Group), tag.Insert(kindVersionKey, req.Kind.Version), @@ -99,9 +116,12 @@ func (r *reporter) ReportAdmissionRequest(req *admissionv1.AdmissionRequest, res tag.Insert(resourceGroupKey, req.Resource.Group), tag.Insert(resourceVersionKey, req.Resource.Version), tag.Insert(resourceResourceKey, req.Resource.Resource), - tag.Insert(resourceNamespaceKey, req.Namespace), tag.Insert(admissionAllowedKey, strconv.FormatBool(resp.Allowed)), - ) + } + if r.opts.resourceNamespace { + mutators = append(mutators, tag.Insert(resourceNamespaceKey, req.Namespace)) + } + ctx, err := tag.New(r.ctx, mutators...) if err != nil { return err } @@ -131,7 +151,7 @@ func (r *reporter) ReportConversionRequest(req *apixv1.ConversionRequest, resp * return nil } -func RegisterMetrics() { +func RegisterMetrics(opts ...Option) { tagKeys := []tag.Key{ requestOperationKey, kindGroupKey, @@ -140,13 +160,20 @@ func RegisterMetrics() { resourceGroupKey, resourceVersionKey, resourceResourceKey, - resourceNamespaceKey, admissionAllowedKey, desiredAPIVersionKey, resultStatusKey, resultReasonKey, resultCodeKey} + options := options{resourceNamespace: true} + for _, opt := range opts { + opt(&options) + } + if options.resourceNamespace { + tagKeys = append(tagKeys, resourceNamespaceKey) + } + if err := view.Register( &view.View{ Description: requestCountM.Description(), diff --git a/webhook/stats_reporter_test.go b/webhook/stats_reporter_test.go index e3911b4d4a..ca997b7689 100644 --- a/webhook/stats_reporter_test.go +++ b/webhook/stats_reporter_test.go @@ -70,6 +70,47 @@ func TestWebhookStatsReporterAdmission(t *testing.T) { metricstest.CheckDistributionData(t, requestLatenciesName, expectedTags, 2, shortTime, longTime) } +func TestWebhookStatsReporterAdmissionWithoutNamespaceTag(t *testing.T) { + setup(WithoutResourceNamespace()) + req := &admissionv1.AdmissionRequest{ + UID: "705ab4f5-6393-11e8-b7cc-42010a800002", + Kind: metav1.GroupVersionKind{Group: "autoscaling", Version: "v1", Kind: "Scale"}, + Resource: metav1.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployments"}, + Name: "my-deployment", + Namespace: "my-namespace", + Operation: admissionv1.Update, + } + + resp := &admissionv1.AdmissionResponse{ + UID: req.UID, + Allowed: true, + } + + r, _ := NewStatsReporter(WithoutResourceNamespace()) + + shortTime, longTime := 1100.0, 9100.0 + expectedTags := map[string]string{ + requestOperationKey.Name(): string(req.Operation), + kindGroupKey.Name(): req.Kind.Group, + kindVersionKey.Name(): req.Kind.Version, + kindKindKey.Name(): req.Kind.Kind, + resourceGroupKey.Name(): req.Resource.Group, + resourceVersionKey.Name(): req.Resource.Version, + resourceResourceKey.Name(): req.Resource.Resource, + admissionAllowedKey.Name(): strconv.FormatBool(resp.Allowed), + } + + if err := r.ReportAdmissionRequest(req, resp, time.Duration(shortTime)*time.Millisecond); err != nil { + t.Fatalf("ReportAdmissionRequest() = %v", err) + } + if err := r.ReportAdmissionRequest(req, resp, time.Duration(longTime)*time.Millisecond); err != nil { + t.Fatalf("ReportAdmissionRequest() = %v", err) + } + + metricstest.CheckCountData(t, requestCountName, expectedTags, 2) + metricstest.CheckDistributionData(t, requestLatenciesName, expectedTags, 2, shortTime, longTime) +} + func TestWebhookStatsReporterConversion(t *testing.T) { setup() req := &apixv1.ConversionRequest{ @@ -103,12 +144,12 @@ func TestWebhookStatsReporterConversion(t *testing.T) { metricstest.CheckDistributionData(t, requestLatenciesName, expectedTags, 2, shortTime, longTime) } -func setup() { +func setup(opts ...Option) { resetMetrics() } // opencensus metrics carry global state that need to be reset between unit tests -func resetMetrics() { +func resetMetrics(opts ...Option) { metricstest.Unregister(requestCountName, requestLatenciesName) - RegisterMetrics() + RegisterMetrics(opts...) } diff --git a/webhook/webhook.go b/webhook/webhook.go index eff693e80d..adeb104b3c 100644 --- a/webhook/webhook.go +++ b/webhook/webhook.go @@ -144,6 +144,7 @@ func New( logger := logging.FromContext(ctx) if opts.StatsReporter == nil { + RegisterMetrics() reporter, err := NewStatsReporter() if err != nil { return nil, err