Reduce cardinality in Dapr metrics and add more information to API logs #6723
Assignees
Labels
Milestone
Comments
|
About this issue, I found that the docs has been updated. Several details are still not very clear:
Regarding point 2: |
ItalyPaleAle
added a commit
to ItalyPaleAle/dapr
that referenced
this issue
Aug 4, 2023
Fixes dapr#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>
ItalyPaleAle
added a commit
to ItalyPaleAle/dapr
that referenced
this issue
Aug 4, 2023
Fixes dapr#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>
ItalyPaleAle
added a commit
to ItalyPaleAle/dapr
that referenced
this issue
Aug 15, 2023
Fixes dapr#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>
ItalyPaleAle
added a commit
to ItalyPaleAle/dapr
that referenced
this issue
Sep 5, 2023
Fixes dapr#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>
ItalyPaleAle
added a commit
to ItalyPaleAle/dapr
that referenced
this issue
Sep 13, 2023
Fixes dapr#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>
ItalyPaleAle
added a commit
to ItalyPaleAle/dapr
that referenced
this issue
Sep 25, 2023
Fixes dapr#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>
ItalyPaleAle
added a commit
to ItalyPaleAle/dapr
that referenced
this issue
Sep 25, 2023
Fixes dapr#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>
|
This issue has been automatically marked as stale because it has not had activity in the last 60 days. It will be closed in the next 7 days unless it is tagged (pinned, good first issue, help wanted or triaged/resolved) or other activity occurs. Thank you for your contributions. |
ItalyPaleAle
added a commit
to ItalyPaleAle/dapr
that referenced
this issue
Oct 20, 2023
Fixes dapr#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>
7 tasks
yaron2
pushed a commit
that referenced
this issue
Oct 24, 2023
…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>
|
Tracking doc issue dapr/docs#3987 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In what area(s)?
/area runtime
What version of Dapr?
1.1.x
master
Description
Related: #6581
The Dapr HTTP server collects metrics with very high cardinality by default. Each path invoked in the HTTP server is used to create a "bucket" in the metrics, and this causes significant memory usage when using Dapr to invoke RESTful APIs (where the path is usually different on every request).
This happens in service invocation metrics as well as in the general HTTP server metrics.
Users can manually configure Dapr to reduce the cardinality of metrics, but that is a manual process that is not immediately clear, and it requires writing regular expressions.
@yaron2 and I have been discussing how to address this issue and our proposal is:
metrics.highCardinality, disabled by default, to retain the current behavior.Release Note
RELEASE NOTE: CHANGED Reduced default cardinality of metrics collected by HTTP server
The text was updated successfully, but these errors were encountered: