Reduce cardinality in Dapr metrics and add more information to API lo…

…gs (#6919)

* Reduce cardinality in Dapr metrics and add more information to API logs

Fixes #6723

Includes:

1. Drastically reduce cardinality of metrics, especially those emitted by the HTTP server:
   - Also removes the possibility of PII being included in the metrics endpoint.
1. Add more information to API logs for both HTTP and gRPC, including things that are not going to be included in metrics by default anymore:
   - Response status code
   - Response size (for HTTP only & if possible)
   - Latency
3. When `obfuscateURLs` is enabled for API logging, in API logs we now include a more descriptive name for the method rather than the path that was matched in the router. The descriptive names map to the names of the methods in gRPC, for example `SaveState` in place of `POST /state/{storeName}/{name}`. Since gRPC API logs are always, only "obfuscated" (because the params are in the body and not in the "URL"), this makes HTTP and gRPC API logs more consistent too
4. Refactors how tracing and API logging (and the API token auth middleware) get the data from the handler/router:
   - The new approach is a lot more declarative, and less based on heuristics (such as parsing the path from the URL again)
   - The new approach also reduces the number of maps that are allocated and used in each request, which generally contained duplicate information generated in multiple parts of the code
5. For both HTTP and gRPC, the way metadata is added to a tracing span has changed and it's now more declarative:
   - When looking at how tracing spans were composed, the metadata was added in `pkg/diagnostics`, separately from the endpoints. Only a VERY SMALL number of APIs had proper tracing configured (as a matter of fact, see the number of "TODO"'s in this PR), in large part due to the fact that this was in a separate package. The approach also relied a lot on heuristics.
   - For HTTP, now the `Endpoint` struct contains a property to add a function that adds properties to a span for tracing purposes. This lives right next to the handler.
   - For gRPC, messages defined in the protos whose name ends in "Request" now must define an `AppendSpanAttribute` method (this is enforced by a unit test)
6. Update API Allowlisting/Denylisting for HTTP to:
   - Make sure that the same constants can be used for both HTTP and gRPC, especially versions. Right now, versions are in the format "v1.0-alpha1" for HTTP, and "v1alpha1" for gRPC. This PR changes the HTTP versions to be in the format "v1alpha1" too ("v1.0-alpha1" is preserved for bacwkards-compatibility)
   - Improved perf of the HTTP Allowlist/Denylist, especially when using the "new" format (e.g. versions with "v1alpha1"): checking the allowlist is now a simple map lookup rather than an iteration over the entire allowlist.

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

* Added missing workflow beta1 APIs

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

* Added IT for metrics

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

* Update tests/integration/framework/binary/binary.go

Signed-off-by: Alessandro (Ale) Segala <43508+ItalyPaleAle@users.noreply.github.com>

---------

Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
Signed-off-by: Alessandro (Ale) Segala <43508+ItalyPaleAle@users.noreply.github.com>
Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com>

dapr/proto/runtime/v1/dapr.proto

@@ -119,7 +119,7 @@ service Dapr {
   rpc DecryptAlpha1(stream DecryptRequest) returns (stream DecryptResponse);
 
   // Gets metadata of the sidecar
-  rpc GetMetadata (google.protobuf.Empty) returns (GetMetadataResponse) {}
+  rpc GetMetadata (GetMetadataRequest) returns (GetMetadataResponse) {}
 
   // Sets value in extended metadata of the sidecar
   rpc SetMetadata (SetMetadataRequest) returns (google.protobuf.Empty) {}
@@ -187,7 +187,7 @@ service Dapr {
   // Raise an event to a running workflow instance
   rpc RaiseEventWorkflowBeta1 (RaiseEventWorkflowRequest) returns (google.protobuf.Empty) {}
   // Shutdown the sidecar
-  rpc Shutdown (google.protobuf.Empty) returns (google.protobuf.Empty) {}
+  rpc Shutdown (ShutdownRequest) returns (google.protobuf.Empty) {}
 }
 
 // InvokeServiceRequest represents the request message for Service invocation.
@@ -404,15 +404,13 @@ message BulkPublishResponse {
 
 // BulkPublishResponseFailedEntry is the message containing the entryID and error of a failed event in BulkPublishEvent call
 message BulkPublishResponseFailedEntry {
-
   // The response scoped unique ID referring to this message
   string entry_id = 1;
 
   // The error message if any on failure
   string error = 2;
 }
 
-
 // InvokeBindingRequest is the message to send data to output bindings
 message InvokeBindingRequest {
   // The name of the output binding to invoke.
@@ -589,7 +587,12 @@ message InvokeActorResponse {
   bytes data = 1;
 }
 
-// GetMetadataResponse is a message that is returned on GetMetadata rpc call
+// GetMetadataRequest is the message for the GetMetadata request.
+message GetMetadataRequest {
+  // Empty
+}
+
+// GetMetadataResponse is a message that is returned on GetMetadata rpc call.
 message GetMetadataResponse {
   string id = 1;
   repeated ActiveActorsCount active_actors_count = 2 [json_name = "actors"];
@@ -1077,3 +1080,8 @@ message PurgeWorkflowRequest {
   // Name of the workflow component.
   string workflow_component = 2 [json_name = "workflowComponent"];
 }
+
+// ShutdownRequest is the request for Shutdown.
+message ShutdownRequest {
+  // Empty
+}

pkg/acl/acl.go

@@ -172,7 +172,7 @@ func ApplyAccessControlPolicies(ctx context.Context, operation string, httpVerb
 	}
 
 	action, actionPolicy := isOperationAllowedByAccessControlPolicy(spiffeID, operation, httpVerb, isHTTP, acl)
-	emitACLMetrics(spiffeID, actionPolicy, operation, httpVerb.String(), action)
+	emitACLMetrics(spiffeID, actionPolicy, action)
 
 	if !action {
 		errMessage = fmt.Sprintf("access control policy has denied access to id: %s operation: %s verb: %s", spiffeID.URL(), operation, httpVerb)
@@ -182,20 +182,20 @@ func ApplyAccessControlPolicies(ctx context.Context, operation string, httpVerb
 	return action, errMessage
 }
 
-func emitACLMetrics(spiffeID *spiffe.Parsed, actionPolicy, operation, verb string, action bool) {
+func emitACLMetrics(spiffeID *spiffe.Parsed, actionPolicy string, action bool) {
 	if action {
 		switch actionPolicy {
 		case config.ActionPolicyApp:
-			diag.DefaultMonitoring.RequestAllowedByAppAction(spiffeID, operation, verb, action)
+			diag.DefaultMonitoring.RequestAllowedByAppAction(spiffeID)
 		case config.ActionPolicyGlobal:
-			diag.DefaultMonitoring.RequestAllowedByGlobalAction(spiffeID, operation, verb, action)
+			diag.DefaultMonitoring.RequestAllowedByGlobalAction(spiffeID)
 		}
 	} else {
 		switch actionPolicy {
 		case config.ActionPolicyApp:
-			diag.DefaultMonitoring.RequestBlockedByAppAction(spiffeID, operation, verb, action)
+			diag.DefaultMonitoring.RequestBlockedByAppAction(spiffeID)
 		case config.ActionPolicyGlobal:
-			diag.DefaultMonitoring.RequestBlockedByGlobalAction(spiffeID, operation, verb, action)
+			diag.DefaultMonitoring.RequestBlockedByGlobalAction(spiffeID)
 		}
 	}
 }

pkg/channel/http/http_channel.go

@@ -228,7 +228,7 @@ func (h *Channel) invokeMethodV1(ctx context.Context, req *invokev1.InvokeMethod
 	}()
 
 	// Emit metric when request is sent
-	diag.DefaultHTTPMonitoring.ClientRequestStarted(ctx, channelReq.Method, req.Message().Method, int64(len(req.Message().Data.GetValue())))
+	diag.DefaultHTTPMonitoring.ClientRequestStarted(ctx, int64(len(req.Message().Data.GetValue())))
 	startRequest := time.Now()
 
 	var resp *http.Response
@@ -270,17 +270,17 @@ func (h *Channel) invokeMethodV1(ctx context.Context, req *invokev1.InvokeMethod
 	}
 
 	if err != nil {
-		diag.DefaultHTTPMonitoring.ClientRequestCompleted(ctx, channelReq.Method, req.Message().GetMethod(), strconv.Itoa(http.StatusInternalServerError), contentLength, elapsedMs)
+		diag.DefaultHTTPMonitoring.ClientRequestCompleted(ctx, strconv.Itoa(http.StatusInternalServerError), contentLength, elapsedMs)
 		return nil, err
 	}
 
 	rsp, err := h.parseChannelResponse(req, resp)
 	if err != nil {
-		diag.DefaultHTTPMonitoring.ClientRequestCompleted(ctx, channelReq.Method, req.Message().GetMethod(), strconv.Itoa(http.StatusInternalServerError), contentLength, elapsedMs)
+		diag.DefaultHTTPMonitoring.ClientRequestCompleted(ctx, strconv.Itoa(http.StatusInternalServerError), contentLength, elapsedMs)
 		return nil, err
 	}
 
-	diag.DefaultHTTPMonitoring.ClientRequestCompleted(ctx, channelReq.Method, req.Message().GetMethod(), strconv.Itoa(int(rsp.Status().Code)), contentLength, elapsedMs)
+	diag.DefaultHTTPMonitoring.ClientRequestCompleted(ctx, strconv.Itoa(int(rsp.Status().Code)), contentLength, elapsedMs)
 
 	return rsp, nil
 }

pkg/config/configuration.go

@@ -156,18 +156,18 @@ const (
 	APIAccessRuleProtocolGRPC APIAccessRuleProtocol = "grpc"
 )
 
-// GetRulesByProtocol returns a list of APIAccessRule objects filtered by protocol
-func (r APIAccessRules) GetRulesByProtocol(protocol APIAccessRuleProtocol) []APIAccessRule {
-	res := make([]APIAccessRule, len(r))
-	n := 0
+// GetRulesByProtocol returns a list of APIAccessRule objects for a protocol
+// The result is a map where the key is in the format "<version>/<endpoint>"
+func (r APIAccessRules) GetRulesByProtocol(protocol APIAccessRuleProtocol) map[string]struct{} {
+	res := make(map[string]struct{}, len(r))
 	for _, v := range r {
 		//nolint:gocritic
 		if strings.ToLower(string(v.Protocol)) == string(protocol) {
-			res[n] = v
-			n++
+			key := v.Version + "/" + v.Name
+			res[key] = struct{}{}
 		}
 	}
-	return res[:n]
+	return res
 }
 
 type HandlerSpec struct {

pkg/config/configuration_test.go

@@ -22,6 +22,7 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/exp/maps"
 
 	"github.com/dapr/dapr/pkg/buildinfo"
 	"github.com/dapr/kit/ptr"
@@ -481,6 +482,29 @@ func TestSetTracingSpecFromEnv(t *testing.T) {
 	require.True(t, conf.Spec.TracingSpec.Otel.GetIsSecure())
 }
 
+func TestAPIAccessRules(t *testing.T) {
+	config := &Configuration{
+		Spec: ConfigurationSpec{
+			APISpec: &APISpec{
+				Allowed: APIAccessRules{
+					APIAccessRule{Name: "foo", Version: "v1", Protocol: "http"},
+					APIAccessRule{Name: "MyMethod", Version: "v1alpha1", Protocol: "grpc"},
+				},
+				Denied: APIAccessRules{
+					APIAccessRule{Name: "bar", Version: "v1", Protocol: "http"},
+				},
+			},
+		},
+	}
+
+	apiSpec := config.Spec.APISpec
+
+	assert.Equal(t, []string{"v1/foo"}, maps.Keys(apiSpec.Allowed.GetRulesByProtocol(APIAccessRuleProtocolHTTP)))
+	assert.Equal(t, []string{"v1alpha1/MyMethod"}, maps.Keys(apiSpec.Allowed.GetRulesByProtocol(APIAccessRuleProtocolGRPC)))
+	assert.Equal(t, []string{"v1/bar"}, maps.Keys(apiSpec.Denied.GetRulesByProtocol(APIAccessRuleProtocolHTTP)))
+	assert.Empty(t, maps.Keys(apiSpec.Denied.GetRulesByProtocol(APIAccessRuleProtocolGRPC)))
+}
+
 func TestSortMetrics(t *testing.T) {
 	t.Run("metrics overrides metric - enabled false", func(t *testing.T) {
 		config := &Configuration{

pkg/diagnostics/consts/consts.go

@@ -0,0 +1,68 @@
+/*
+Copyright 2023 The Dapr Authors
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+    http://www.apache.org/licenses/LICENSE-2.0
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package consts
+
+import (
+	semconv "go.opentelemetry.io/otel/semconv/v1.10.0"
+)
+
+const (
+	// DaprInternalSpanAttrPrefix is the internal span attribution prefix.
+	// Middleware will not populate it if the span key starts with this prefix.
+	DaprInternalSpanAttrPrefix = "__dapr."
+	// DaprAPISpanNameInternal is the internal attribution, but not populated to span attribution.
+	DaprAPISpanNameInternal = DaprInternalSpanAttrPrefix + "spanname"
+
+	// Span attribute keys
+	// Reference trace semantics https://github.com/open-telemetry/opentelemetry-specification/tree/master/specification/trace/semantic_conventions
+	DBSystemSpanAttributeKey                 = string(semconv.DBSystemKey)
+	DBNameSpanAttributeKey                   = string(semconv.DBNameKey)
+	DBStatementSpanAttributeKey              = string(semconv.DBStatementKey)
+	DBConnectionStringSpanAttributeKey       = string(semconv.DBConnectionStringKey)
+	MessagingSystemSpanAttributeKey          = string(semconv.MessagingSystemKey)
+	MessagingDestinationSpanAttributeKey     = string(semconv.MessagingDestinationKey)
+	MessagingDestinationKindSpanAttributeKey = string(semconv.MessagingDestinationKindKey)
+	GrpcServiceSpanAttributeKey              = string(semconv.RPCServiceKey)
+	NetPeerNameSpanAttributeKey              = string(semconv.NetPeerNameKey)
+
+	DaprAPISpanAttributeKey           = "dapr.api"
+	DaprAPIStatusCodeSpanAttributeKey = "dapr.status_code"
+	DaprAPIProtocolSpanAttributeKey   = "dapr.protocol"
+	DaprAPIInvokeMethod               = "dapr.invoke_method"
+	DaprAPIActorTypeID                = "dapr.actor"
+
+	DaprAPIHTTPSpanAttrValue = "http"
+	DaprAPIGRPCSpanAttrValue = "grpc"
+
+	StateBuildingBlockType   = "state"
+	SecretBuildingBlockType  = "secrets"
+	BindingBuildingBlockType = "bindings"
+	PubsubBuildingBlockType  = "pubsub"
+
+	DaprGRPCServiceInvocationService = "ServiceInvocation"
+	DaprGRPCDaprService              = "Dapr"
+
+	// Keys used in the context's metadata for streaming calls
+	// Note: these keys must always be all-lowercase
+	DaprCallLocalStreamMethodKey = "__dapr_calllocalstream_method"
+)
+
+// MessagingDestinationTopicKind is effectively const, but isn't a const from upstream.
+var MessagingDestinationTopicKind = semconv.MessagingDestinationKindTopic.Value.AsString()
+
+// GrpcAppendSpanAttributesFn is the interface that applies to gRPC requests that add span attributes.
+type GrpcAppendSpanAttributesFn interface {
+	// AppendSpanAttributes appends attributes to the map used for the span in tracing for the gRPC method.
+	AppendSpanAttributes(rpcMethod string, m map[string]string)
+}