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.

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 security_mode metric tag to request metrics #14265

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cmd/activator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,8 @@ func main() {
logger.Fatalw("Failed to construct network config", zap.Error(err))
}

// Enable TLS against queue-proxy when internal-encryption is enabled.
tlsEnabled := networkConfig.InternalEncryption
// Enable TLS against queue-proxy when dataplane-trust != disabled.
tlsEnabled := networkConfig.InternalTLSEnabled()

var certCache *certificate.CertCache

Expand Down
16 changes: 13 additions & 3 deletions pkg/activator/config/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"

"go.uber.org/atomic"
netcfg "knative.dev/networking/pkg/config"
"knative.dev/pkg/configmap"
tracingconfig "knative.dev/pkg/tracing/config"
)
Expand All @@ -29,6 +30,7 @@ type cfgKey struct{}
// Config is the configuration for the activator.
type Config struct {
Tracing *tracingconfig.Config
Network *netcfg.Config
}

// FromContext obtains a Config injected into the passed context.
Expand All @@ -51,15 +53,23 @@ func NewStore(logger configmap.Logger, onAfterStore ...func(name string, value i
// Append an update function to run after a ConfigMap has updated to update the
// current state of the Config.
onAfterStore = append(onAfterStore, func(_ string, _ interface{}) {
s.current.Store(&Config{
Tracing: s.UntypedLoad(tracingconfig.ConfigName).(*tracingconfig.Config).DeepCopy(),
})
c := &Config{}
tracing := s.UntypedLoad(tracingconfig.ConfigName)
if tracing != nil {
c.Tracing = tracing.(*tracingconfig.Config).DeepCopy()
}
network := s.UntypedLoad(netcfg.ConfigMapName)
if network != nil {
c.Network = network.(*netcfg.Config).DeepCopy()
}
s.current.Store(c)
})
s.UntypedStore = configmap.NewUntypedStore(
"activator",
logger,
configmap.Constructors{
tracingconfig.ConfigName: tracingconfig.NewTracingConfigFromConfigMap,
netcfg.ConfigMapName: netcfg.NewConfigFromConfigMap,
},
onAfterStore...,
)
Expand Down
14 changes: 14 additions & 0 deletions pkg/activator/config/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
netcfg "knative.dev/networking/pkg/config"
ltesting "knative.dev/pkg/logging/testing"
tracingconfig "knative.dev/pkg/tracing/config"
)
Expand All @@ -35,17 +36,30 @@ var tracingConfig = &corev1.ConfigMap{
},
}

var networkingConfig = &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: netcfg.ConfigMapName,
},
Data: map[string]string{
"dataplane-trust": "Disabled",
},
}

func TestStore(t *testing.T) {
logger := ltesting.TestLogger(t)
store := NewStore(logger)
store.OnConfigChanged(tracingConfig)
store.OnConfigChanged(networkingConfig)

ctx := store.ToContext(context.Background())
cfg := FromContext(ctx)

if got, want := cfg.Tracing.Backend, tracingconfig.None; got != want {
t.Fatalf("Tracing.Backend = %v, want %v", got, want)
}
if got, want := cfg.Network.DataplaneTrust, netcfg.TrustDisabled; got != want {
t.Fatalf("Networking.DataplaneTrust = %v, want %v", got, want)
}

newConfig := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Expand Down
1 change: 1 addition & 0 deletions pkg/activator/handler/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ func revision(namespace, name string) *v1.Revision {
func setupConfigStore(t testing.TB, logger *zap.SugaredLogger) *activatorconfig.Store {
configStore := activatorconfig.NewStore(logger)
configStore.OnConfigChanged(tracingConfig(false))
configStore.OnConfigChanged(networkConfig(false))
return configStore
}

Expand Down
5 changes: 5 additions & 0 deletions pkg/activator/handler/metric_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ import (
"net/http"
"time"

"go.opencensus.io/tag"
pkgmetrics "knative.dev/pkg/metrics"
"knative.dev/serving/pkg/activator"
activatorconfig "knative.dev/serving/pkg/activator/config"
"knative.dev/serving/pkg/apis/serving"
pkghttp "knative.dev/serving/pkg/http"
"knative.dev/serving/pkg/metrics"
Expand All @@ -46,6 +48,9 @@ func (h *MetricHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
reporterCtx, _ := metrics.PodRevisionContext(h.podName, activator.Name,
rev.Namespace, rev.Labels[serving.ServiceLabelKey], rev.Labels[serving.ConfigurationLabelKey], rev.Name)

securityMode := activatorconfig.FromContext(r.Context()).Network.DataplaneTrust
reporterCtx, _ = tag.New(reporterCtx, tag.Upsert(metrics.SecurityMode, string(securityMode)))

start := time.Now()

rr := pkghttp.NewResponseRecorder(w, http.StatusOK)
Expand Down
24 changes: 24 additions & 0 deletions pkg/activator/handler/metric_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,15 @@ import (
"testing"

"go.opencensus.io/resource"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
netcfg "knative.dev/networking/pkg/config"
"knative.dev/pkg/logging"
"knative.dev/pkg/metrics/metricstest"
_ "knative.dev/pkg/metrics/testing"
"knative.dev/serving/pkg/activator"
activatorconfig "knative.dev/serving/pkg/activator/config"
"knative.dev/serving/pkg/apis/serving"
"knative.dev/serving/pkg/metrics"
)
Expand Down Expand Up @@ -108,13 +113,19 @@ func TestRequestMetricHandler(t *testing.T) {
metrics.LabelContainerName: activator.Name,
metrics.LabelResponseCode: strconv.Itoa(labelCode),
metrics.LabelResponseCodeClass: strconv.Itoa(labelCode/100) + "xx",
metrics.LabelSecurityMode: string(netcfg.TrustDisabled),
}

metricstest.AssertMetric(t, metricstest.IntMetric(requestCountM.Name(), 1, wantTags).WithResource(wantResource))
metricstest.AssertMetricExists(t, responseTimeInMsecM.Name())
}()

cm := networkConfig(false)

reqCtx := WithRevisionAndID(context.Background(), rev, types.NamespacedName{Namespace: testNamespace, Name: testRevName})
configStore := activatorconfig.NewStore(logging.FromContext(reqCtx))
configStore.OnConfigChanged(cm)
reqCtx = configStore.ToContext(reqCtx)
handler.ServeHTTP(resp, req.WithContext(reqCtx))
})
}
Expand Down Expand Up @@ -148,3 +159,16 @@ func BenchmarkMetricHandler(b *testing.B) {
})
})
}

func networkConfig(internalTLSEnabled bool) *corev1.ConfigMap {
cm := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: netcfg.ConfigMapName,
},
Data: map[string]string{},
}
if internalTLSEnabled {
cm.Data["dataplane-trust"] = string(netcfg.TrustEnabled)
}
return cm
}
6 changes: 3 additions & 3 deletions pkg/activator/handler/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,19 +57,19 @@ func register() {
Description: "Concurrent requests that are routed to Activator",
Measure: requestConcurrencyM,
Aggregation: view.LastValue(),
TagKeys: []tag.Key{metrics.PodKey, metrics.ContainerKey},
TagKeys: []tag.Key{metrics.PodKey, metrics.ContainerKey, metrics.SecurityMode},
},
&view.View{
Description: "The number of requests that are routed to Activator",
Measure: requestCountM,
Aggregation: view.Count(),
TagKeys: []tag.Key{metrics.PodKey, metrics.ContainerKey, metrics.ResponseCodeKey, metrics.ResponseCodeClassKey},
TagKeys: []tag.Key{metrics.PodKey, metrics.ContainerKey, metrics.ResponseCodeKey, metrics.ResponseCodeClassKey, metrics.SecurityMode},
},
&view.View{
Description: "The response time in millisecond",
Measure: responseTimeInMsecM,
Aggregation: defaultLatencyDistribution,
TagKeys: []tag.Key{metrics.PodKey, metrics.ContainerKey, metrics.ResponseCodeKey, metrics.ResponseCodeClassKey},
TagKeys: []tag.Key{metrics.PodKey, metrics.ContainerKey, metrics.ResponseCodeKey, metrics.ResponseCodeClassKey, metrics.SecurityMode},
},
); err != nil {
panic(err)
Expand Down
4 changes: 4 additions & 0 deletions pkg/metrics/key.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ const (
// LabelResponseTimeout is the label timeout.
LabelResponseTimeout = metricskey.LabelResponseTimeout

// LabelSecurityMode is the label for Security Mode Knative is configured to use (see dataplane-trust in config-networking).
LabelSecurityMode = "security_mode"

// ValueUnknown is the default value if the field is unknown, e.g. project will be unknown if Knative
// is not running on GKE.
ValueUnknown = metricskey.ValueUnknown
Expand All @@ -77,4 +80,5 @@ var (
ResponseCodeKey = tag.MustNewKey(LabelResponseCode)
ResponseCodeClassKey = tag.MustNewKey(LabelResponseCodeClass)
RouteTagKey = tag.MustNewKey(LabelRouteTag)
SecurityMode = tag.MustNewKey(LabelSecurityMode)
)
21 changes: 13 additions & 8 deletions pkg/queue/request_metric.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"go.opencensus.io/stats/view"
"go.opencensus.io/tag"

netcfg "knative.dev/networking/pkg/config"
netheader "knative.dev/networking/pkg/http/header"
pkgmetrics "knative.dev/pkg/metrics"
pkghttp "knative.dev/serving/pkg/http"
Expand Down Expand Up @@ -62,8 +63,9 @@ var (
)

type requestMetricsHandler struct {
next http.Handler
statsCtx context.Context
next http.Handler
statsCtx context.Context
securityMode netcfg.Trust
}

type appRequestMetricsHandler struct {
Expand All @@ -74,8 +76,8 @@ type appRequestMetricsHandler struct {

// NewRequestMetricsHandler creates an http.Handler that emits request metrics.
func NewRequestMetricsHandler(next http.Handler,
ns, service, config, rev, pod string) (http.Handler, error) {
keys := []tag.Key{metrics.PodKey, metrics.ContainerKey, metrics.ResponseCodeKey, metrics.ResponseCodeClassKey, metrics.RouteTagKey}
ns, service, config, rev, pod string, securityMode netcfg.Trust) (http.Handler, error) {
keys := []tag.Key{metrics.PodKey, metrics.ContainerKey, metrics.ResponseCodeKey, metrics.ResponseCodeClassKey, metrics.RouteTagKey, metrics.SecurityMode}
if err := pkgmetrics.RegisterResourceView(
&view.View{
Description: "The number of requests that are routed to queue-proxy",
Expand All @@ -99,15 +101,18 @@ func NewRequestMetricsHandler(next http.Handler,
}

return &requestMetricsHandler{
next: next,
statsCtx: ctx,
next: next,
statsCtx: ctx,
securityMode: securityMode,
}, nil
}

func (h *requestMetricsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
rr := pkghttp.NewResponseRecorder(w, http.StatusOK)
startTime := time.Now()

ctx, _ := tag.New(h.statsCtx, tag.Upsert(metrics.SecurityMode, string(h.securityMode)))

defer func() {
// Filter probe requests for revision metrics.
if netheader.IsProbe(r) {
Expand All @@ -119,13 +124,13 @@ func (h *requestMetricsHandler) ServeHTTP(w http.ResponseWriter, r *http.Request
latency := time.Since(startTime)
routeTag := GetRouteTagNameFromRequest(r)
if err != nil {
ctx := metrics.AugmentWithResponseAndRouteTag(h.statsCtx,
ctx = metrics.AugmentWithResponseAndRouteTag(ctx,
http.StatusInternalServerError, routeTag)
pkgmetrics.RecordBatch(ctx, requestCountM.M(1),
responseTimeInMsecM.M(float64(latency.Milliseconds())))
panic(err)
}
ctx := metrics.AugmentWithResponseAndRouteTag(h.statsCtx,
ctx = metrics.AugmentWithResponseAndRouteTag(ctx,
rr.ResponseCode, routeTag)
pkgmetrics.RecordBatch(ctx, requestCountM.M(1),
responseTimeInMsecM.M(float64(latency.Milliseconds())))
Expand Down
20 changes: 12 additions & 8 deletions pkg/queue/request_metric_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,23 @@ import (
"knative.dev/pkg/metrics/metricstest"
"knative.dev/serving/pkg/metrics"

netcfg "knative.dev/networking/pkg/config"
_ "knative.dev/pkg/metrics/testing"
)

const targetURI = "http://example.com"

func TestNewRequestMetricsHandlerFailure(t *testing.T) {
t.Cleanup(reset)
if _, err := NewRequestMetricsHandler(nil /*next*/, "a", "b", "c", "d", "shøüld fail"); err == nil {
if _, err := NewRequestMetricsHandler(nil /*next*/, "a", "b", "c", "d", "shøüld fail", netcfg.TrustDisabled); err == nil {
t.Error("Should get error when tag value is not ascii")
}
}

func TestRequestMetricsHandler(t *testing.T) {
defer reset()
baseHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})
handler, err := NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod")
handler, err := NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod", netcfg.TrustDisabled)
if err != nil {
t.Fatal("Failed to create handler:", err)
}
Expand All @@ -57,6 +58,7 @@ func TestRequestMetricsHandler(t *testing.T) {
metrics.LabelResponseCode: "200",
metrics.LabelResponseCodeClass: "2xx",
"route_tag": disabledTagName,
metrics.LabelSecurityMode: string(netcfg.TrustDisabled),
}
wantResource := &resource.Resource{
Type: "knative_revision",
Expand All @@ -81,7 +83,7 @@ func TestRequestMetricsHandler(t *testing.T) {
func TestRequestMetricsHandlerWithEnablingTagOnRequestMetrics(t *testing.T) {
defer reset()
baseHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})
handler, err := NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod")
handler, err := NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod", netcfg.TrustDisabled)
if err != nil {
t.Fatal("Failed to create handler:", err)
}
Expand All @@ -98,6 +100,7 @@ func TestRequestMetricsHandlerWithEnablingTagOnRequestMetrics(t *testing.T) {
metrics.LabelResponseCode: "200",
metrics.LabelResponseCodeClass: "2xx",
metrics.LabelRouteTag: "test-tag",
metrics.LabelSecurityMode: string(netcfg.TrustDisabled),
}
wantResource := &resource.Resource{
Type: "knative_revision",
Expand All @@ -113,23 +116,23 @@ func TestRequestMetricsHandlerWithEnablingTagOnRequestMetrics(t *testing.T) {

// Testing for default route
reset()
handler, _ = NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod")
handler, _ = NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod", netcfg.TrustDisabled)
req.Header.Del(netheader.RouteTagKey)
req.Header.Set(netheader.DefaultRouteKey, "true")
handler.ServeHTTP(resp, req)
wantTags["route_tag"] = defaultTagName
metricstest.AssertMetric(t, metricstest.IntMetric("request_count", 1, wantTags).WithResource(wantResource))

reset()
handler, _ = NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod")
handler, _ = NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod", netcfg.TrustDisabled)
req.Header.Set(netheader.RouteTagKey, "test-tag")
req.Header.Set(netheader.DefaultRouteKey, "true")
handler.ServeHTTP(resp, req)
wantTags["route_tag"] = undefinedTagName
metricstest.AssertMetric(t, metricstest.IntMetric("request_count", 1, wantTags).WithResource(wantResource))

reset()
handler, _ = NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod")
handler, _ = NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod", netcfg.TrustDisabled)
req.Header.Set(netheader.RouteTagKey, "test-tag")
req.Header.Set(netheader.DefaultRouteKey, "false")
handler.ServeHTTP(resp, req)
Expand All @@ -149,7 +152,7 @@ func TestRequestMetricsHandlerPanickingHandler(t *testing.T) {
baseHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
panic("no!")
})
handler, err := NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod")
handler, err := NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod", netcfg.TrustDisabled)
if err != nil {
t.Fatal("Failed to create handler:", err)
}
Expand All @@ -166,6 +169,7 @@ func TestRequestMetricsHandlerPanickingHandler(t *testing.T) {
metrics.LabelResponseCode: "500",
metrics.LabelResponseCodeClass: "5xx",
"route_tag": disabledTagName,
metrics.LabelSecurityMode: string(netcfg.TrustDisabled),
}
wantResource := &resource.Resource{
Type: "knative_revision",
Expand Down Expand Up @@ -292,7 +296,7 @@ func TestAppRequestMetricsHandler(t *testing.T) {

func BenchmarkRequestMetricsHandler(b *testing.B) {
baseHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})
handler, _ := NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod")
handler, _ := NewRequestMetricsHandler(baseHandler, "ns", "svc", "cfg", "rev", "pod", netcfg.TrustDisabled)
req := httptest.NewRequest(http.MethodPost, "http://example.com", nil)

b.Run("sequential", func(b *testing.B) {
Expand Down
Loading
Loading