V4 Updates for Metrics, Tracing and Security #336
Conversation
Preview link: https://docs-steeltoe-pr-336.azurewebsites.net
|
api/v4/management/prometheus.md
Outdated
| ## Instrumentation | ||
|
|
||
| In order for the Prometheus endpoint to return metrics, the application and relevant libraries need to be instrumented. | ||
| This page will cover the basics that Steeltoe has previously configured automatically, and you should refer to the [OpenTelemetry documentation](https://opentelemetry.io/docs/languages/net/instrumentation/) for more information. |
There was a problem hiding this comment.
the basics that Steeltoe has previously configured automatically
Does this refer to earlier versions of Steeltoe, as opposed to referring to an earlier section in this document?
There was a problem hiding this comment.
Yes, it does, and should probably be worded differently. I don't think this is the only location that refers to previous Steeltoe versions, I'm not sure if we should always say it exactly the same way, or maybe rotate several variations just to avoid verbatim repetition... either way it all needs another pass to definitively avoid ambiguities like you've pointed out here
Co-authored-by: Bart Koelman <[email protected]>
|
|
| | CLR | Heap memory, garbage collections, thread utilization. | | ||
| | HTTPClient | Request timings and counts. | | ||
| | ASP.NET Core | Request timings and counts. | |
There was a problem hiding this comment.
Perhaps it's just me, but I find it hard to understand the relationship of these entries with the actual configuration settings. I recommend to drop this table (which adds an indirection that's hard to correlate to the actual settings) and instead provide good documentation in the configuration keys themselves.
For the CLR-type observers, we should link to the list of metrics it affects, such as here, or list them explicitly. And sync up the doc-comments on MetricsObserverOptions in Steeltoe.
Looking at the Steeltoe source, these are the affected metrics:
- AspNetCoreHosting (I wonder why these are different from https://learn.microsoft.com/en-us/dotnet/core/diagnostics/available-counters#microsoftaspnetcorehosting-counters, which points to https://github.com/dotnet/aspnetcore/blob/main/src/Hosting/Hosting/src/Internal/HostingEventSource.cs):
http.server.requests.secondshttp.server.requests.count
- GCEvents:
clr.memory.usedclr.gc.collectionsclr.process.uptimeclr.cpu.count
- ThreadPoolEvents:
clr.threadpool.activeclr.threadpool.avail
- EventCounterEvents: The entries listed at https://learn.microsoft.com/en-us/dotnet/core/diagnostics/available-counters#systemruntime-counters.
- HttpClient
http.client.request.timehttp.client.request.count
I'm not sure these lists are complete or actually work. Surely needs testing, there are bugs.
Why do we have our own observers, instead of rely on the built-in metrics that the .NET runtime provides?
There was a problem hiding this comment.
Honestly, I just shuffled this content around and made surface-level changes. But you're right, we need to comprehensively review the observers too since it's just not very well done.
I thought the work around Observers was focused on removing the OpenTelemetry dependency and syncing with Spring, which should explain why we wouldn't be completely in sync with .NET... But what we're seeing still doesn't make much sense through that lens either and it might take a bit longer to sort out properly.
I believe we still need the DiagnosticObserver abstraction (or something similar) in order to collect what the EventSources emit... The OpenTelemetry instrumentation packages have a ListenerHandler that is pretty much the same thing.
| | `HttpClientCore` | Enable Http Client Metrics. | `false` | | ||
| | `HttpClientDesktop` | Enable Http Client Desktop Metrics. | `false` | | ||
| | `ExcludedMetrics` | Specify a list of metrics that should not be captured | none | | ||
| | `HttpClient` | Capture outgoing HTTP requests using [`HttpClient`](https://learn.microsoft.com//dotnet/api/system.net.http.httpclient). | `false` | |
There was a problem hiding this comment.
HttpClient is actually incorrect, unless we rename the property. See SteeltoeOSS/Steeltoe#1423.
Co-authored-by: Bart Koelman <[email protected]>
|
|
||
| The Steeltoe package `Steeltoe.Configuration.CloudFoundry` reads Single Sign-On credentials from Cloud Foundry service bindings (`VCAP_SERVICES`) and re-maps them for Microsoft's JwtBearer library to read. Add the configuration provider to your application with this code: | ||
|
|
||
| ```csharp |
There was a problem hiding this comment.
Namespace imports are missing here.
| @@ -4,7 +4,7 @@ To improve the Steeltoe developer experience, this feature allows the configurat | |||
|
|
|||
| Get started by adding a reference to the AutoConfiguration package (you may want to add other Steeltoe references at this point too, see [the table below](#supported-steeltoe-packages) for the full list of what's supported): | |||
|
|
|||
| ``` | |||
| ```shell | |||
There was a problem hiding this comment.
This could also be added in fileshares/usage.md
| @@ -56,6 +57,9 @@ Several steps need to happen before certificate authorization policies can be us | |||
| 1. Authorization services and policies need to be added | |||
| 1. Middleware needs to be activated | |||
|
|
|||
| > [!NOTE] | |||
| > This section is only required on applications that are receiving certificate-authorized requests. | |||
|
|
|||
There was a problem hiding this comment.
The namespace imports in the code block below are incorrect. It should be:
using Microsoft.AspNetCore.Authentication.Certificate;
using Steeltoe.Security.Authorization.Certificate;| @@ -56,6 +57,9 @@ Several steps need to happen before certificate authorization policies can be us | |||
| 1. Authorization services and policies need to be added | |||
| 1. Middleware needs to be activated | |||
|
|
|||
| > [!NOTE] | |||
| > This section is only required on applications that are receiving certificate-authorized requests. | |||
|
|
|||
There was a problem hiding this comment.
The comment in the code block below should be changed from:
Register Steeltoe components and policies requiring space or org to match between client and server certificates
to:
Register Steeltoe components and policies requiring org and space to match between client and server certificates
api/v4/security/certificate.md
Outdated
| @@ -74,6 +78,7 @@ builder.Services.AddAuthorizationBuilder() | |||
|
|
|||
| > [!TIP] | |||
| > Steeltoe configures the certificate forwarding middleware to look for a certificate in the `X-Client-Cert` HTTP header. | |||
| > To change the header name used for authorization, pass your value in when registering the policy: `.AddOrgAndSpacePolicies("Custom-Certificate-Header")`. | |||
There was a problem hiding this comment.
Line below:
use the IApplicationBuilder extension method UseCertificateAuthorization:
use the UseCertificateAuthorization extension method on IApplicationBuilder:
api/v4/security/certificate.md
Outdated
| @@ -74,6 +78,7 @@ builder.Services.AddAuthorizationBuilder() | |||
|
|
|||
| > [!TIP] | |||
| > Steeltoe configures the certificate forwarding middleware to look for a certificate in the `X-Client-Cert` HTTP header. | |||
| > To change the header name used for authorization, pass your value in when registering the policy: `.AddOrgAndSpacePolicies("Custom-Certificate-Header")`. | |||
There was a problem hiding this comment.
Moving the section "Steeltoe exposes some of the policy-related..." before "To activate certificate-based authorization" feels more natural to me.
api/v4/security/certificate.md
Outdated
| @@ -74,6 +78,7 @@ builder.Services.AddAuthorizationBuilder() | |||
|
|
|||
| > [!TIP] | |||
| > Steeltoe configures the certificate forwarding middleware to look for a certificate in the `X-Client-Cert` HTTP header. | |||
| > To change the header name used for authorization, pass your value in when registering the policy: `.AddOrgAndSpacePolicies("Custom-Certificate-Header")`. | |||
There was a problem hiding this comment.
The comment below:
// Requirement classes are public
Should be replaced with:
// Or the equivalent using different syntax
And would:
[
new SameOrgRequirement(),
new SameSpaceRequirement()
]fit on a single line?
|
|
||
| If Single Sign-On for Tanzu is not available or desired for your application, you can use UAA as an alternative. | ||
|
|
||
| There is no service broker available to manage service instances or bindings for UAA, so a [user provided service instance](https://docs.cloudfoundry.org/devguide/services/user-provided.html) should be used to hold the credentials. |
There was a problem hiding this comment.
| There is no service broker available to manage service instances or bindings for UAA, so a [user provided service instance](https://docs.cloudfoundry.org/devguide/services/user-provided.html) should be used to hold the credentials. | |
| There is no service broker available to manage service instances or bindings for UAA, so a [user provided service instance](https://docs.cloudfoundry.org/devguide/services/user-provided.html) must be used to hold the credentials. |
Co-authored-by: Bart Koelman <[email protected]>
Addresses the remaining parts of #317.