Skip to content

Commit

Permalink
Add option to restore high cardinality for metrics for HTTP server (#…
Browse files Browse the repository at this point in the history 
…7429)

* Add option to restore high cardinality for metrics for HTTP server

This is configured using the Configuration CRD. High cardinality is enabled by default in Dapr 1.13, but shows a warning. The default behavior will be changed in 1.14.

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>

* Added ITs and some fixes

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>

* Fixed tests

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>

* Fixed E2E tests

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>

* Code review

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>

* Code review pt2

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>

* Add IT with default configuration

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>

* fix nits

---------

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
Co-authored-by: Yaron Schneider <schneider.yaron@live.com>
Co-authored-by: Artur Souza <asouza.pro@gmail.com>
Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com>
  • Loading branch information
@ItalyPaleAle @yaron2
@artursouza @dapr-bot
4 people committed Feb 6, 2024
1 parent afe79bc commit 6b7c94f
Show file tree
Hide file tree
Showing 24 changed files with 1,032 additions and 625 deletions.
140 changes: 0 additions & 140 deletions ...ase-1.12/dapr-sidecar-master/0003-Added-patch-for-updated-http_metrics-test-in-1.12.patch
View file

This file was deleted.

20 changes: 20 additions & 0 deletions charts/dapr/crds/configuration.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,16 @@ spec:
properties:
enabled:
type: boolean
http:
description: MetricHTTP defines configuration for metrics for
the HTTP server
properties:
increasedCardinality:
description: 'If true, metrics for the HTTP server are collected
with increased cardinality. The default is true in Dapr 1.13,
but will be changed to false in 1.14+'
type: boolean
type: object
rules:
items:
description: MetricsRule defines configuration options for a
Expand Down Expand Up @@ -286,6 +296,16 @@ spec:
properties:
enabled:
type: boolean
http:
description: MetricHTTP defines configuration for metrics for
the HTTP server
properties:
increasedCardinality:
description: 'If true, metrics for the HTTP server are collected
with increased cardinality. The default is true in Dapr 1.13,
but will be changed to false in 1.14+'
type: boolean
type: object
rules:
items:
description: MetricsRule defines configuration options for a
Expand Down
11 changes: 11 additions & 0 deletions pkg/apis/configuration/v1alpha1/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,9 +212,20 @@ type ZipkinSpec struct {
type MetricSpec struct {
Enabled *bool `json:"enabled"`
// +optional
HTTP *MetricHTTP `json:"http,omitempty"`
// +optional
Rules []MetricsRule `json:"rules,omitempty"`
}

// MetricHTTP defines configuration for metrics for the HTTP server
type MetricHTTP struct {
// If false, metrics for the HTTP server are collected with increased cardinality.
// The default is true in Dapr 1.13, but will be changed to false in 1.14+
// TODO @ItalyPaleAle [MetricsCardinality] Change default in 1.14
// +optional
IncreasedCardinality *bool `json:"increasedCardinality,omitempty"`
}

// MetricsRule defines configuration options for a metric.
type MetricsRule struct {
Name string `json:"name"`
Expand Down
25 changes: 25 additions & 0 deletions pkg/apis/configuration/v1alpha1/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions pkg/channel/http/http_channel.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ func (h *Channel) invokeMethodV1(ctx context.Context, req *invokev1.InvokeMethod
}()

// Emit metric when request is sent
diag.DefaultHTTPMonitoring.ClientRequestStarted(ctx, int64(len(req.Message().GetData().GetValue())))
diag.DefaultHTTPMonitoring.ClientRequestStarted(ctx, channelReq.Method, req.Message().GetMethod(), int64(len(req.Message().GetData().GetValue())))
startRequest := time.Now()

rw := &RWRecorder{
Expand Down Expand Up @@ -261,17 +261,17 @@ func (h *Channel) invokeMethodV1(ctx context.Context, req *invokev1.InvokeMethod
}

if err != nil {
diag.DefaultHTTPMonitoring.ClientRequestCompleted(ctx, strconv.Itoa(http.StatusInternalServerError), contentLength, elapsedMs)
diag.DefaultHTTPMonitoring.ClientRequestCompleted(ctx, channelReq.Method, req.Message().GetMethod(), strconv.Itoa(http.StatusInternalServerError), contentLength, elapsedMs)
return nil, err
}

rsp, err := h.parseChannelResponse(req, resp)
if err != nil {
diag.DefaultHTTPMonitoring.ClientRequestCompleted(ctx, strconv.Itoa(http.StatusInternalServerError), contentLength, elapsedMs)
diag.DefaultHTTPMonitoring.ClientRequestCompleted(ctx, channelReq.Method, req.Message().GetMethod(), strconv.Itoa(http.StatusInternalServerError), contentLength, elapsedMs)
return nil, err
}

diag.DefaultHTTPMonitoring.ClientRequestCompleted(ctx, strconv.Itoa(int(rsp.Status().GetCode())), contentLength, elapsedMs)
diag.DefaultHTTPMonitoring.ClientRequestCompleted(ctx, channelReq.Method, req.Message().GetMethod(), strconv.Itoa(int(rsp.Status().GetCode())), contentLength, elapsedMs)

return rsp, nil
}
Expand Down
63 changes: 45 additions & 18 deletions pkg/config/configuration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
env "github.com/dapr/dapr/pkg/config/env"
operatorv1pb "github.com/dapr/dapr/pkg/proto/operator/v1"
"github.com/dapr/dapr/utils"
"github.com/dapr/kit/logger"
"github.com/dapr/kit/ptr"
)

Expand Down Expand Up @@ -220,9 +221,9 @@ type SelectorField struct {

type TracingSpec struct {
SamplingRate string `json:"samplingRate,omitempty" yaml:"samplingRate,omitempty"`
Stdout bool `json:"stdout,omitempty" yaml:"stdout,omitempty"`
Zipkin *ZipkinSpec `json:"zipkin,omitempty" yaml:"zipkin,omitempty"`
Otel *OtelSpec `json:"otel,omitempty" yaml:"otel,omitempty"`
Stdout bool `json:"stdout,omitempty" yaml:"stdout,omitempty"`
Zipkin *ZipkinSpec `json:"zipkin,omitempty" yaml:"zipkin,omitempty"`
Otel *OtelSpec `json:"otel,omitempty" yaml:"otel,omitempty"`
}

// ZipkinSpec defines Zipkin exporter configurations.
Expand All @@ -232,7 +233,7 @@ type ZipkinSpec struct {

// OtelSpec defines Otel exporter configurations.
type OtelSpec struct {
Protocol string `json:"protocol,omitempty" yaml:"protocol,omitempty"`
Protocol string `json:"protocol,omitempty" yaml:"protocol,omitempty"`
EndpointAddress string `json:"endpointAddress,omitempty" yaml:"endpointAddress,omitempty"`
// Defaults to true
IsSecure *bool `json:"isSecure,omitempty" yaml:"isSecure,omitempty"`
Expand All @@ -248,7 +249,8 @@ func (o OtelSpec) GetIsSecure() bool {
type MetricSpec struct {
// Defaults to true
Enabled *bool `json:"enabled,omitempty" yaml:"enabled,omitempty"`
Rules []MetricsRule `json:"rules,omitempty" yaml:"rules,omitempty"`
HTTP *MetricHTTP `json:"http,omitempty" yaml:"http,omitempty"`
Rules []MetricsRule `json:"rules,omitempty" yaml:"rules,omitempty"`
}

// GetEnabled returns true if metrics are enabled.
Expand All @@ -257,6 +259,25 @@ func (m MetricSpec) GetEnabled() bool {
return m.Enabled == nil || *m.Enabled
}

// GetHTTPIncreasedCardinality returns true if increased cardinality is enabled for HTTP metrics
func (m MetricSpec) GetHTTPIncreasedCardinality(log logger.Logger) bool {
if m.HTTP == nil || m.HTTP.IncreasedCardinality == nil {
// The default is true in Dapr 1.13, but will be changed to false in 1.14+
// TODO @ItalyPaleAle [MetricsCardinality] Change default in 1.14
log.Warn("The default value for 'spec.metric.http.increasedCardinality' will change to 'false' in Dapr 1.14")
return true
}
return *m.HTTP.IncreasedCardinality
}

// MetricHTTP defines configuration for metrics for the HTTP server
type MetricHTTP struct {
// If false, metrics for the HTTP server are collected with increased cardinality.
// The default is true in Dapr 1.13, but will be changed to false in 1.14+
// TODO @ItalyPaleAle [MetricsCardinality] Change default in 1.14
IncreasedCardinality *bool `json:"increasedCardinality,omitempty" yaml:"increasedCardinality,omitempty"`
}

// MetricsRu le defines configuration options for a metric.
type MetricsRule struct {
Name string `json:"name,omitempty" yaml:"name,omitempty"`
Expand All @@ -271,18 +292,18 @@ type MetricLabel struct {

// AppPolicySpec defines the policy data structure for each app.
type AppPolicySpec struct {
AppName string `json:"appId,omitempty" yaml:"appId,omitempty"`
AppName string `json:"appId,omitempty" yaml:"appId,omitempty"`
DefaultAction string `json:"defaultAction,omitempty" yaml:"defaultAction,omitempty"`
TrustDomain string `json:"trustDomain,omitempty" yaml:"trustDomain,omitempty"`
Namespace string `json:"namespace,omitempty" yaml:"namespace,omitempty"`
AppOperationActions []AppOperation `json:"operations,omitempty" yaml:"operations,omitempty"`
TrustDomain string `json:"trustDomain,omitempty" yaml:"trustDomain,omitempty"`
Namespace string `json:"namespace,omitempty" yaml:"namespace,omitempty"`
AppOperationActions []AppOperation `json:"operations,omitempty" yaml:"operations,omitempty"`
}

// AppOperation defines the data structure for each app operation.
type AppOperation struct {
Operation string `json:"name,omitempty" yaml:"name,omitempty"`
Operation string `json:"name,omitempty" yaml:"name,omitempty"`
HTTPVerb []string `json:"httpVerb,omitempty" yaml:"httpVerb,omitempty"`
Action string `json:"action,omitempty" yaml:"action,omitempty"`
Action string `json:"action,omitempty" yaml:"action,omitempty"`
}

// AccessControlSpec is the spec object in ConfigurationSpec.
Expand Down Expand Up @@ -645,14 +666,20 @@ func (c *Configuration) String() string {

// Apply .metrics if set. If not, retain .metric.
func (c *Configuration) sortMetricsSpec() {
if c.Spec.MetricsSpec != nil {
if c.Spec.MetricsSpec.Enabled != nil {
c.Spec.MetricSpec.Enabled = c.Spec.MetricsSpec.Enabled
}
if c.Spec.MetricsSpec == nil {
return
}

if len(c.Spec.MetricsSpec.Rules) > 0 {
c.Spec.MetricSpec.Rules = c.Spec.MetricsSpec.Rules
}
if c.Spec.MetricsSpec.Enabled != nil {
c.Spec.MetricSpec.Enabled = c.Spec.MetricsSpec.Enabled
}

if len(c.Spec.MetricsSpec.Rules) > 0 {
c.Spec.MetricSpec.Rules = c.Spec.MetricsSpec.Rules
}

if c.Spec.MetricsSpec.HTTP != nil {
c.Spec.MetricSpec.HTTP = c.Spec.MetricsSpec.HTTP
}
}

Expand Down

0 comments on commit 6b7c94f

Please sign in to comment.