Skip to content
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.

Sign up for GitHub

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

Jump to bottom

promote apiserver_request_total to STABLE status #99788

Merged
merged 1 commit into from Mar 6, 2021
Merged

promote apiserver_request_total to STABLE status #99788

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
46 changes: 7 additions & 39 deletions staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics.go
View file
Open in desktop
Expand Up @@ -50,7 +50,6 @@ type resettableCollector interface {

const (
APIServerComponent string = "apiserver"
OtherContentType string = "other"
OtherRequestMethod string = "other"
)

Expand All @@ -77,14 +76,10 @@ var (
requestCounter = compbasemetrics.NewCounterVec(
&compbasemetrics.CounterOpts{
Name: "apiserver_request_total",
Help: "Counter of apiserver requests broken out for each verb, dry run value, group, version, resource, scope, component, and HTTP response contentType and code.",
StabilityLevel: compbasemetrics.ALPHA,
Help: "Counter of apiserver requests broken out for each verb, dry run value, group, version, resource, scope, component, and HTTP response code.",
StabilityLevel: compbasemetrics.STABLE,
},
// The label_name contentType doesn't follow the label_name convention defined here:
// https://github.com/kubernetes/community/blob/master/contributors/devel/sig-instrumentation/instrumentation.md
// But changing it would break backwards compatibility. Future label_names
// should be all lowercase and separated by underscores.
[]string{"verb", "dry_run", "group", "version", "resource", "subresource", "scope", "component", "contentType", "code"},
[]string{"verb", "dry_run", "group", "version", "resource", "subresource", "scope", "component", "code"},
)
longRunningRequestGauge = compbasemetrics.NewGaugeVec(
&compbasemetrics.GaugeOpts{
Expand Down Expand Up @@ -238,19 +233,6 @@ var (
requestAbortsTotal,
}

// these are the known (e.g. whitelisted/known) content types which we will report for
// request metrics. Any other RFC compliant content types will be aggregated under 'unknown'
knownMetricContentTypes = utilsets.NewString(
"application/apply-patch+yaml",
"application/json",
"application/json-patch+json",
"application/merge-patch+json",
"application/strategic-merge-patch+json",
"application/vnd.kubernetes.protobuf",
"application/vnd.kubernetes.protobuf;stream=watch",
"application/yaml",
"text/plain",
"text/plain;charset=utf-8")
// these are the valid request methods which we report in our metrics. Any other request methods
// will be aggregated under 'unknown'
validRequestMethods = utilsets.NewString(
Expand Down Expand Up @@ -395,7 +377,7 @@ func RecordLongRunning(req *http.Request, requestInfo *request.RequestInfo, comp

// MonitorRequest handles standard transformations for client and the reported verb and then invokes Monitor to record
// a request. verb must be uppercase to be backwards compatible with existing monitoring tooling.
func MonitorRequest(req *http.Request, verb, group, version, resource, subresource, scope, component string, deprecated bool, removedRelease string, contentType string, httpCode, respSize int, elapsed time.Duration) {
func MonitorRequest(req *http.Request, verb, group, version, resource, subresource, scope, component string, deprecated bool, removedRelease string, httpCode, respSize int, elapsed time.Duration) {
// We don't use verb from <requestInfo>, as this may be propagated from
// InstrumentRouteFunc which is registered in installer.go with predefined
// list of verbs (different than those translated to RequestInfo).
Expand All @@ -404,8 +386,7 @@ func MonitorRequest(req *http.Request, verb, group, version, resource, subresour

dryRun := cleanDryRun(req.URL)
elapsedSeconds := elapsed.Seconds()
cleanContentType := cleanContentType(contentType)
requestCounter.WithContext(req.Context()).WithLabelValues(reportedVerb, dryRun, group, version, resource, subresource, scope, component, cleanContentType, codeToString(httpCode)).Inc()
requestCounter.WithContext(req.Context()).WithLabelValues(reportedVerb, dryRun, group, version, resource, subresource, scope, component, codeToString(httpCode)).Inc()
// MonitorRequest happens after authentication, so we can trust the username given by the request
info, ok := request.UserFrom(req.Context())
if ok && info.GetName() == user.APIServerUser {
Expand Down Expand Up @@ -449,7 +430,7 @@ func InstrumentRouteFunc(verb, group, version, resource, subresource, scope, com

routeFunc(req, response)

MonitorRequest(req.Request, verb, group, version, resource, subresource, scope, component, deprecated, removedRelease, delegate.Header().Get("Content-Type"), delegate.Status(), delegate.ContentLength(), time.Since(requestReceivedTimestamp))
MonitorRequest(req.Request, verb, group, version, resource, subresource, scope, component, deprecated, removedRelease, delegate.Status(), delegate.ContentLength(), time.Since(requestReceivedTimestamp))
})
}

Expand All @@ -474,21 +455,8 @@ func InstrumentHandlerFunc(verb, group, version, resource, subresource, scope, c

handler(w, req)

MonitorRequest(req, verb, group, version, resource, subresource, scope, component, deprecated, removedRelease, delegate.Header().Get("Content-Type"), delegate.Status(), delegate.ContentLength(), time.Since(requestReceivedTimestamp))
}
}

// cleanContentType binds the contentType (for metrics related purposes) to a
// bounded set of known/expected content-types.
func cleanContentType(contentType string) string {
normalizedContentType := strings.ToLower(contentType)
if strings.HasSuffix(contentType, " stream=watch") || strings.HasSuffix(contentType, " charset=utf-8") {
normalizedContentType = strings.ReplaceAll(contentType, " ", "")
}
if knownMetricContentTypes.Has(normalizedContentType) {
return normalizedContentType
MonitorRequest(req, verb, group, version, resource, subresource, scope, component, deprecated, removedRelease, delegate.Status(), delegate.ContentLength(), time.Since(requestReceivedTimestamp))
}
return OtherContentType
}

// CleanScope returns the scope of the request.
Expand Down
42 changes: 0 additions & 42 deletions staging/src/k8s.io/apiserver/pkg/endpoints/metrics/metrics_test.go
View file
Open in desktop
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package metrics

import (
"fmt"
"net/http"
"net/url"
"testing"
Expand Down Expand Up @@ -110,44 +109,3 @@ func TestCleanVerb(t *testing.T) {
})
}
}

func TestContentType(t *testing.T) {
testCases := []struct {
rawContentType string
expectedContentType string
}{
{
rawContentType: "application/json",
expectedContentType: "application/json",
},
{
rawContentType: "image/svg+xml",
expectedContentType: "other",
},
{
rawContentType: "text/plain; charset=utf-8",
expectedContentType: "text/plain;charset=utf-8",
},
{
rawContentType: "application/json;foo=bar",
expectedContentType: "other",
},
{
rawContentType: "application/json;charset=hancoding",
expectedContentType: "other",
},
{
rawContentType: "unknownbutvalidtype",
expectedContentType: "other",
},
}

for _, tt := range testCases {
t.Run(fmt.Sprintf("parse %s", tt.rawContentType), func(t *testing.T) {
cleansedContentType := cleanContentType(tt.rawContentType)
if cleansedContentType != tt.expectedContentType {
t.Errorf("Got %s, but expected %s", cleansedContentType, tt.expectedContentType)
}
})
}
}
8 changes: 4 additions & 4 deletions staging/src/k8s.io/apiserver/pkg/server/healthz/healthz_test.go
View file
Open in desktop
Expand Up @@ -252,11 +252,11 @@ func TestMetrics(t *testing.T) {
}

expected := strings.NewReader(`
# HELP apiserver_request_total [ALPHA] Counter of apiserver requests broken out for each verb, dry run value, group, version, resource, scope, component, and HTTP response contentType and code.
# HELP apiserver_request_total [STABLE] Counter of apiserver requests broken out for each verb, dry run value, group, version, resource, scope, component, and HTTP response code.
# TYPE apiserver_request_total counter
apiserver_request_total{code="200",component="",contentType="text/plain;charset=utf-8",dry_run="",group="",resource="",scope="",subresource="/healthz",verb="GET",version=""} 1
apiserver_request_total{code="200",component="",contentType="text/plain;charset=utf-8",dry_run="",group="",resource="",scope="",subresource="/livez",verb="GET",version=""} 1
apiserver_request_total{code="200",component="",contentType="text/plain;charset=utf-8",dry_run="",group="",resource="",scope="",subresource="/readyz",verb="GET",version=""} 1
apiserver_request_total{code="200",component="",dry_run="",group="",resource="",scope="",subresource="/healthz",verb="GET",version=""} 1
apiserver_request_total{code="200",component="",dry_run="",group="",resource="",scope="",subresource="/livez",verb="GET",version=""} 1
apiserver_request_total{code="200",component="",dry_run="",group="",resource="",scope="",subresource="/readyz",verb="GET",version=""} 1
`)
if err := testutil.GatherAndCompare(legacyregistry.DefaultGatherer, expected, "apiserver_request_total"); err != nil {
t.Error(err)
Expand Down
15 changes: 15 additions & 0 deletions test/instrumentation/testdata/stable-metrics-list.yaml
View file
Open in desktop
@@ -0,0 +1,15 @@
- name: apiserver_request_total
help: Counter of apiserver requests broken out for each verb, dry run value, group,
version, resource, scope, component, and HTTP response code.
type: Counter
stabilityLevel: STABLE
labels:
- code
- component
- dry_run
- group
- resource
- scope
- subresource
- verb
- version
24 changes: 12 additions & 12 deletions test/integration/metrics/metrics_test.go
View file
Open in desktop
Expand Up @@ -287,42 +287,42 @@ func TestApiserverMetricsPods(t *testing.T) {
executor: func() {
callOrDie(c.Create(context.TODO(), makePod("foo"), metav1.CreateOptions{}))
},
want: `apiserver_request_total{code="201", component="apiserver", contentType="application/json", dry_run="", group="", resource="pods", scope="resource", subresource="", verb="POST", version="v1"}`,
want: `apiserver_request_total{code="201", component="apiserver", dry_run="", group="", resource="pods", scope="resource", subresource="", verb="POST", version="v1"}`,
},
{
name: "update pod",
executor: func() {
callOrDie(c.Update(context.TODO(), makePod("bar"), metav1.UpdateOptions{}))
},
want: `apiserver_request_total{code="200", component="apiserver", contentType="application/json", dry_run="", group="", resource="pods", scope="resource", subresource="", verb="PUT", version="v1"}`,
want: `apiserver_request_total{code="200", component="apiserver", dry_run="", group="", resource="pods", scope="resource", subresource="", verb="PUT", version="v1"}`,
},
{
name: "update pod status",
executor: func() {
callOrDie(c.UpdateStatus(context.TODO(), makePod("bar"), metav1.UpdateOptions{}))
},
want: `apiserver_request_total{code="200", component="apiserver", contentType="application/json", dry_run="", group="", resource="pods", scope="resource", subresource="status", verb="PUT", version="v1"}`,
want: `apiserver_request_total{code="200", component="apiserver", dry_run="", group="", resource="pods", scope="resource", subresource="status", verb="PUT", version="v1"}`,
},
{
name: "get pod",
executor: func() {
callOrDie(c.Get(context.TODO(), "foo", metav1.GetOptions{}))
},
want: `apiserver_request_total{code="200", component="apiserver", contentType="application/json", dry_run="", group="", resource="pods", scope="resource", subresource="", verb="GET", version="v1"}`,
want: `apiserver_request_total{code="200", component="apiserver", dry_run="", group="", resource="pods", scope="resource", subresource="", verb="GET", version="v1"}`,
},
{
name: "list pod",
executor: func() {
callOrDie(c.List(context.TODO(), metav1.ListOptions{}))
},
want: `apiserver_request_total{code="200", component="apiserver", contentType="application/json", dry_run="", group="", resource="pods", scope="namespace", subresource="", verb="LIST", version="v1"}`,
want: `apiserver_request_total{code="200", component="apiserver", dry_run="", group="", resource="pods", scope="namespace", subresource="", verb="LIST", version="v1"}`,
},
{
name: "delete pod",
executor: func() {
callOrDie(nil, c.Delete(context.TODO(), "foo", metav1.DeleteOptions{}))
},
want: `apiserver_request_total{code="200", component="apiserver", contentType="application/json", dry_run="", group="", resource="pods", scope="resource", subresource="", verb="DELETE", version="v1"}`,
want: `apiserver_request_total{code="200", component="apiserver", dry_run="", group="", resource="pods", scope="resource", subresource="", verb="DELETE", version="v1"}`,
},
} {
t.Run(tc.name, func(t *testing.T) {
Expand Down Expand Up @@ -393,42 +393,42 @@ func TestApiserverMetricsNamespaces(t *testing.T) {
executor: func() {
callOrDie(c.Create(context.TODO(), makeNamespace("foo"), metav1.CreateOptions{}))
},
want: `apiserver_request_total{code="201", component="apiserver", contentType="application/json", dry_run="", group="", resource="namespaces", scope="resource", subresource="", verb="POST", version="v1"}`,
want: `apiserver_request_total{code="201", component="apiserver", dry_run="", group="", resource="namespaces", scope="resource", subresource="", verb="POST", version="v1"}`,
},
{
name: "update namespace",
executor: func() {
callOrDie(c.Update(context.TODO(), makeNamespace("bar"), metav1.UpdateOptions{}))
},
want: `apiserver_request_total{code="200", component="apiserver", contentType="application/json", dry_run="", group="", resource="namespaces", scope="resource", subresource="", verb="PUT", version="v1"}`,
want: `apiserver_request_total{code="200", component="apiserver", dry_run="", group="", resource="namespaces", scope="resource", subresource="", verb="PUT", version="v1"}`,
},
{
name: "update namespace status",
executor: func() {
callOrDie(c.UpdateStatus(context.TODO(), makeNamespace("bar"), metav1.UpdateOptions{}))
},
want: `apiserver_request_total{code="200", component="apiserver", contentType="application/json", dry_run="", group="", resource="namespaces", scope="resource", subresource="status", verb="PUT", version="v1"}`,
want: `apiserver_request_total{code="200", component="apiserver", dry_run="", group="", resource="namespaces", scope="resource", subresource="status", verb="PUT", version="v1"}`,
},
{
name: "get namespace",
executor: func() {
callOrDie(c.Get(context.TODO(), "foo", metav1.GetOptions{}))
},
want: `apiserver_request_total{code="200", component="apiserver", contentType="application/json", dry_run="", group="", resource="namespaces", scope="resource", subresource="", verb="GET", version="v1"}`,
want: `apiserver_request_total{code="200", component="apiserver", dry_run="", group="", resource="namespaces", scope="resource", subresource="", verb="GET", version="v1"}`,
},
{
name: "list namespace",
executor: func() {
callOrDie(c.List(context.TODO(), metav1.ListOptions{}))
},
want: `apiserver_request_total{code="200", component="apiserver", contentType="application/json", dry_run="", group="", resource="namespaces", scope="cluster", subresource="", verb="LIST", version="v1"}`,
want: `apiserver_request_total{code="200", component="apiserver", dry_run="", group="", resource="namespaces", scope="cluster", subresource="", verb="LIST", version="v1"}`,
},
{
name: "delete namespace",
executor: func() {
callOrDie(nil, c.Delete(context.TODO(), "foo", metav1.DeleteOptions{}))
},
want: `apiserver_request_total{code="200", component="apiserver", contentType="application/json", dry_run="", group="", resource="namespaces", scope="resource", subresource="", verb="DELETE", version="v1"}`,
want: `apiserver_request_total{code="200", component="apiserver", dry_run="", group="", resource="namespaces", scope="resource", subresource="", verb="DELETE", version="v1"}`,
},
} {
t.Run(tc.name, func(t *testing.T) {
Expand Down