Initial metrics integration #290

sozercan · 2019-11-04T23:44:25Z

Adds initial set up for OpenCensus metrics with Prometheus exporter and implements following metrics to get started:

Audit:

total violations per enforcement action
number of constraints per enforcement action
audit latency

Webhook: (with tags for admission allowed)

request count
request latency

Related #157

Signed-off-by: Sertaç Özercan <sozercan@gmail.com>

maxsmythe

Thanks for doing this! Apologies for the delay, I was heads down on Kubebuilder v2.

I've done a first-pass review though I'd definitely like to take a closer look. I'd also like to get Erik (our metrics expert) to take a look.

maxsmythe · 2019-11-08T04:49:06Z

pkg/metrics/exporter.go

+	return curMetricsExporter
+}
+
+func SetCurMetricsExporter(e view.Exporter) {


Is it necessary to separate this from NewMetricsExporter()? It looks like it's called immediately after NewMetricsExporter() anyway, and NewMetricsExporter() returns a cached object.

updated. let me know what you think

LGTM. I'm on the fence between GetMetricsExporter() and NewMetricsExporter() as the behavior changes depending on when this is called. Maybe just MetricsExporter()?

NVM, NewMetricsExporter() always creates a new one. The name is apt.

Style nit picks: avoid "Get" as a prefix, per https://golang.org/doc/effective_go.html#Getters . Also avoid package stutter, per https://golang.org/doc/effective_go.html#package-names
With those in mind, I suggest New() so that you have exporter.New(). This is coding style, so feel free defer for now.

pkg/metrics/exporter.go

pkg/audit/stats_reporter.go

pkg/webhook/policy.go

pkg/webhook/stats_reporter.go

pkg/audit/stats_reporter.go

ekitson

Hello! My name is Erik and I work with Max on various projects. I have some experience with setting up Prometheus metrics for other projects so he asked me to lend another pair of eyes to this review. I hope that's alright and feel free to ask for any clarification on my comments or suggestions.

pkg/audit/stats_reporter.go

ekitson · 2019-11-08T22:17:42Z

pkg/metrics/exporter.go

+)
+
+var (
+	metricsBackend = flag.String("metricsBackend", "Prometheus", "Backend used for metrics")


Have you considered exporting to the OpenCensus Agent instead? That would allow your users to choose for themselves if they want Prometheus, Jaeger, Datadog, etc.

Thanks, we can add OC Agent as a follow up to limit the scope of this PR

Sounds good. Since you already have the exporter behind a flag, you could just add "open-census-agent" as another value that you handle. Alternatively you could just always initialize both exporters and configure them on separate ports.

pkg/webhook/stats_reporter.go

Signed-off-by: Sertaç Özercan <sozercan@users.noreply.github.com>

pkg/webhook/stats_reporter.go

pkg/webhook/policy.go

pkg/audit/stats_reporter.go

pkg/audit/manager.go

pkg/webhook/stats_reporter.go

Signed-off-by: Sertaç Özercan <sozercan@users.noreply.github.com>

Signed-off-by: Sertaç Özercan <sozercan@gmail.com>

pkg/audit/manager.go

pkg/audit/stats_reporter.go

Signed-off-by: Sertaç Özercan <sozercan@gmail.com>

Signed-off-by: Sertaç Özercan <sozercan@users.noreply.github.com> # Conflicts: # Gopkg.lock # main.go # pkg/audit/manager.go # pkg/webhook/policy.go

ekitson

Looks good! I think these are probably my last suggestions for this PR.

pkg/controller/constraint/constraint_controller.go

pkg/controller/constrainttemplate/constrainttemplate_controller.go

Signed-off-by: Sertaç Özercan <sozercan@gmail.com>

pkg/controller/constraint/constraint_controller.go

maxsmythe · 2019-11-21T08:42:37Z

pkg/controller/constraint/constraint_controller.go

@@ -112,6 +137,16 @@ func (r *ReconcileConstraint) Reconcile(request reconcile.Request) (reconcile.Re
 		return reconcile.Result{}, err
 	}

+	constraintKindName := strings.Join([]string{instance.GetKind(), instance.GetName()}, "/")


When I suggested string joining, I was forgetting that

struct { Kind string Name string }

would also work.

Either method is fine with me.

pkg/controller/constraint/constraint_controller.go

maxsmythe · 2019-11-22T23:07:32Z

Needs rebase and DCO check failing.

pkg/metrics/exporter.go

Signed-off-by: Sertaç Özercan <sozercan@gmail.com>

pkg/metrics/prometheus_exporter.go

maxsmythe · 2019-12-12T01:19:15Z

pkg/webhook/policy.go

@@ -118,6 +131,15 @@ func (h *validationHandler) Handle(ctx context.Context, req admission.Request) a
 		return vResp
 	}

+	var requestResponse requestResponse


pkg/metrics/prometheus_exporter.go

pkg/metrics/exporter.go

Signed-off-by: Sertaç Özercan <sozercan@gmail.com>

pkg/metrics/prometheus_exporter.go

pkg/metrics/exporter.go

pkg/webhook/policy.go

Signed-off-by: Sertaç Özercan <sozercan@users.noreply.github.com>

pkg/metrics/prometheus_exporter.go

maxsmythe

LGTM

Signed-off-by: Sertaç Özercan <sozercan@users.noreply.github.com> Signed-off-by: Sertaç Özercan <sozercan@gmail.com>

Signed-off-by: Sertaç Özercan <sozercan@gmail.com>

) Fixes open-policy-agent#346 Signed-off-by: Max Smythe <smythe@google.com>

* Add capability PSP control aspect to library Signed-off-by: Max Smythe <smythe@google.com> * Update PSP library README Signed-off-by: Max Smythe <smythe@google.com> * Fix template kind Signed-off-by: Max Smythe <smythe@google.com> Signed-off-by: Sertaç Özercan <sozercan@gmail.com>

sozercan requested review from ritazh and maxsmythe November 4, 2019 23:44

add opencensus metrics and prom exporter

2fc30aa

Signed-off-by: Sertaç Özercan <sozercan@gmail.com>

sozercan force-pushed the opencensus-metrics branch from ef5930d to 2fc30aa Compare November 4, 2019 23:46

prom exporter test

34a0d07

Signed-off-by: Sertaç Özercan <sozercan@gmail.com>

sozercan force-pushed the opencensus-metrics branch from fbc5aa5 to 34a0d07 Compare November 5, 2019 00:25

record test

b6eda03

Signed-off-by: Sertaç Özercan <sozercan@gmail.com>

maxsmythe requested changes Nov 8, 2019

View reviewed changes

surenderssm reviewed Nov 8, 2019

View reviewed changes

pkg/audit/stats_reporter.go Outdated Show resolved Hide resolved

ekitson suggested changes Nov 8, 2019

View reviewed changes

address comments

9e68577

Signed-off-by: Sertaç Özercan <sozercan@users.noreply.github.com>

sozercan force-pushed the opencensus-metrics branch from 39b70c4 to 9e68577 Compare November 9, 2019 01:14

update opencensus v0.22.2

45737ba

Signed-off-by: Sertaç Özercan <sozercan@users.noreply.github.com>

ekitson suggested changes Nov 11, 2019

View reviewed changes

sozercan and others added 5 commits November 12, 2019 18:00

address comments

51babd8

Signed-off-by: Sertaç Özercan <sozercan@users.noreply.github.com>

change default port

93d4c96

Signed-off-by: Sertaç Özercan <sozercan@users.noreply.github.com>

fix defer

5a4f350

Signed-off-by: Sertaç Özercan <sozercan@users.noreply.github.com>

fix auditduration name in test

c255e12

Signed-off-by: Sertaç Özercan <sozercan@gmail.com>

fix expected port in test

1e18efc

Signed-off-by: Sertaç Özercan <sozercan@gmail.com>

ekitson reviewed Nov 13, 2019

View reviewed changes

pkg/audit/manager.go Outdated Show resolved Hide resolved

pkg/audit/stats_reporter.go Outdated Show resolved Hide resolved

move total constraints to controller

84af144

Signed-off-by: Sertaç Özercan <sozercan@gmail.com>

sozercan force-pushed the opencensus-metrics branch from 7bd357b to 84af144 Compare November 15, 2019 01:15

Merge branch 'master' into opencensus-metrics

2636829

Signed-off-by: Sertaç Özercan <sozercan@users.noreply.github.com> # Conflicts: # Gopkg.lock # main.go # pkg/audit/manager.go # pkg/webhook/policy.go

ekitson suggested changes Nov 15, 2019

View reviewed changes

pkg/controller/constraint/constraint_controller.go Outdated Show resolved Hide resolved

pkg/controller/constrainttemplate/constrainttemplate_controller.go Outdated Show resolved Hide resolved

sozercan force-pushed the opencensus-metrics branch from 4fe498c to ea07168 Compare November 20, 2019 19:35

update with new map for cache

314594f

Signed-off-by: Sertaç Özercan <sozercan@gmail.com>

sozercan force-pushed the opencensus-metrics branch from ea07168 to 314594f Compare November 20, 2019 19:43

maxsmythe reviewed Nov 21, 2019

View reviewed changes

pkg/controller/constraint/constraint_controller.go Outdated Show resolved Hide resolved

maxsmythe reviewed Dec 11, 2019

View reviewed changes

pkg/metrics/exporter.go Outdated Show resolved Hide resolved

maxsmythe reviewed Dec 11, 2019

View reviewed changes

pkg/metrics/exporter.go Outdated Show resolved Hide resolved

sozercan added 2 commits December 11, 2019 14:12

remove goroutine

73589f7

Signed-off-by: Sertaç Özercan <sozercan@gmail.com>

fix test

8786798

Signed-off-by: Sertaç Özercan <sozercan@gmail.com>

maxsmythe reviewed Dec 12, 2019

View reviewed changes

fix lint

8d3152b

Signed-off-by: Sertaç Özercan <sozercan@gmail.com>

maxsmythe reviewed Dec 12, 2019

View reviewed changes

pkg/metrics/prometheus_exporter.go Outdated Show resolved Hide resolved

maxsmythe reviewed Dec 12, 2019

View reviewed changes

pkg/metrics/exporter.go Show resolved Hide resolved

sozercan commented Dec 12, 2019

View reviewed changes

pkg/webhook/policy.go Outdated Show resolved Hide resolved

handle stop

77aed07

Signed-off-by: Sertaç Özercan <sozercan@users.noreply.github.com>

sozercan force-pushed the opencensus-metrics branch from 447b8c0 to 77aed07 Compare December 12, 2019 20:26

Merge branch 'master' into opencensus-metrics

09979d5

maxsmythe reviewed Dec 13, 2019

View reviewed changes

pkg/metrics/prometheus_exporter.go Outdated Show resolved Hide resolved

sozercan force-pushed the opencensus-metrics branch from e2ba39b to 2fa7a98 Compare December 16, 2019 17:15

maxsmythe approved these changes Dec 16, 2019

View reviewed changes

sozercan force-pushed the opencensus-metrics branch 4 times, most recently from 0d17c64 to 0d4e561 Compare December 17, 2019 21:39

update

9958a2b

Signed-off-by: Sertaç Özercan <sozercan@users.noreply.github.com> Signed-off-by: Sertaç Özercan <sozercan@gmail.com>

sozercan force-pushed the opencensus-metrics branch 4 times, most recently from 972da17 to a246318 Compare December 17, 2019 21:50

sozercan and others added 3 commits December 17, 2019 13:51

update

a9ca652

Signed-off-by: Sertaç Özercan <sozercan@gmail.com>

Use namespace of Pod as namespace for cert secret (open-policy-agent#347

efc07af

) Fixes open-policy-agent#346 Signed-off-by: Max Smythe <smythe@google.com>

sozercan force-pushed the opencensus-metrics branch from a246318 to 5120010 Compare December 17, 2019 21:51

sozercan merged commit 6d1c641 into open-policy-agent:master Dec 17, 2019

sozercan deleted the opencensus-metrics branch December 17, 2019 22:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial metrics integration #290

Initial metrics integration #290

sozercan commented Nov 4, 2019 •

edited

Loading

maxsmythe left a comment

maxsmythe Nov 8, 2019

sozercan Nov 9, 2019

maxsmythe Nov 12, 2019

maxsmythe Nov 12, 2019

ekitson Nov 12, 2019

ekitson left a comment

ekitson Nov 8, 2019

sozercan Nov 11, 2019

ekitson Nov 12, 2019

ekitson left a comment

maxsmythe Nov 21, 2019

maxsmythe commented Nov 22, 2019

maxsmythe Dec 12, 2019

maxsmythe left a comment

Initial metrics integration #290

Initial metrics integration #290

Conversation

sozercan commented Nov 4, 2019 • edited Loading

maxsmythe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ekitson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ekitson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxsmythe commented Nov 22, 2019

Choose a reason for hiding this comment

maxsmythe left a comment

Choose a reason for hiding this comment

sozercan commented Nov 4, 2019 •

edited

Loading