-
Notifications
You must be signed in to change notification settings - Fork 93
Add metrics #158
base: master
Are you sure you want to change the base?
Add metrics #158
Changes from all commits
5c2aa3c
5661a3e
92a2c19
25742fb
5fd47bd
ed444df
8a9acc5
539a941
da5e757
38d2ace
cf0add4
f20288d
94f3579
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,10 +7,13 @@ import ( | |
"github.com/spf13/cobra" | ||
"k8s.io/apiserver/pkg/server" | ||
"k8s.io/client-go/rest" | ||
"k8s.io/klog" | ||
|
||
"github.com/jetstack/kube-oidc-proxy/cmd/app/options" | ||
"github.com/jetstack/kube-oidc-proxy/pkg/metrics" | ||
"github.com/jetstack/kube-oidc-proxy/pkg/probe" | ||
"github.com/jetstack/kube-oidc-proxy/pkg/proxy" | ||
"github.com/jetstack/kube-oidc-proxy/pkg/proxy/hooks" | ||
"github.com/jetstack/kube-oidc-proxy/pkg/proxy/tokenreview" | ||
"github.com/jetstack/kube-oidc-proxy/pkg/util" | ||
) | ||
|
@@ -38,6 +41,14 @@ func buildRunCommand(stopCh <-chan struct{}, opts *options.Options) *cobra.Comma | |
return err | ||
} | ||
|
||
// Initialise hooks handler | ||
hooks := hooks.New() | ||
defer func() { | ||
if err := hooks.RunPreShutdownHooks(); err != nil { | ||
klog.Errorf("failed to run shut down hooks: %s", err) | ||
} | ||
}() | ||
|
||
// Here we determine to either use custom or 'in-cluster' client configuration | ||
var err error | ||
var restConfig *rest.Config | ||
|
@@ -57,10 +68,15 @@ func buildRunCommand(stopCh <-chan struct{}, opts *options.Options) *cobra.Comma | |
} | ||
} | ||
|
||
// Initialise metrics handler | ||
metrics := metrics.New() | ||
// Add the metrics server as a shutdown hook | ||
hooks.AddPreShutdownHook("Metrics", metrics.Shutdown) | ||
|
||
// Initialise token reviewer if enabled | ||
var tokenReviewer *tokenreview.TokenReview | ||
if opts.App.TokenPassthrough.Enabled { | ||
tokenReviewer, err = tokenreview.New(restConfig, opts.App.TokenPassthrough.Audiences) | ||
tokenReviewer, err = tokenreview.New(restConfig, metrics, opts.App.TokenPassthrough.Audiences) | ||
if err != nil { | ||
return err | ||
} | ||
|
@@ -85,7 +101,7 @@ func buildRunCommand(stopCh <-chan struct{}, opts *options.Options) *cobra.Comma | |
|
||
// Initialise proxy with OIDC token authenticator | ||
p, err := proxy.New(restConfig, opts.OIDCAuthentication, opts.Audit, | ||
tokenReviewer, secureServingInfo, proxyConfig) | ||
tokenReviewer, secureServingInfo, hooks, metrics, proxyConfig) | ||
if err != nil { | ||
return err | ||
} | ||
|
@@ -97,10 +113,20 @@ func buildRunCommand(stopCh <-chan struct{}, opts *options.Options) *cobra.Comma | |
} | ||
|
||
// Start readiness probe | ||
if err := probe.Run(strconv.Itoa(opts.App.ReadinessProbePort), | ||
fakeJWT, p.OIDCTokenAuthenticator()); err != nil { | ||
readinessHandler, err := probe.Run( | ||
strconv.Itoa(opts.App.ReadinessProbePort), fakeJWT, p.OIDCTokenAuthenticator()) | ||
if err != nil { | ||
return err | ||
} | ||
hooks.AddPreShutdownHook("Readiness Probe", readinessHandler.Shutdown) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: are spaces in hook names okay/normal? |
||
|
||
if len(opts.App.MetricsListenAddress) > 0 { | ||
if err := metrics.Start(opts.App.MetricsListenAddress); err != nil { | ||
return err | ||
} | ||
} else { | ||
klog.Info("metrics listen address empty, disabling serving metrics") | ||
} | ||
|
||
// Run proxy | ||
waitCh, err := p.Run(stopCh) | ||
|
@@ -110,10 +136,6 @@ func buildRunCommand(stopCh <-chan struct{}, opts *options.Options) *cobra.Comma | |
|
||
<-waitCh | ||
|
||
if err := p.RunPreShutdownHooks(); err != nil { | ||
return err | ||
} | ||
|
||
return nil | ||
}, | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,13 +18,21 @@ spec: | |
labels: | ||
app.kubernetes.io/name: {{ include "kube-oidc-proxy.name" . }} | ||
app.kubernetes.io/instance: {{ .Release.Name }} | ||
annotations: | ||
{{- if and .Values.metrics.enabled }} | ||
prometheus.io/scrape: 'true' | ||
prometheus.io/port: '{{ .Values.metrics.port }}' | ||
{{ end }} | ||
spec: | ||
serviceAccountName: {{ include "kube-oidc-proxy.fullname" . }} | ||
containers: | ||
- name: {{ .Chart.Name }} | ||
image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}" | ||
imagePullPolicy: {{ .Values.image.pullPolicy }} | ||
ports: | ||
{{- if and .Values.metrics.enabled }} | ||
- containerPort: {{ .Values.metrics.port }} | ||
{{ end }} | ||
- containerPort: 443 | ||
- containerPort: 8080 | ||
readinessProbe: | ||
|
@@ -74,6 +82,9 @@ spec: | |
{{- range $key, $value := .Values.extraArgs -}} | ||
- "--{{ $key }}={{ $value -}}" | ||
{{ end }} | ||
{{- if and .Values.metrics.enabled }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What this |
||
- "--metrics-serving-address={{ .Values.metrics.address }}:{{ .Values.metrics.port }}" | ||
{{ end }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I think we want this to be |
||
resources: | ||
{{- toYaml .Values.resources | nindent 12 }} | ||
env: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,13 +26,18 @@ spec: | |
metadata: | ||
labels: | ||
app: kube-oidc-proxy | ||
annotations: | ||
prometheus.io/path: /metrics | ||
prometheus.io/port: "80" | ||
prometheus.io/scrape: "true" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This manifest does not have |
||
spec: | ||
serviceAccountName: kube-oidc-proxy | ||
containers: | ||
- image: quay.io/jetstack/kube-oidc-proxy:v0.3.0 | ||
ports: | ||
- containerPort: 443 | ||
- containerPort: 8080 | ||
- containerPort: 80 | ||
readinessProbe: | ||
httpGet: | ||
path: /ready | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
# Metrics | ||
|
||
kube-oidc-proxy exposes a number of Prometheus metrics to give some insights | ||
into how the proxy is performing. These metrics are designed to be used to | ||
better inform how the proxy is behaving, and the general usage from clients, | ||
*_not_* to alert, or otherwise give other security insights. If you are | ||
interested in auditing and access review functionality, please refer to | ||
[auditing](../auditing.md). | ||
|
||
The proxy exposes the following metrics: | ||
|
||
### kube_oidc_proxy_http_client_requests | ||
counter - {http status code, path, remote address} | ||
The number of incoming requests. | ||
|
||
### kube_oidc_proxy_http_client_duration_seconds | ||
histogram - {remote address} | ||
The duration in seconds for incoming client requests to be responded to. | ||
|
||
### kube_oidc_proxy_http_server_requests | ||
counter - {http status code, path, remote address} | ||
The number of outgoing server requests. | ||
|
||
### kube_oidc_proxy_http_server_duration_seconds | ||
histogram - {remote address} | ||
The duration in seconds for outgoing server requests to be responded to. | ||
|
||
### kube_oidc_proxy_token_review_duration_seconds | ||
histogram - {authenticated, http status code, remote address, user} | ||
The duration in seconds for a token review lookup. Authenticated requests are 1, else 0. | ||
|
||
### kube_oidc_proxy_oidc_authentication_count | ||
counter - {authenticated, remote address, user} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also is a username considered PII? Not sure how we want to handle that sort of stuff 😅 |
||
The count for OIDC authentication. Authenticated requests are 1, else 0. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not too sure what |
||
|
||
## Metrics Address | ||
|
||
By default, metrics are exposed on `0.0.0.0:80/metrics`. The flag | ||
`--metrics-serving-address` can be used to change the address, however the | ||
`/metrics` path will remain the same. The metrics address must _not_ conflict | ||
with the proxy and probe addresses. Setting `--metrics-serving-address=""` will | ||
disable the metrics server. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: perhaps provide an example of how to override this? We mention the path component here, which is great, but could confuse things without a concrete example :) |
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.
Is it possible to explicitly set
--metrics-serving-address=""
without it being defaulted to0.0.0.0:80
?