diff --git a/AGENTS.md b/AGENTS.md index 70c1597998..4f7f05fc04 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -29,6 +29,23 @@ Prefer public records close to the implementation. Keep `docs/README.md` current 5. If sources conflict, do not resolve the conflict by inference. Use the current implementation and public contract for external behavior, report the conflict, and ask the record owner when it affects the decision. 6. In the response or pull request, cite the records consulted, distinguish evidence from inference, and state when relevant private context was unavailable or unauthorized. Keep private locations, quotations, customer names, and other confidential details out of public artifacts such as pull request descriptions, code comments, and `docs/`; say that internal context was consulted instead. +## Writing + +These rules apply to everything written for a reader: pull request descriptions, commit messages, `docs/`, ADRs, code comments, and review replies. Write in en-US. + +- Start with the main point: the decision, the change, or the answer. Add only the background the reader needs to act on it or to agree with it. +- Every sentence adds something the reader does not already have from the code, the diff, the title, or earlier text in the same document. Cut sentences that announce, summarize, or repeat. +- State facts plainly. Do not add weight with a contrast against a claim nobody made, a one-line closer, inflated significance, or words such as pivotal, seamless, or robust used figuratively. Keep a contrast that corrects something a reader would likely assume. +- Write short, direct sentences, under 25 words where possible, with a named actor and an active verb. Keep the passive when the source does not say who acts. Keep should and must where the text sets a rule. Qualify a claim only when the evidence is uncertain, and say what is uncertain. +- Use one term for one concept, and use the term the code uses. Define a term on first use when the audience may not know it. Avoid idioms and phrasal verbs that a non-native reader or a translation tool can misread. +- In a procedure, write one action per numbered step, in the imperative, in the order the reader performs it. +- Write a commit subject or pull request title as a short imperative summary of the change, without emoji or type prefixes; a squash merge turns the title into the commit subject. Give each commit one intent, and put renames and formatting changes in their own commits. +- Use formatting only where it helps scanning: sentence-case headings, lists for three or more parallel items, no bold label on every item, no emoji, and no dashes to join clauses in prose. Index entries and list items may be fragments, with a dash or colon between a term and its description. +- Do not invent facts, numbers, rationale, or sources. When the record does not say, write that it does not say. +- Leave out chat and drafting residue: offers, praise, and notes on how the text was produced or what it replaced. Keep provenance that changes how to read the text, such as a record reconstructed after the decision. Mention earlier behavior only where it explains the current design or a default, or in changelogs, release notes, and upgrade guides. + +Default to no code comment. Write one only for what the code cannot state, such as a non-obvious invariant, an ordering requirement, or a workaround whose cause is not visible. Explain why, not what, in one line, two at most. When it needs more, put the explanation in `docs/` or the pull request and link to it with a pointer comment. History such as "changed to fix" belongs in the commit, not the comment. + ## Pointer comments A brief code comment may link to a canonical public source, such as a `docs/` file or an ADR under `docs/decisions/`, when the relevant rationale is not apparent from the surrounding code. It is a signpost, not a copy of the rationale: keep the durable explanation in the linked record. Do not use a comment to narrate obvious code, and do not restate a pull request or ADR in the comment body. diff --git a/docs/README.md b/docs/README.md index f421eb3b1c..41a26ddec2 100644 --- a/docs/README.md +++ b/docs/README.md @@ -2,7 +2,7 @@ ## About this repository -ServiceControl is the monitoring component of the Particular Service Platform: it ingests audit and error messages, tracks endpoint heartbeats, and exposes results over an HTTP API consumed by ServicePulse. For local run and debug steps see the `README.md`, for test categories and setup see `testing.md`, and for coding conventions see `coding-and-design-guidelines.md`. +ServiceControl is the monitoring component of the Particular Service Platform: it ingests audit and error messages, tracks endpoint heartbeats, and exposes results over an HTTP API consumed by ServicePulse. For local run and debug steps, see the `README.md`. For test categories and setup, see `testing.md`. For coding conventions, see `coding-and-design-guidelines.md`. - `src/` — ServiceControl, ServiceControl.Audit, ServiceControl.Monitoring instances, persisters, and their test projects - `docs/` — design rationale, testing guidance, and architecture decision records @@ -23,15 +23,15 @@ ServiceControl is the monitoring component of the Particular Service Platform: i This section points to sources that explain why ServiceControl is designed the way it is. Each entry says which question it answers. How-to material such as testing setup stays in the pages linked under Start here. - [Ingestion pipeline](ingestion-pipeline.md) — why batch parallelism is a storage decision, not an instance decision -- [Error ingestion design](error-ingestion-design.md) — relational-persister error ingestion design +- [Error ingestion design](error-ingestion-design.md) — why the relational persisters write failed messages with hand-written SQL - [Bulk retries design](bulk-retries-design.md) — how ServiceControl retries failed messages in bulk -- [Retries over Azure Storage Queues transport](retries-asq-transport.md) — transport-specific retry handling -- [Data versioning design](data-versioning-design.md) — the cache-versioning invariant for API responses +- [Retries over Azure Storage Queues transport](retries-asq-transport.md) — how retries behave with one or several storage accounts +- [Data versioning design](data-versioning-design.md) — which invariant keeps a cached API response from going stale - [Event log design](eventlog-design.md) — what the event log is and what it records - [Multiple ServiceControl instances communication](multipleservicecontrolinstancescommunication.md) — how primary, audit, and monitoring instances talk to each other - [Handling unavailable runtime dependencies](handling-unavailable-runtime-dependencies.md) — how instances react when a dependency is unavailable -- [Telemetry](telemetry.md) — telemetry configuration and emitted metrics -- [Throughput collection](throughput-collection.md) — why and how usage data is collected +- [Telemetry](telemetry.md) — how to configure telemetry export and which metrics instances emit +- [Throughput collection](throughput-collection.md) — how usage data is collected and why the throughput queue has a fixed name ## Decisions and rationale diff --git a/docs/authentication-testing.md b/docs/authentication-testing.md index a2f30886e0..7d9f7481fd 100644 --- a/docs/authentication-testing.md +++ b/docs/authentication-testing.md @@ -1,20 +1,20 @@ -# Local Authentication Testing +# Local authentication testing This guide explains how to test authentication configuration for ServiceControl instances. This approach uses curl to test authentication enforcement and configuration endpoints. ## Prerequisites - ServiceControl built locally (see [main README for instructions](../README.md#how-to-rundebug-locally)) -- **Identity Provider (IdP) configured** - For real authentication testing (Scenarios 7+), you need an OIDC provider configured with: +- An identity provider (IdP), configured for real authentication testing (Scenarios 7+). You need an OIDC provider configured with: - An API application registration (for ServiceControl) - A client application registration (for ServicePulse) - API scopes configured and permissions granted - See [ServiceControl Authentication](https://docs.particular.net/servicecontrol/security/configuration/authentication) for example setups - curl (included with Windows 10/11, Git Bash, or WSL) -- HTTP Request logging to view comms to and from instances +- HTTP request logging to view communication to and from instances - (Optional) For formatted JSON output: `npm install -g json` then pipe curl output through `| json` -## Enabling Debug Logs +## Enabling debug logs To enable detailed logging for troubleshooting, set the `LogLevel` environment variable before starting each instance: @@ -26,11 +26,11 @@ set MONITORING_LOGLEVEL=Debug **Valid log levels:** `Trace`, `Debug`, `Information` (or `Info`), `Warning` (or `Warn`), `Error`, `Critical` (or `Fatal`), `None` (or `Off`) -Debug logs will show detailed authentication flow information including token validation, claims processing, and authorization decisions. +Debug logs show detailed authentication flow information including token validation, claims processing, and authorization decisions. -### HTTP Request Logs +### HTTP request logs -HTTP logs can be enabled by adding a `nlog.config` file in beside the exe: +HTTP logs can be enabled by adding a `nlog.config` file next to the executable: ```xml @@ -55,7 +55,7 @@ HTTP logs can be enabled by adding a `nlog.config` file in beside the exe: ``` -## Instance Reference +## Instance reference | Instance | Project Directory | Default Port | Environment Variable Prefix | |---------------------------|---------------------------------|--------------|-----------------------------| @@ -63,7 +63,7 @@ HTTP logs can be enabled by adding a `nlog.config` file in beside the exe: | ServiceControl.Audit | `src\ServiceControl.Audit` | 44444 | `SERVICECONTROL_AUDIT_` | | ServiceControl.Monitoring | `src\ServiceControl.Monitoring` | 33633 | `MONITORING_` | -## How Authentication Works +## How authentication works When authentication is enabled: @@ -72,22 +72,22 @@ When authentication is enabled: 3. Requests without a valid token receive a `401 Unauthorized` response 4. The `/api/authentication/configuration` endpoint returns authentication configuration for clients (like ServicePulse) -## Configuration Methods +## Configuration methods Settings can be configured via: -1. **Environment variables** (recommended for testing) - Easy to change between scenarios, no file edits needed -2. **App.config** - Persisted settings, requires app restart after changes +1. Environment variables (recommended for testing): Easy to change between scenarios, no file edits needed +2. App.config: Persisted settings, requires an app restart after changes Both methods work identically. This guide uses environment variables for convenience during iterative testing. -## Test Scenarios +## Test scenarios > [!IMPORTANT] > Set environment variables in the same terminal where you run `dotnet run`. Environment variables are scoped to the terminal session. > Check the application startup logs to verify which settings were applied. The authentication configuration is logged at startup. -### Test Grouping by Configuration +### Test grouping by configuration To minimize service restarts during testing, scenarios are grouped by configuration. Run all tests within a group before changing configuration: @@ -104,7 +104,7 @@ To minimize service restarts during testing, scenarios are grouped by configurat --- -## Group A: Authentication Disabled Configuration +## Group A: Authentication disabled configuration **Start the instance once (Scenario 1).** @@ -139,7 +139,7 @@ set MONITORING_AUTHENTICATION_VALIDATEAUDIENCE= dotnet run ``` -### Scenario 1: Authentication Disabled (Default) +### Scenario 1: Authentication disabled (default) Test the default behavior where authentication is disabled and all requests are allowed. @@ -192,7 +192,7 @@ The configuration indicates authentication is disabled. Other fields are omitted --- -## Group B: Authentication Enabled (Test Authority) Configuration +## Group B: Authentication enabled (test authority) configuration **Restart the instance with this configuration, then run all tests in this group (Scenarios 2, 3, 4).** @@ -229,9 +229,9 @@ dotnet run ``` > [!NOTE] -> This configuration uses a test authority URL. For testing authentication enforcement without a real provider, any HTTP URL works - requests fail before token validation because no valid token is provided. +> This configuration uses a test authority URL. For testing authentication enforcement without a real provider, any HTTP URL works. Requests fail before token validation because no valid token is provided. -### Scenario 2: Authentication Enabled (No Token) +### Scenario 2: Authentication enabled (no token) Test that requests without a token are rejected when authentication is enabled. @@ -258,12 +258,12 @@ curl -v http://localhost:33633/monitored-endpoints 2>&1 | findstr /C:"HTTP/" Requests without a token are rejected with `401 Unauthorized`. > [!NOTE] -> The endpoint `/api/authentication/configuration` are marked as anonymous and will return `200 OK` even with authentication enabled. Test protected endpoints like `/api/endpoints` to verify authentication enforcement. +> The endpoint `/api/authentication/configuration` is marked as anonymous and returns `200 OK` even with authentication enabled. Test protected endpoints like `/api/endpoints` to verify authentication enforcement. #### Check authentication configuration endpoint (no auth required) > [!NOTE] -> Only the primary instance has this endpoint. Requesting this endpoint from the audit and monitoring instance will return unauthorized. +> Only the primary instance has this endpoint. Requesting this endpoint from the audit and monitoring instances returns unauthorized. ```cmd rem ServiceControl (Primary) @@ -290,7 +290,7 @@ curl http://localhost:33633/api/authentication/configuration | json The authentication configuration endpoint is accessible without authentication and returns the configuration that clients need to authenticate. The `authority` field is omitted when `ServicePulse.Authority` is not explicitly set (it defaults to the main Authority for ServicePulse clients). The `audience` field is copied from the `ServiceControl/Authentication.Audience` value. The `apiScopes` field is the raw JSON array as configured. The `scopes` field is the complete, space-separated scope string ServicePulse should request, composed by ServiceControl by parsing the `apiScopes` JSON array and adding the fixed `openid profile email` scopes plus `offline_access` unless `ServiceControl/Authentication.ServicePulse.OfflineAccessScopeEnabled` is set to `false`. -### Scenario 3: Authentication with Invalid Token +### Scenario 3: Authentication with invalid token Test that requests with an invalid token are rejected. @@ -316,7 +316,7 @@ curl -v -H "Authorization: Bearer invalid-token-here" http://localhost:33633/mon Invalid tokens are rejected with `401 Unauthorized`. -### Scenario 4: Anonymous Endpoints +### Scenario 4: Anonymous endpoints Test that anonymous endpoints remain accessible when authentication is enabled. @@ -346,7 +346,7 @@ See [Authentication](https://docs.particular.net/servicecontrol/security/#authen --- -## Group C: Relaxed Validation Configuration +## Group C: Relaxed validation configuration **Restart the instance with this configuration (Scenario 5).** @@ -381,7 +381,7 @@ set MONITORING_AUTHENTICATION_VALIDATEAUDIENCE=false dotnet run ``` -### Scenario 5: Validation Settings Warnings +### Scenario 5: Validation settings warnings Test that disabling validation settings produces warnings in the logs. @@ -396,7 +396,7 @@ The application warns about insecure validation settings. --- -## Group D: Missing Settings Configuration (Startup Failure Test) +## Group D: Missing settings configuration (startup failure test) **Attempt to start the instance with this configuration (Scenario 6). The instance should fail to start.** @@ -431,7 +431,7 @@ set MONITORING_AUTHENTICATION_VALIDATEAUDIENCE= dotnet run ``` -### Scenario 6: Missing Required Settings +### Scenario 6: Missing required settings Test that missing required settings prevent startup. @@ -445,16 +445,16 @@ Authentication.Authority is required when authentication is enabled. Please prov --- -## Group E: Real Identity Provider Configuration +## Group E: Real identity provider configuration > [!IMPORTANT] -> This group requires a configured OIDC provider (e.g., Microsoft Entra ID, Auth0, Okta). +> This group requires a configured OIDC provider (for example Microsoft Entra ID, Auth0, Okta). > See [ServiceControl Authentication](https://docs.particular.net/servicecontrol/security/configuration/authentication) for setup examples. **Start all instances with this configuration, then run all tests in this group (Scenarios 7, 8, 10, 11, 14).** > [!NOTE] -> See [HTTPS Testing](https-testing.md) for certificate setup instructions using mkcert. +> See [Local testing with direct HTTPS](https-testing.md) for certificate setup instructions using mkcert. ```cmd rem ServiceControl (Primary) @@ -497,7 +497,7 @@ set MONITORING_AUTHENTICATION_VALIDATEAUDIENCE= dotnet run ``` -### Scenario 7: Authentication with Valid Token (Real Identity Provider) +### Scenario 7: Authentication with valid token (real identity provider) Test end-to-end authentication with a valid token from a real OIDC provider. @@ -527,9 +527,9 @@ curl --ssl-no-revoke -H "Authorization: Bearer %TOKEN%" https://localhost:33633/ [] ``` -Requests with a valid token are processed successfully. The response will be an empty array if no data exists, or a list of items if data exists. +Requests with a valid token are processed successfully. The response is an empty array if no data exists, or a list of items if data exists. -### Scenario 8: Scatter-Gather with Authentication (Token Forwarding) +### Scenario 8: Scatter-gather with authentication (token forwarding) Test that the primary instance forwards authentication tokens to remote instances during scatter-gather operations. @@ -545,7 +545,7 @@ set TOKEN=$(az account get-access-token --resource api://servicecontrol --query curl --ssl-no-revoke -H "Authorization: Bearer %TOKEN%" https://localhost:33333/api/messages | json ``` -Ensure `Debug` logs are enabled. Take a look at the primary and audit logs. You should see the requests being sent/received indicating if an auth header is included. +Ensure `Debug` logs are enabled. Check the primary and audit logs. They show the requests that were sent and received and whether an auth header is included. #### Test with no token (should fail) @@ -561,7 +561,7 @@ No audit logs, and: < HTTP/1.1 401 Unauthorized ``` -### Scenario 10: Remote Instance Health Checks with Authentication +### Scenario 10: Remote instance health checks with authentication Test that the primary instance can check remote instance health when authentication is enabled. @@ -591,7 +591,7 @@ You should see a log in the audit instance stating a request was received at the The health check should succeed because `/api` is an anonymous endpoint. -### Scenario 11: Platform Connection Details with Authentication +### Scenario 11: Platform connection details with authentication Test that platform connection details can be retrieved when authentication is enabled on remote instances. @@ -606,9 +606,9 @@ curl --ssl-no-revoke -H "Authorization: Bearer %TOKEN%" https://localhost:33333/ **Expected behavior:** -The platform connection response includes connection details from both the primary and remote instances. The audit log will show the request. +The platform connection response includes connection details from both the primary and remote instances. The audit log shows the request. -### Scenario 14: Expired Token Forwarding +### Scenario 14: Expired token forwarding Test how scatter-gather handles expired tokens being forwarded to remote instances. @@ -628,12 +628,12 @@ The primary instance rejects the expired token before any remote requests are ma --- -## Group F: Mismatched Audiences Configuration +## Group F: Mismatched audiences configuration **Restart all instances with this configuration (Scenario 9). Note the DIFFERENT audience for Audit.** > [!NOTE] -> See [HTTPS Testing](https-testing.md) for certificate setup instructions using mkcert. +> See [Local testing with direct HTTPS](https-testing.md) for certificate setup instructions using mkcert. ```cmd rem ServiceControl (Primary) @@ -676,7 +676,7 @@ set MONITORING_AUTHENTICATION_VALIDATEAUDIENCE= dotnet run ``` -### Scenario 9: Scatter-Gather with Mismatched Authentication Configuration +### Scenario 9: Scatter-gather with mismatched authentication configuration Test that scatter-gather fails gracefully when remote instances have different authentication settings. @@ -686,7 +686,7 @@ Test that scatter-gather fails gracefully when remote instances have different a curl --ssl-no-revoke -H "Authorization: Bearer %TOKEN%" https://localhost:33333/api/messages | json ``` -You should see a warning logged in the primary isntance. +The primary instance logs a warning. ```text warn: Authentication failed when querying remote instance at https://localhost:44444. Ensure authentication is correctly configured. @@ -694,12 +694,12 @@ You should see a warning logged in the primary isntance. --- -## Group G: Mixed Configuration (Primary Only Auth) +## Group G: Mixed configuration (primary only auth) **Restart all instances with this configuration (Scenario 12). Primary has auth, Audit and Monitoring do not.** > [!NOTE] -> See [HTTPS Testing](https-testing.md) for certificate setup instructions using mkcert. +> See [Local testing with direct HTTPS](https-testing.md) for certificate setup instructions using mkcert. ```cmd rem ServiceControl (Primary) - WITH authentication @@ -742,7 +742,7 @@ set MONITORING_AUTHENTICATION_VALIDATEAUDIENCE= dotnet run ``` -### Scenario 12: Mixed Authentication Configuration (Primary Only) +### Scenario 12: Mixed authentication configuration (primary only) Test behavior when only the primary instance has authentication enabled, but remote instances do not. @@ -765,12 +765,12 @@ Logs in the primary instance show that the request was sent successfully (with a --- -## Group H: Mixed Configuration (Remotes Only Auth) +## Group H: Mixed configuration (remotes only auth) **Restart all instances with this configuration (Scenario 13). Audit and Monitoring have auth, Primary does not.** > [!NOTE] -> See [HTTPS Testing](https-testing.md) for certificate setup instructions using mkcert. +> See [Local testing with direct HTTPS](https-testing.md) for certificate setup instructions using mkcert. ```cmd rem ServiceControl (Primary) - WITHOUT authentication @@ -813,11 +813,11 @@ set MONITORING_AUTHENTICATION_VALIDATEAUDIENCE= dotnet run ``` -### Scenario 13: Mixed Authentication Configuration (Remotes Only) +### Scenario 13: Mixed authentication configuration (remotes only) Test behavior when remote instances have authentication enabled, but the primary does not. -Chech the primary logs. All health checks (service-to-service) calls complete successfully as these are anonymous endpoints. +Check the primary logs. All health check calls (service-to-service) complete successfully, because these are anonymous endpoints. #### Query without a token @@ -825,7 +825,7 @@ Chech the primary logs. All health checks (service-to-service) calls complete su curl --ssl-no-revoke https://localhost:33333/api/messages | json ``` -The original request to the primary instance will be successfull and give the below output. If you check the primary instance logs however, there will be an error message saying the call to the audit instance failed due to authentication issues. +The original request to the primary instance succeeds and returns the output below. The primary instance logs an error message saying that the call to the audit instance failed due to authentication issues. **Expected output:** @@ -838,8 +838,8 @@ The original request to the primary instance will be successfull and give the be --- -## See Also +## See also -- [Authentication Configuration](https://docs.particular.net/servicecontrol/security/configuration/authentication#configuration) - Configuration reference for authentication settings -- [TLS Configuration](https://docs.particular.net/servicecontrol/security/configuration/tls#configuration) - HTTPS/TLS is recommended when authentication is enabled -- [Forwarded Headers Testing](forward-headers-testing.md) - Testing forwarded headers +- [Authentication Configuration](https://docs.particular.net/servicecontrol/security/configuration/authentication#configuration): Configuration reference for authentication settings +- [TLS Configuration](https://docs.particular.net/servicecontrol/security/configuration/tls#configuration): HTTPS/TLS is recommended when authentication is enabled +- [Local testing of forwarded headers without NGINX](forward-headers-testing.md): Testing forwarded headers diff --git a/docs/bulk-retries-design.md b/docs/bulk-retries-design.md index 0f4299cfd1..c218ce40fc 100644 --- a/docs/bulk-retries-design.md +++ b/docs/bulk-retries-design.md @@ -1,106 +1,105 @@ -# How ServiceControl Retries Works +# How ServiceControl retries work ## Overview -When you request a Retry (Bulk or Individual) using the ServiceControl API, ServiceControl creates one (or more) Retry Batches which go through a series of steps to ensure that matching Failed Messages get sent back to their intended destinations. +When you request a retry (bulk or individual) using the ServiceControl API, ServiceControl creates one or more retry batches. Each batch goes through a series of steps that send the matching failed messages back to their intended destinations. -Each Retry Batch consists of no more than 1,000 failed messages. Anything bigger than that gets broken up into Retry Batches of max 1,000. +A retry batch contains at most 1,000 failed messages. ServiceControl splits larger requests into several batches of at most 1,000. -Batches go through 4 stages in order: `Marking Documents` -> `Staging` -> `Forwarding` -> `Done` +Batches go through four stages in order: `Marking Documents` -> `Staging` -> `Forwarding` -> `Done` -The code that handles each stage is idempotent so re-processing a batch is never a problem. As a batch can only ever move forward through the stages, if a message is picked up in the first stage it should eventually be retried. +The code that handles each stage is idempotent, so re-processing a batch is safe. A batch only moves forward through the stages. A message picked up in the first stage is eventually retried. -In addition we also keep an in-memory representation of the retry operation to use for progress tracking. It is created when the request is received and updated as the retry batches making up an operation flow through the various stages. When the operation completes, a history item is persisted to enable users to view finished operations. +ServiceControl also keeps an in-memory representation of the retry operation for progress tracking. It is created when the request is received and updated as the retry batches of the operation move through the stages. When the operation completes, ServiceControl persists a history item so that users can view finished operations. -When ServiceControl restarts, the in-memory representations are rebuilt by aggregating state from the persisted batches. +When ServiceControl restarts, it rebuilds the in-memory representations by aggregating state from the persisted batches. ### Marking Documents -When a retry batch is first created it has this state. This means that ServiceControl is finding and marking failed messages as belonging to this batch. +A retry batch has this state when it is first created. In this state, ServiceControl finds failed messages and marks them as belonging to the batch. -To do this, a new document is created for each failed message. Each one has an id `FailedMessageRetry/{messageId}` and contains the failed message id and the retry batch id. As only one document with this key can exist at a time, this ensures that a Failed Message can only ever belong to a single batch. This FailedMessageRetry document will exist until such a time as a new Failed Message with same Id comes through the error queue. This guarantees that there can only be one outstanding retry for a failed message at a time. +ServiceControl creates a new document for each failed message. Each document has the id `FailedMessageRetry/{messageId}` and contains the failed message id and the retry batch id. Only one document with this key can exist at a time, so a failed message can belong to only one batch. The `FailedMessageRetry` document exists until a new failed message with the same id comes through the error queue. This guarantees that a failed message has only one outstanding retry at a time. -When all messages have been marked, a list of the `FailedMessageRetry` ids is appended to the batch and the batch changes status to `Staging`. The list inside of the Batch may contain failed messages which do not belong to this batch (because another batch claimed them in parallel). These will get filtered out during staging (below). +When all messages are marked, ServiceControl appends a list of the `FailedMessageRetry` ids to the batch and changes the batch status to `Staging`. The list in the batch can contain failed messages that do not belong to this batch, because another batch claimed them in parallel. Staging filters these out (see below). -When ServiceControl starts up it will attempt to adopt any batches that it finds in this status and move them to `Staging`. This can only happen if the ServiceControl process stops during the above process. Any documents that were already marked will be added to the batch. Any documents that had not yet been marked are ignored and will have to be retried again by the user. +When ServiceControl starts up, it adopts any batch it finds in the `Marking Documents` status and moves it to `Staging`. This happens only if the ServiceControl process stopped during marking. ServiceControl adds the documents that were already marked to the batch. It ignores documents that were not yet marked, and the user has to retry those again. -When a bulk retry is issued a background process is started that looks for all of the messages that match its criteria and assign them to a retry batch id (in lots of 1,000). If the process is killed before it is done, the details of what you were trying to retry are gone. Any documents that had already marked for retry will be retried. Any that were not marked for retry will require manual intervention to retry +When a bulk retry is issued, ServiceControl starts a background process. The process looks for all messages that match the criteria and assigns them to a retry batch id, in lots of 1,000. If the process stops before it finishes, the details of the requested retry are lost. Documents already marked for retry are retried. Messages that were not marked require the user to retry them manually. -When ServiceControl starts up, it will generate a `Session ID` GUID. This GUID is stamped onto each new batch as it is created. This is how ServiceControl can tell if a batch is from a previous session and adopt it. Only Batches with a non-current session Id will be adopted by the orphan batch process. +When ServiceControl starts up, it generates a `Session ID` GUID. ServiceControl stamps this GUID onto each new batch as it creates the batch. This is how ServiceControl tells that a batch is from a previous session. The orphan batch process adopts only batches with a non-current session id. ## Recovering retry batches -When ServiceControl starts it will look for documents marked with a retry batch id, without a matching retry batch document (i.e. We started making the batch but we never completed it). Retry batches in this state as called "orphaned" because the process that was assembling them has been lost. +When ServiceControl starts, it looks for documents marked with a retry batch id that have no matching retry batch document. This happens when the process started making the batch but never completed it. ServiceControl calls such batches "orphaned" because the process that was assembling them is lost. -When an instance of ServiceControl "adopts" an orphaned batch, it takes over the processing of it. That instance will find all of the messages with the same missing retry batch id and create a retry batch with that id for them. This new retry batch document will then get picked up and processed like any other. +When an instance of ServiceControl "adopts" an orphaned batch, it takes over the processing. The instance finds all messages with the same missing retry batch id and creates a retry batch with that id for them. ServiceControl then processes this new retry batch document like any other. ### Staging Staging reduces the chance of sending a message for retry more than once. Transports that do not support `SendsAtomicWithReceive` strictly would not need staging to prevent more-than-once delivery. -When a batch enters this state, it means that failed messages belonging to the batch are being added to a special `staging` queue. This queue is used during `Forwarding` to ensure that messages are sent back to their original destination transactionally (using the transports recieve transaction). +In this state, ServiceControl adds the failed messages of the batch to a special `staging` queue. `Forwarding` uses this queue to send messages back to their original destination transactionally, using the receive transaction of the transport. -NOTE: Although multiple batches may be in `Staging` or `Forwarding` at a time, these are processed by a single thread ensuring that the rest of the process is serialized. This is important to ensure that only one batch at a time has access to the `staging` queue. Batches in `Staging` will only be processed if there are no batches in `Forwarding`. +Multiple batches can be in `Staging` or `Forwarding` at a time, but a single thread processes them, which serializes the rest of the process. Only one batch at a time can have access to the `staging` queue. ServiceControl processes batches in `Staging` only if no batch is in `Forwarding`. -If a message fails to be forwarded it is removed from the batch. If the batch contains no messages, then it is marked as complete. +If a message fails to be forwarded, ServiceControl removes it from the batch. If the batch then contains no messages, ServiceControl marks it as complete. -When a batch is selected for staging a new `Staging Id` is generated for it and the list of failed messages belonging to the batch is loaded. At this time, if a message had been snagged by another batch, it gets filtered out. +When ServiceControl selects a batch for staging, it generates a new `Staging Id` for the batch and loads the list of failed messages that belong to it. At this point, ServiceControl filters out any message that another batch claimed. -Each message, one at a time, is dispatched to the `staging` queue. As each message is dispatched: +ServiceControl dispatches each message to the `staging` queue, one at a time. For each message, it does the following: -1. The corresponding `Failedmessage` document is updated to reflect that it has entered `RetryIssued` mode. -2. Error headers are stripped from the copy sent to `staging` -3. A header is added to the copy sent to `staging` to stamp it with the `Staging Id` -4. A header is added to the copy sent to `staging` to indicate the messages final destination +1. Updates the corresponding `Failedmessage` document to the `RetryIssued` mode. +2. Strips the error headers from the copy sent to `staging`. +3. Adds a header to the copy sent to `staging` that stamps it with the `Staging Id`. +4. Adds a header to the copy sent to `staging` that indicates the final destination of the message. -Once all messages have been staged, a final count of the messages that have been staged gets added to the batch, and the batches status is updated to `Forwarding`. +When all messages are staged, ServiceControl adds the final count of staged messages to the batch and updates the batch status to `Forwarding`. -NOTE: If this process fails part way through, there will be messages on the `staging` queue but we won't be sure which ones. This is the purpose of the `Staging Id`. By saving the `Staging Id` and updating the status to `Forwarding` at the same time (and because this process happens on a single thread), we guarantee that only one `Staging Id` will make it to the `Forwarding` state. When we start forwarding messages, we will only send ones with a matching `Staging Id`. If the `Staging Id` does not match then it is from a previous staging attempt and can be safely discarded. +If staging fails part way through, the `staging` queue contains messages, but ServiceControl does not know which ones. The `Staging Id` solves this. ServiceControl saves the `Staging Id` and updates the status to `Forwarding` at the same time, and a single thread does this work. Therefore only one `Staging Id` reaches the `Forwarding` state. When forwarding starts, ServiceControl sends only messages with a matching `Staging Id`. A message with a different `Staging Id` is from a previous staging attempt and ServiceControl discards it. -NOTE: On top of moving the batch into `Forwarding` we also record it's Id in a Raven document with a well known Id (`RetryBatches/NowForwarding`). We do this to avoid a query and potentially stale indexes from Raven when we want to check if there is a batch in `Forwarding`. It is imperative that only one batch is ever in `Forwarding` at any given time. This is because it will clear out the contents of the `staging` queue. +When ServiceControl moves the batch into `Forwarding`, it also records the batch id in a Raven document with a well-known id (`RetryBatches/NowForwarding`). This avoids a query, and potentially stale indexes from Raven, when ServiceControl checks whether a batch is in `Forwarding`. Only one batch can be in `Forwarding` at any time, because forwarding clears the contents of the `staging` queue. ### Forwarding -Once a batch reaches this status it means that all of the failed messages that are a part of the batch are in the `staging` queue and we can start sending them to their final destination. This can happen in one of two modes: Counting and not-Counting. Counting is the standard mode of operation. +A batch in this status has all of its failed messages in the `staging` queue, and ServiceControl can start sending them to their final destination. Forwarding runs in one of two modes: counting and non-counting. Counting is the standard mode. -There is a Satellite attached to the `staging` queue which can be turned on and off. When a batch is found with the `Forwarding` status we turn on the satellite and pass in the `Staging Id` and `Message Count` of the batch. Each message that is received by the satellite will be checked to ensure that it has a matching `Staging Id`. If it does, then it is forwarded to it's final destination and an internal counter is incremented. If the internal counter reaches `Message Count` for the batch then the entire batch has been forwarded. +A satellite is attached to the `staging` queue and can be turned on and off. When ServiceControl finds a batch with the `Forwarding` status, it turns on the satellite and passes in the `Staging Id` and `Message Count` of the batch. The satellite checks that each received message has a matching `Staging Id`. If it does, the satellite forwards the message to its final destination and increments an internal counter. When the counter reaches `Message Count`, the entire batch is forwarded. -Because each message send is done in the context of a Satellite Receive operation, this process should utilize the transports native transactions. +Each message send happens in the context of a satellite receive operation, so the process should use the native transactions of the transport. -If there is a message in the `Forwarding` status when ServiceControl starts, then we don't know how many messages there are still in the staging queue to send. To counter this we turn on the satellite in Non-Counting mode. The idea for this is that the satellite will run until the queue is empty. Unfortunately there is nothing built into the Transport abstraction that allows us to query this so SC assumes that if it does not see any new messages from the `staging` queue within 45 seconds then it is empty. +If a message is in the `Forwarding` status when ServiceControl starts, ServiceControl does not know how many messages are still in the staging queue. In that case, ServiceControl turns on the satellite in non-counting mode, which runs until the queue is empty. The transport abstraction cannot query the queue length. ServiceControl therefore assumes that the queue is empty if it sees no new messages from the `staging` queue within 45 seconds. -If forwarding a specific message fail, then we count it, mark it as unresolved, and remove it from the batch. You should see warnings in the log that look like this with the error information attached: +If forwarding a specific message fails, ServiceControl counts it, marks it as unresolved, and removes it from the batch. The log then contains a warning like this, with the error information attached: > Failed to send UNIQUE-MESSAGE-ID message to DESTINATION for retry. Attempting to revert message status to unresoved so it can be tried again. -If forwarding completes and ServiceControl immediately crashes then after restart it finds an empty staging queue and times out. +If ServiceControl crashes immediately after forwarding completes, it finds an empty staging queue after the restart and times out. ### Done -There is no status to indicate that a batch is `Done`. When the `Forwarding` status is completed, the batch is deleted as it is no longer relevant. Note that each message that was retried as a part of the batch still have a corresponding `FailedMessageRetry/{messageId}` document. This will prevent the message from being retried again. +No status indicates that a batch is `Done`. When the `Forwarding` status completes, ServiceControl deletes the batch. Each message retried as part of the batch still has a corresponding `FailedMessageRetry/{messageId}` document, which prevents ServiceControl from retrying the message again. -Once all batches for a retry operation complete, we add two entries into a retry history document. An "unacknowledged" entry is kept until a user acknowledges the completion of the operation, while the other is used to show users the historic retry operations. +When all batches of a retry operation complete, ServiceControl adds two entries to a retry history document. ServiceControl keeps the "unacknowledged" entry until a user acknowledges the completion of the operation. The other entry shows users the historic retry operations. ## Other notes -1. A message can only be a part of one batch at a time. The `FailedMessageRetry/{messageId}` document will prevent a message from being added to a second batch. This document will only be removed if we see the message coming back through the error queue. -2. Once a batch is created it will eventually be forwarded. If the SC process dies while the batch is in `Marking Documents` then it will be picked up by the Adopt Orphan Batches process which will move it into `Staging`. Once a batch is in `Staging` the Retry Processor will repeatedly attempt to stage it until successful at which point it will be selected for `Forwarding`. -3. Only one batch at a time will be processed in the `Forwarding` or `Staging` status. If there is a batch in `Forwarding` then it must be fully forwarded and deleted before a new batch can be staged. If a batch is selected to be staged then it will move to the `Forwarding` status once it is fully staged. All of this happens on a single background thread that will sleep for 30 seconds if it can't find anything to do. -4. A batch can only be forwarded if it has been completely staged. -5. If an attempt to stage a batch is interrupted, the next attempt will result in the entire batch being staged again. As each staging attempt has it's own `Staging Id`, only one staging attempt will make it to `Forwarding`. Any messages from a previous attempt will be dropped as a part of the `Forwarding` process. This makes the staging step idempotent. -6. If an attempt to forward a batch is interrupted, the next attempt will simply forward matching staged messages until the staging queue is empty. During this phase, any message that does not match the recorded `Staging Id` is dropped. When the staging queue is empty, every previously staged message must have been sent. This makes the forwarding step idempotent. -7. Each message that is sent as a part of a forwarding operation is Received from the staging queue and Dispatched to it's final destination as a part of the same Transport Transaction. If a message is received but cannot be forwarded then the receive should be rolled back. The satellite that handles forwarding includes a custom Fault Manager that will attempt to eject the failed message from the batch. Under this circumstance, it is possible for a message to be retried multiple times. +1. A message can be part of only one batch at a time. The `FailedMessageRetry/{messageId}` document prevents a message from being added to a second batch. ServiceControl removes this document only when the message comes back through the error queue. +2. A batch that is created is eventually forwarded. If the ServiceControl process stops while the batch is in `Marking Documents`, the Adopt Orphan Batches process picks it up and moves it into `Staging`. Once a batch is in `Staging`, the Retry Processor repeatedly attempts to stage it until it succeeds. ServiceControl then selects the batch for `Forwarding`. +3. ServiceControl processes only one batch at a time in the `Forwarding` or `Staging` status. A batch in `Forwarding` must be fully forwarded and deleted before ServiceControl can stage a new batch. A batch selected for staging moves to the `Forwarding` status once it is fully staged. All of this happens on a single background thread that sleeps for 30 seconds if it finds nothing to do. +4. A batch can be forwarded only if it is completely staged. +5. If an attempt to stage a batch is interrupted, the next attempt stages the entire batch again. Each staging attempt has its own `Staging Id`, so only one staging attempt reaches `Forwarding`. The `Forwarding` process drops any messages from a previous attempt. This makes the staging step idempotent. +6. If an attempt to forward a batch is interrupted, the next attempt forwards the matching staged messages until the staging queue is empty. During this phase, ServiceControl drops any message that does not match the recorded `Staging Id`. When the staging queue is empty, every previously staged message was sent. This makes the forwarding step idempotent. +7. ServiceControl receives each message of a forwarding operation from the staging queue and dispatches it to its final destination as part of the same transport transaction. If a message is received but cannot be forwarded, the receive should be rolled back. The satellite that handles forwarding includes a custom fault manager that attempts to eject the failed message from the batch. In this case, a message can be retried multiple times. ### Technicalities of retrying messages - * `Headers.FailedQ` header is used as a new destination of the message. - * FailedQ is populated with the queue name to send the message back to. - * The original message headers are striped from following header values: - * `NServiceBus.Retries` - * `NServiceBus.FailedQ` - * `NServiceBus.TimeOfFailure` - * `NServiceBus.ExceptionInfo.ExceptionType` - * `NServiceBus.ExceptionInfo.AuditMessage` - * `NServiceBus.ExceptionInfo.Source` - * `NServiceBus.ExceptionInfo.StackTrace"` - - * Message is sent using above destination calling `ISendMessages.Send` implementation of given transport. +- The `Headers.FailedQ` header is used as the new destination of the message. + - ServiceControl populates `FailedQ` with the name of the queue to send the message back to. +- ServiceControl strips the following headers from the original message headers: + - `NServiceBus.Retries` + - `NServiceBus.FailedQ` + - `NServiceBus.TimeOfFailure` + - `NServiceBus.ExceptionInfo.ExceptionType` + - `NServiceBus.ExceptionInfo.AuditMessage` + - `NServiceBus.ExceptionInfo.Source` + - `NServiceBus.ExceptionInfo.StackTrace"` +- ServiceControl sends the message to the above destination by calling the `ISendMessages.Send` implementation of the transport. diff --git a/docs/coding-and-design-guidelines.md b/docs/coding-and-design-guidelines.md index e3cecf4d1d..bc1aa91116 100644 --- a/docs/coding-and-design-guidelines.md +++ b/docs/coding-and-design-guidelines.md @@ -1,69 +1,66 @@ # Coding and design guidelines -This document lists coding and design guidelines that should be followed when working on ServiceControl. In case of conflicts with other coding and design guidelines, this document should take precedence when working on ServiceControl. +This document lists the coding and design guidelines for ServiceControl. When it conflicts with other coding and design guidelines, this document takes precedence for ServiceControl. ## Prefer Microsoft abstractions -Microsoft maintains a number of abstractions for common cross-cutting concerns in the [framework libraries](https://docs.microsoft.com/en-us/dotnet/standard/framework-libraries) and in the `Microsoft.Extensions.*` packages. Where such an abstraction exists, we prefer to use it in preference to any other abstraction (including one of our own). +Microsoft maintains a number of abstractions for common cross-cutting concerns in the [framework libraries](https://docs.microsoft.com/en-us/dotnet/standard/framework-libraries) and in the `Microsoft.Extensions.*` packages. Where such an abstraction exists, use it instead of any other abstraction, including one of our own. -This makes it easier for us to isolate the application from third party dependencies by relying on relatively stable abstractions. It also helps to keep parts of the application isolated from each other (e.g. running the embedded database in maintenance mode without starting the NServiceBus endpoint). +Relying on relatively stable abstractions makes it easier to isolate the application from third-party dependencies. It also keeps parts of the application isolated from each other, for example running the embedded database in maintenance mode without starting the NServiceBus endpoint. For example: -- **Prefer `IHostBuilder` extensions over NServiceBus features**: Unless a new feature specifically alters the NServiceBus endpoint, it should be added as an extension to `IHostBuilder` rather than as a NServiceBus `Feature` implementation. Some existing ServiceControl features may still be using the `Feature` abstraction to register components. If these features do not modify the NServiceBus endpoint, they should be migrated to `IHostBuilder` extensions over time. -- **Prefer `IHostedService` to NServiceBus `FeatureStartupTask`**: ServiceControl hosts many background tasks. `IHostedService` and `FeatureStartupTask` are both abstractions for building background tasks. `FeatureStartupTask` implementations are tied to the lifecycle of an endpoint. In general, we prefer to use the `IHostedService` abstraction which is tied to the lifecycle of the host application. NOTE: `IHostedService` implementations are started in the order that they are registered, which provides more control over the startup sequence. They are shut down in the reverse order. `FeatureStartupTask` implementations are started in an order that is based on the order of `Feature` activation. This means that controlling startup sequence has to be done by configuring feature dependencies. `FeatureStartupTask` implementations are also shut down in reverse order. -- **Prefer `IServiceCollection` over `IConfigureComponents` and `IContainerBuilder`**: Where possible, we use the Microsoft DI abstraction (`IServiceCollection`) rather than the NServiceBus one (`IConfigureComponents`) or the Autofac one (`IContainerBuilder`). - - When registering components from within an NServiceBus Feature, it still makes sense to use `IConfigureComponents`, but we should consider whether it makes sense to move the code out of an NServiceBus feature. `IConfigureComponents` was [deprecated in NServiceBus version 8](https://github.com/Particular/NServiceBus/blob/335ed21dc7d230406d675bd61570b903a69c879c/src/NServiceBus.Core/obsoletes-v8.cs#L192). - - When relying on Autofac-specific features, it makes sense to use `IContainerBuilder`. We prefer to implement features in way that does not rely on Autofac-specific features. In the future, we may decide to remove our dependency on Autofac. -- **Prefer `IServiceProvider` over `ILifetimeService` and `IContainer`**: As above, we prefer to use the Microsoft DI abstraction where possible and only fall back `ILifetimeService` where strictly necessary. - - When relying on a Autofac-specific feature, `ILifetimeService` should be used. - - `IContainer` should never be used. It is functionally equivalent to `IServiceProvider` and was [deprecated in NServiceBus version 8](https://github.com/Particular/NServiceBus/blob/335ed21dc7d230406d675bd61570b903a69c879c/src/NServiceBus.Core/obsoletes-v8.cs#L252). +- Prefer `IHostBuilder` extensions over NServiceBus features: Add a new feature as an extension to `IHostBuilder` instead of a NServiceBus `Feature` implementation, unless it specifically alters the NServiceBus endpoint. Some existing ServiceControl features still use the `Feature` abstraction to register components. Migrate those that do not modify the NServiceBus endpoint to `IHostBuilder` extensions over time. +- Prefer `IHostedService` to NServiceBus `FeatureStartupTask`: ServiceControl hosts many background tasks. `IHostedService` and `FeatureStartupTask` are both abstractions for building background tasks. `FeatureStartupTask` implementations are tied to the lifecycle of an endpoint. `IHostedService` is tied to the lifecycle of the host application, and we prefer it in general. NOTE: ServiceControl starts `IHostedService` implementations in the order of registration, which gives more control over the startup sequence, and shuts them down in the reverse order. `FeatureStartupTask` implementations start in an order based on `Feature` activation, so controlling the startup sequence requires configuring feature dependencies. They also shut down in reverse order. +- Prefer `IServiceCollection` over `IConfigureComponents` and `IContainerBuilder`: Where possible, use the Microsoft DI abstraction (`IServiceCollection`) instead of the NServiceBus one (`IConfigureComponents`) or the Autofac one (`IContainerBuilder`). + - When registering components from within an NServiceBus feature, using `IConfigureComponents` is still appropriate. Consider moving the code out of the NServiceBus feature. `IConfigureComponents` was [deprecated in NServiceBus version 8](https://github.com/Particular/NServiceBus/blob/335ed21dc7d230406d675bd61570b903a69c879c/src/NServiceBus.Core/obsoletes-v8.cs#L192). + - When relying on Autofac-specific features, use `IContainerBuilder`. Prefer to implement features in a way that does not rely on Autofac-specific features. In the future, we may decide to remove our dependency on Autofac. +- Prefer `IServiceProvider` over `ILifetimeService` and `IContainer`: As above, use the Microsoft DI abstraction where possible and use `ILifetimeService` only where strictly necessary. + - When relying on an Autofac-specific feature, use `ILifetimeService`. + - Never use `IContainer`. It is functionally equivalent to `IServiceProvider` and was [deprecated in NServiceBus version 8](https://github.com/Particular/NServiceBus/blob/335ed21dc7d230406d675bd61570b903a69c879c/src/NServiceBus.Core/obsoletes-v8.cs#L252). ### Exceptions -There is one exception to the preference for Microsoft abstractions - -- **Use NServiceBus Logging abstractions over Microsoft or NLog abstractions**: The existing ServiceControl code uses the static `LogManager` classes to get access to the logging infrastructure and all new code should follow this pattern. In the future we are likely to switch to the Microsoft abstraction but until then we want to maintain consistency. - +Logging is the one exception to the preference for Microsoft abstractions. Use the NServiceBus logging abstractions instead of the Microsoft or NLog abstractions. The existing ServiceControl code uses the static `LogManager` classes to access the logging infrastructure, and all new code should follow this pattern for consistency. In the future we are likely to switch to the Microsoft abstraction. ## Prefer explicit container registration -Where possible, we prefer explicit container registration for services instead of convention-based registration. This provides better visibility of which classes belong to which ServiceControl components or to which part of the ServiceControl infrastructure. It also gives us more explicit control over which services are available within the container, which helps to reduce inappropriate cross-component access. In the future we may be able to make this more explicit by moving ServiceControl components into their own assemblies and keeping non-shared services internal. +Where possible, prefer explicit container registration for services over convention-based registration. Explicit registration shows which classes belong to which ServiceControl components or to which part of the ServiceControl infrastructure. It also controls which services are available within the container, which reduces inappropriate cross-component access. In the future we may be able to make this more explicit by moving ServiceControl components into their own assemblies and keeping non-shared services internal. ### Exceptions -There are a few things that are still registered using convention. Note that these are all registered using Autofac and not the Microsoft DI abstractions. +A few things are still registered by convention. Autofac registers them, not the Microsoft DI abstractions. -- **API Controllers** -- **Scatter-Gather API components** +- API controllers +- Scatter-gather API components -Additionally, because NServiceBus does a type-scan at startup it will automatically register any implementations of `Feature` and `IHandleMessage<>`. We have chosen to leave this alone as we would be fighting with NServiceBus in order to turn this off. +NServiceBus also scans types at startup and automatically registers any implementations of `Feature` and `IHandleMessage<>`. We leave this unchanged because turning it off would work against NServiceBus. ## Prefer explicit persistence operations -Use a direct data-store method when all inputs for a persistence operation fit in one method call. The method should own and dispose its EF Core scope/context or RavenDB session, accept a `CancellationToken`, and commit before returning. Returned entities are detached snapshots; callers must not be required to mutate tracked entities as an implicit persistence command. +Use a direct data-store method when all inputs for a persistence operation fit in one method call. The method owns and disposes its EF Core scope or context or its RavenDB session, accepts a `CancellationToken`, and commits before returning. Returned entities are detached snapshots. Callers must not need to mutate tracked entities as an implicit persistence command. -Atomic operations that can encounter concurrency conflicts should document their provider guarantees and translate expected provider exceptions into explicit domain outcomes. For example, a unique-key or optimistic-concurrency conflict should not escape when contention is part of the operation's normal contract. +Atomic operations that can encounter concurrency conflicts document their provider guarantees and translate expected provider exceptions into explicit domain outcomes. For example, a unique-key or optimistic-concurrency conflict should not escape when contention is part of the normal contract of the operation. -Reserve a specialized unit of work for cases where a caller genuinely composes several writes into one atomic batch. New unit-of-work APIs should consistently provide: +Use a specialized unit of work only when a caller composes several writes into one atomic batch. New unit-of-work APIs provide: -- an `I...UnitOfWorkFactory`; -- a `StartNew` factory method; -- a `Complete(CancellationToken)` commit method; -- `IAsyncDisposable` lifetime ownership; -- explicit operation-recording methods rather than mutation of tracked return values; -- documented commit, abandon, repeated-completion, and concurrency behavior. +- an `I...UnitOfWorkFactory` +- a `StartNew` factory method +- a `Complete(CancellationToken)` commit method +- `IAsyncDisposable` lifetime ownership +- explicit operation-recording methods rather than mutation of tracked return values +- documented commit, abandon, repeated-completion, and concurrency behavior Do not introduce generic `IDataSessionManager`-style abstractions or persistence managers with hidden call-order protocols. During review, prefer one explicit store operation unless caller-composed atomicity requires a unit of work. ## Avoid property injection -Although the Autofac container can be configured to allow property injection, we prefer to avoid it. There is no way to specify property injection using the Microsoft DI abstractions, and the default `IServiceProvider` implementation does not support it. Where possible, use constructor injection instead. +Avoid property injection, although the Autofac container can be configured to allow it. The Microsoft DI abstractions cannot specify property injection, and the default `IServiceProvider` implementation does not support it. Where possible, use constructor injection. ### Exceptions -There are a few places where property injection is still used. These are all registered using Autofac, and not the Microsoft DI abstractions. +A few places still use property injection. Autofac registers them, not the Microsoft DI abstractions. -- **API Controllers** -- **Scatter-Gather API components** +- API controllers +- Scatter-gather API components diff --git a/docs/data-versioning-design.md b/docs/data-versioning-design.md index dadb89dbcb..44935db1c8 100644 --- a/docs/data-versioning-design.md +++ b/docs/data-versioning-design.md @@ -10,9 +10,9 @@ This is the primary (error) instance only. The audit instance still carries a `s ## The one rule -**If a field the response renders can change without the version changing, a client caches that page for ever and nothing reveals it.** No log line, no exception, no failing test. +**If a field the response renders can change without the version changing, a client caches that page forever and nothing reveals it.** No log line, no exception, no failing test. -The rule covers fields that can change on their own. A field that is a **pure function of a covered field** cannot: it moves only when its source does, and the source already moves the version, so the page is never stale on the field's own account. `CustomCheckView.Internal` is that case — it is a computed, get-only property classified out of `CustomCheckId` at read time (see `InternalCustomCheckClassification`), which is itself a version term, and the view rather than the stored `CustomCheck` is what `/api/customchecks` renders. Being get-only, it cannot be assigned at all, so the reflection test above never sees it as a field that could drift. The one residual window is a ServiceControl upgrade that reclassifies while a client holds a pre-upgrade tag, and it closes itself: internal checks re-report every 5s to 1h, which moves `ReportedAt` and therefore the version. +The rule covers fields that can change on their own. A field that is a **pure function of a covered field** cannot: it moves only when its source does, and the source already moves the version, so the page is never stale on the field's own account. `CustomCheckView.Internal` is that case. It is a computed, get-only property classified out of `CustomCheckId` at read time (see `InternalCustomCheckClassification`), which is itself a version term. The view, not the stored `CustomCheck`, is what `/api/customchecks` renders. Being get-only, it cannot be assigned at all, so the reflection test above never sees it as a field that could drift. The one residual window is a ServiceControl upgrade that reclassifies while a client holds a pre-upgrade tag, and it closes itself: internal checks re-report every 5s to 1h, which moves `ReportedAt` and therefore the version. The promise is scoped to **one URL**, because a client only ever sends a validator back to the URL that issued it. So what must never happen is one URL answering `304` when its own body would have differed. Two different URLs sharing a value is harmless: an HTTP cache is keyed on the whole URL. @@ -37,9 +37,9 @@ Every term's value, and every field inside a row, is **length prefixed**. Withou ## Absence -`DataVersion.None` is `default`, and it means there is no version to offer. Two parties that both know nothing have not established that nothing changed, so **absence must never answer `304`**: an empty validator that matched itself would serve a cached page for every request for ever. +`DataVersion.None` is `default`, and it means there is no version to offer. Two parties that both know nothing have not established that nothing changed, so **absence must never answer `304`**: an empty validator that matched itself would serve a cached page for every request forever. -`None` means "no answer", not "no rows", and the difference matters. A query that found nothing still produces a real version, because a list always contributes a summary term and `Compose` over `[("messages", 0)]` is as good a validator as any other. So an empty page is cacheable, and a client watching something that stays empty gets its `304`. What produces `None` is a question that was never answered: a store that has no token of its own to offer, a remote instance that timed out or refused the call, a response whose `ETag` header was absent or unparseable. +`None` means "no answer", not "no rows". A query that found nothing still produces a real version, because a list always contributes a summary term and `Compose` over `[("messages", 0)]` is as good a validator as any other. So an empty page is cacheable, and a client watching something that stays empty gets its `304`. What produces `None` is a question that was never answered: a store that has no token of its own to offer, a remote instance that timed out or refused the call, a response whose `ETag` header was absent or unparseable. `Combine` returns `None` as soon as any instance reports none, and that is why: an instance reporting none is one whose data could not be seen at all, so no promise can be made about it. It is not an instance reporting that it is empty, which would come with a version like anything else. diff --git a/docs/deployment.md b/docs/deployment.md index 0686efa97f..6d2c0bb893 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -1,32 +1,32 @@ # Deployment -All ServiceControl deployment options listed below rely on the files produced by [packaging](packaging.md). During packaging, deployment binaries are created in the `deploy` directory, some of which are packaged into zip files in the `zips` folder, and other deployment artifacts are built from these pieces. +All ServiceControl deployment options listed below rely on the files produced by [packaging](packaging.md). Packaging creates the deployment binaries in the `deploy` directory. Some of them are packaged into zip files in the `zips` folder, and other deployment artifacts are built from these pieces. ## ServiceControl installer -The zips are packaged as embedded resources in the ServiceControl Management Utility. Although technically embedded in a specific assembly, ServiceControl Management is shipped as a self-contained, single-file executable that has everything inside it. +The zips are packaged as embedded resources in the ServiceControl Management Utility. Although the zips are embedded in a specific assembly, ServiceControl Management ships as a self-contained, single-file executable that contains everything. When installing an instance, the following binaries are unzipped from the embedded resources and combined to form a complete instance: -- Application & persistence files +- Application and persistence files - All transport assemblies (not just the selected one) - All files from `InstanceShared.zip` that are common to all 3 application instances - RavenDB server files (ServiceControl and Audit only) -A configuration file generated based on: +The installer also generates a configuration file based on: - User input, defaults, and previous settings -- In some cases, hints from the `transport.manifest` or `persistence.manifest` such as config keys that are no longer used. +- In some cases, hints from the `transport.manifest` or `persistence.manifest`, such as config keys that are no longer used ## PowerShell module -The PowerShell module is built during the release workflow in the `deploy/PowerShellModules` directory. During the release process, this is pushed to the [PowerShell Gallery](https://www.powershellgallery.com/packages/Particular.ServiceControl.Management/). +The PowerShell module is built during the release workflow in the `deploy/PowerShellModules` directory. The release process pushes it to the [PowerShell Gallery](https://www.powershellgallery.com/packages/Particular.ServiceControl.Management/). ## Docker images -The release workflow builds multi-arch Docker images with a specific version tag and pushes the images to the [GitHub Container Registry(https://github.com/Particular/ServiceControl/packages)]. Packages are distributed to Docker Hub with multiple tags (i.e. `latest`, `5`, `5.4`, and `5.4.0`) during the release process using the [push container images workflow](/.github/workflows/push-container-images.yml). +The release workflow builds multi-arch Docker images with a specific version tag and pushes the images to the [GitHub Container Registry](https://github.com/Particular/ServiceControl/packages). The release process distributes the packages to Docker Hub with multiple tags (for example `latest`, `5`, `5.4`, and `5.4.0`) using the [push container images workflow](/.github/workflows/push-container-images.yml). -Images are availble in the following Docker Hub repositories: +Images are available in the following Docker Hub repositories: - [servicecontrol](https://hub.docker.com/r/particular/servicecontrol) - [servicecontrol-audit](https://hub.docker.com/r/particular/servicecontrol-audit) @@ -35,6 +35,6 @@ Images are availble in the following Docker Hub repositories: ## NuGet package -The binaries are shipped to the [PlatformSample](https://github.com/Particular/Particular.PlatformSample) via the `Particular.PlatformSample.ServiceControl` NuGet package with transport hardcoded to `LearningTransport` and persister hardcoded to `RavenDB`. In order to avoid shipping RavenDB binaries twice, the platform sample hosts its own RavenDB.Embedded instance and connects both the ServiceControl and Audit instances to it. +The binaries are shipped to the [PlatformSample](https://github.com/Particular/Particular.PlatformSample) via the `Particular.PlatformSample.ServiceControl` NuGet package with the transport hard-coded to `LearningTransport` and the persister hard-coded to `RavenDB`. To avoid shipping RavenDB binaries twice, the platform sample hosts its own RavenDB.Embedded instance and connects both the ServiceControl and Audit instances to it. diff --git a/docs/error-ingestion-design.md b/docs/error-ingestion-design.md index 3b5e9a7963..e9fcbcab6b 100644 --- a/docs/error-ingestion-design.md +++ b/docs/error-ingestion-design.md @@ -29,12 +29,12 @@ repeatedly keeps a single row that records: This is a deliberate difference from the document-database persister, which retained an array of attempts. The read side only ever consumed the last attempt plus the count, so storing the full -history earned nothing and cost write amplification. It is also an improvement: because the count +history had no benefit and cost write amplification. It is also an improvement: because the count is a column rather than the length of a capped array, `NumberOfProcessingAttempts` always reports the true number of attempts, where Raven's implementation silently stopped counting once the retained array hit its cap of ten. -### Stored source data vs derived columns +### Stored source data and derived columns Only two pieces of data are **stored as source of truth**: the full headers dictionary (`HeadersJson`) and the message body. Every other column (message type, endpoints, exception @@ -43,7 +43,7 @@ and queried. On read, the `FailureDetails` object and the metadata dictionary th system expects are **reconstructed** from the headers and these columns. Nothing downstream of ingestion reads a column expecting it to carry information the headers do not already contain. -`BodyUrl`, `ContentType`, and `ContentLength` are examples worth calling out: the document store +`BodyUrl`, `ContentType`, and `ContentLength` are examples: the document store persisted them into a metadata dictionary, but they are all derivable (`BodyUrl` from the `UniqueMessageId`, the other two from the `BodyContentType` and `BodySize` columns), so they are not stored again. @@ -60,7 +60,7 @@ Bodies are **always** stored. The `MaxBodySizeToStore` setting (default 100 KB) `BodyText` is null, regardless of size. When `BodyStoredExternally` is true the external copy is authoritative and `BodyText` is a -search aid only; it must never be served as the body. `BodySize` is always the true original +search aid only; it must never be served as the body. `BodySize` is always the original size. External writes happen before the row that points at them is committed. ### Groups, endpoints, retention @@ -115,10 +115,10 @@ use them: the group delete and the retry resolution are ordinary set-based EF op `IFailedMessageIngestionSqlDialect` seam. Three requirements together force that, and no ORM-level API satisfies all three at once. -### 1. The upsert is a conditional merge, not a save +### 1. The upsert is a conditional merge Writing a failed message is not "insert this row" or "update this row". For a message that already -exists the statement must, in one shot: +exists the statement must, in one statement: - flip the status back to `Unresolved`, and reset the retention clock **only** if the row was previously resolved or archived, @@ -146,14 +146,14 @@ follow: key. - **Read-modify-write cost.** EF's optimistic concurrency (a rowversion/xmin token) would catch a conflicting write instead of silently losing it, but only via a read before every write and a - retry loop per message, the per-row round trip reason 3 rules out, and it still can't express the + retry loop per message, the per-row round trip reason 3 rules out, and it still cannot express the conditional merge from reason 1. Closing the insert race requires the database's own concurrency-safe upsert primitive, and those are **provider-specific**: -- PostgreSQL: `INSERT ... ON CONFLICT (unique_message_id) DO UPDATE`. The conflict clause makes a - concurrent insert fall through to the update instead of failing, and the whole statement is +- PostgreSQL: `INSERT ... ON CONFLICT (unique_message_id) DO UPDATE`. The conflict clause turns a + concurrent insert into an update instead of a failure, and the whole statement is atomic so the count arithmetic cannot lose an increment. - SQL Server: `MERGE ... WITH (HOLDLOCK)`. The lock hint serializes concurrent merges on the same key so the second one sees the row and updates instead of colliding. @@ -172,7 +172,7 @@ that the database can cache a plan for, with no per-row round trips and no tempo ### What stays portable -Only the genuinely divergent statements are raw. The retry resolution and the group delete are set +Only the divergent statements are raw SQL. The retry resolution and the group delete are set based and identical across providers, so they remain EF operations in the shared writer. Failed message upserts and insert-if-absent group and endpoint writes are owned by each provider's `IFailedMessageIngestionSqlDialect` implementation. Insert-if-absent retry claims belong to the @@ -186,13 +186,11 @@ from drifting. Everything in a batch runs in one transaction opened by the writer. The raw dialect commands are explicitly enlisted onto that transaction, and the EF `ExecuteUpdate`/`ExecuteDelete` operations -participate in it as well, so a failure anywhere rolls the whole batch back. Nothing is committed -piecemeal. +participate in it as well, so a failure anywhere rolls the whole batch back. The transaction is wrapped in the provider's execution strategy so that a transient failure (a dropped connection, or a deadlock between concurrent writers) retries the **entire** batch. This is safe because the batch is **idempotent**: re-running it folds to the same rows, the upsert is a merge, the group rows are deleted and re-inserted, endpoints are insert-if-absent, and retry resolution is a set update plus delete. Replaying a batch after an ambiguous commit changes -nothing. Stable lock ordering (the fold sorts by `UniqueMessageId`) keeps deadlocks rare in the -first place. +nothing. Stable lock ordering (the fold sorts by `UniqueMessageId`) keeps deadlocks rare. diff --git a/docs/eventlog-design.md b/docs/eventlog-design.md index b9b9f8208f..f7589f6c7d 100644 --- a/docs/eventlog-design.md +++ b/docs/eventlog-design.md @@ -4,11 +4,11 @@ The event log is the primary instance's activity feed: the chronological "what has this instance noticed" list that ServicePulse shows. Message failures, retries, redirects, heartbeats, custom checks and integration failures all surface here. -It is a **projection of domain events, not a log file**. Nothing writes to it directly. Components raise domain events for their own reasons, and the event log turns a chosen subset of those into feed items. An event only appears if someone has declared how it should read, which makes the feed an editorial selection rather than a dump. +It is a **projection of domain events, not a log file**. Nothing writes to it directly. Components raise domain events for their own reasons, and the event log turns a chosen subset of those into feed items. An event appears only if someone has declared how it should read, so the feed is an editorial selection. Only the primary instance has an event log. Audit and monitoring instances have none. -Four contracts define the whole part: `EventLogItem` (what is written), `EventLogItemView` (what is read), `EventLogMappingDefinition` (how a component declares an event belongs in the feed), and `IEventLogDataStore` (the storage seam). Everything else is machinery behind them. +Four contracts define the event log: `EventLogItem` (what is written), `EventLogItemView` (what is read), `EventLogMappingDefinition` (how a component declares an event belongs in the feed), and `IEventLogDataStore` (the storage seam). ## What gets recorded @@ -20,20 +20,20 @@ Four contracts define the whole part: `EventLogItem` (what is written), `EventLo ## Declaring an event -A component makes one of its events visible with two things, and nothing in the event log changes: +A component makes one of its events visible with two things. The event log itself does not change: -1. A class deriving **directly** from `EventLogMappingDefinition`. Derive through an intermediate non-generic base and the definition is silently skipped at registration. +1. A class deriving **directly** from `EventLogMappingDefinition`. A definition that derives through an intermediate non-generic base is silently skipped at registration. 2. A matching `services.AddEventLogMapping()` in that component's configuration. Two definitions for one event type is an error. -A definition is a **declarative builder, not a handler**: its constructor calls `Description(…)`, and optionally `RaisedAt(…)`, `Severity(…)` / `TreatAsError()` and the `RelatesTo*` helpers, to specify how one row reads. Definitions live with the component that raises the event: **this part owns the machinery, the components own the content**. An event with no declaration is ignored, deliberately and silently. +A definition is a **declarative builder, not a handler**: its constructor calls `Description(…)`, and optionally `RaisedAt(…)`, `Severity(…)` / `TreatAsError()` and the `RelatesTo*` helpers, to specify how one row reads. Definitions live with the component that raises the event: the event log owns the machinery, and the components own the content. The event log deliberately and silently ignores an event with no declaration. ## Reading the feed `GET /api/eventlogitems` is the entire HTTP surface, gated on `Permissions.ErrorEventLogView`. It takes `page` and `pageSize`, returns one page of `EventLogItemView` newest first, and sets `Total-Count`, `ETag` and `Link`. There is no write, no delete, no per-item lookup, and no filtering or search: `Category`, `Severity` and `EventType` are returned but cannot be queried on. -Clients discover new items by **polling**; nothing is pushed, and recording an item has no outward effect at all. Because polling is the only path, a caller that echoes its `ETag` back as `If-None-Match` gets `304` with no body, and the request costs a header exchange instead of a page of JSON. +Clients discover new items by **polling**. Nothing is pushed, and recording an item has no outward effect. A caller that echoes its `ETag` back as `If-None-Match` gets `304` with no body, so the request costs a header exchange instead of a page of JSON. -Timestamps are when the thing happened, so an item can land in the middle of the feed rather than at its head. +Timestamps are when the thing happened, so an item can land in the middle of the feed instead of at its head. ## The storage seam @@ -44,12 +44,12 @@ Timestamps are when the thing happened, so an item can land in the middle of the ## Retention -Items age out on their own after `EventsRetentionPeriod`, 14 days by default. Nothing a user does removes one and there is no API to try. Enforcement is left to each storage backend and is invisible through the seam, so a change to the setting applies retrospectively on some backends and to new items only on others. +Items age out after `EventsRetentionPeriod`, 14 days by default. Users cannot remove an item, and no API exists for it. Enforcement is left to each storage backend and is invisible through the seam, so a change to the setting applies retrospectively on some backends and to new items only on others. -## Failure behaviour +## Failure behavior Recording is **in-band, not best-effort**. Domain event dispatch rethrows, so if storage is unreachable the failure propagates to whatever raised the event and that operation fails. There is no retry, no queue and no buffer in front of the write. ## Known limits -- **The feed is per error instance.** A federated deployment gets no aggregation and nothing in the response says which instance answered. +- The feed is per error instance. A federated deployment gets no aggregation and nothing in the response says which instance answered. diff --git a/docs/forward-headers-testing.md b/docs/forward-headers-testing.md index fb13a55d1a..b7baaa2505 100644 --- a/docs/forward-headers-testing.md +++ b/docs/forward-headers-testing.md @@ -1,4 +1,4 @@ -# Local Testing Forwarded Headers (Without NGINX) +# Local testing of forwarded headers without NGINX This guide explains how to test forwarded headers configuration for ServiceControl instances without using NGINX or Docker. This approach uses curl to manually send `X-Forwarded-*` headers directly to the instances. @@ -9,7 +9,7 @@ This guide explains how to test forwarded headers configuration for ServiceContr - (Optional) For formatted JSON output: `npm install -g json` then pipe curl output through `| json` - All commands assume you are in the respective project directory -## Enabling Debug Logs +## Enabling debug logs To enable detailed logging for troubleshooting, set the `LogLevel` environment variable before starting each instance: @@ -26,74 +26,74 @@ set MONITORING_LOGLEVEL=Debug **Valid log levels:** `Trace`, `Debug`, `Information` (or `Info`), `Warning` (or `Warn`), `Error`, `Critical` (or `Fatal`), `None` (or `Off`) -Debug logs will show detailed forwarded headers processing and trust evaluation information. +Debug logs show detailed forwarded headers processing and trust evaluation information. -## Instance Reference +## Instance reference -| Instance | Project Directory | Default Port | Environment Variable Prefix | +| Instance | Project directory | Default port | Environment variable prefix | |---------------------------|---------------------------------|--------------|-----------------------------| | ServiceControl (Primary) | `src\ServiceControl` | 33333 | `SERVICECONTROL_` | | ServiceControl.Audit | `src\ServiceControl.Audit` | 44444 | `SERVICECONTROL_AUDIT_` | | ServiceControl.Monitoring | `src\ServiceControl.Monitoring` | 33633 | `MONITORING_` | > [!NOTE] -> Environment variables must include the instance prefix (e.g., `SERVICECONTROL_FORWARDEDHEADERS_ENABLED` for the primary instance). +> Environment variables must include the instance prefix (for example `SERVICECONTROL_FORWARDEDHEADERS_ENABLED` for the primary instance). -## How Forwarded Headers Work +## How forwarded headers work When a ServiceControl instance is behind a reverse proxy, the proxy sends headers to indicate the original request details: -- `X-Forwarded-For` - Original client IP address -- `X-Forwarded-Proto` - Original protocol (http/https) -- `X-Forwarded-Host` - Original host header +- `X-Forwarded-For`: Original client IP address +- `X-Forwarded-Proto`: Original protocol (http/https) +- `X-Forwarded-Host`: Original host header -Each instance can be configured to trust these headers from specific proxies or trust all proxies. +You can configure each instance to trust these headers from specific proxies or from all proxies. -### Trust Evaluation Rules +### Trust evaluation rules The middleware determines whether to process forwarded headers based on these rules: -1. **If `TrustAllProxies` = true**: All requests are trusted, headers are always processed -2. **If `TrustAllProxies` = false**: The caller's IP must match **either**: - - **KnownProxies**: Exact IP address match (e.g., `127.0.0.1`, `::1`) - - **KnownNetworks**: CIDR range match (e.g., `127.0.0.0/8`, `10.0.0.0/8`) +1. If `TrustAllProxies` = true: All requests are trusted, and headers are always processed. +2. If `TrustAllProxies` = false: The IP of the caller must match **either**: + - KnownProxies: Exact IP address match (for example `127.0.0.1`, `::1`) + - KnownNetworks: CIDR range match (for example `127.0.0.0/8`, `10.0.0.0/8`) > [!IMPORTANT] -> KnownProxies and KnownNetworks use **OR logic** - a match in either grants trust. The check is against the **immediate caller's IP** (the proxy connecting to ServiceControl), not the original client IP from `X-Forwarded-For`. +> KnownProxies and KnownNetworks use **OR logic**: a match in either grants trust. The check is against the **immediate caller's IP** (the proxy connecting to ServiceControl), not the original client IP from `X-Forwarded-For`. -## Configuration Methods +## Configuration methods Settings can be configured via: -1. **Environment variables** (recommended for testing) - Easy to change between scenarios, no file edits needed -2. **App.config** - Persisted settings, requires app restart after changes +1. Environment variables (recommended for testing): Easy to change between scenarios, no file edits needed +2. App.config: Persisted settings, requires an app restart after changes Both methods work identically. This guide uses environment variables for convenience during iterative testing. -## Test Scenarios +## Test scenarios > [!IMPORTANT] > Set environment variables in the same terminal where you run `dotnet run`. Environment variables are scoped to the terminal session and won't be seen if you run from Visual Studio or a different terminal. > Check the application startup logs to verify which settings were applied. The forwarded headers configuration is logged at startup. -### Test Grouping by Configuration +### Test grouping by configuration To minimize service restarts during testing, scenarios are grouped by configuration. Run all tests within a group before changing configuration: -| Configuration Group | Scenarios | Description | +| Configuration group | Scenarios | Description | |----------------------------------------|--------------------|--------------------------------------------------------------| -| **Group A**: Default/TrustAllProxies | 0, 1, 2, 8, 11, 13 | Tests with default settings or explicit TrustAllProxies=true | -| **Group B**: KnownProxies (localhost) | 3, 9, 14 | Tests with KnownProxies=127.0.0.1,::1 | -| **Group C**: KnownNetworks (localhost) | 4 | Tests with KnownNetworks=127.0.0.0/8,::1/128 | -| **Group D**: Untrusted Proxy | 5 | Tests with KnownProxies=192.168.1.100 | -| **Group E**: Untrusted Network | 6 | Tests with KnownNetworks=10.0.0.0/8,192.168.0.0/16 | -| **Group F**: Disabled | 7 | Tests with Enabled=false | -| **Group G**: Combined | 10 | Tests with both KnownProxies and KnownNetworks | -| **Group H**: IPv4 Only | 12 | Tests with KnownProxies=127.0.0.1 (no IPv6) | +| Group A: Default/TrustAllProxies | 0, 1, 2, 8, 11, 13 | Tests with default settings or explicit TrustAllProxies=true | +| Group B: KnownProxies (localhost) | 3, 9, 14 | Tests with KnownProxies=127.0.0.1,::1 | +| Group C: KnownNetworks (localhost) | 4 | Tests with KnownNetworks=127.0.0.0/8,::1/128 | +| Group D: Untrusted proxy | 5 | Tests with KnownProxies=192.168.1.100 | +| Group E: Untrusted network | 6 | Tests with KnownNetworks=10.0.0.0/8,192.168.0.0/16 | +| Group F: Disabled | 7 | Tests with Enabled=false | +| Group G: Combined | 10 | Tests with both KnownProxies and KnownNetworks | +| Group H: IPv4 only | 12 | Tests with KnownProxies=127.0.0.1 (no IPv6) | --- -## Group A: Default/TrustAllProxies Configuration +## Group A: Default/TrustAllProxies configuration **Start the instance once, then run all tests in this group (Scenarios 0, 1, 2, 8, 11, 13).** @@ -119,7 +119,7 @@ set MONITORING_FORWARDEDHEADERS_KNOWNNETWORKS= dotnet run ``` -### Scenario 0: Direct Access (No Proxy) +### Scenario 0: Direct access (no proxy) Test a direct request without any forwarded headers, simulating access without a reverse proxy. @@ -161,7 +161,7 @@ curl http://localhost:33633/debug/request-info | json When no forwarded headers are sent, the request values remain unchanged. -### Scenario 1: Default Behavior (With Headers) +### Scenario 1: Default behavior (with headers) Test the default behavior when no forwarded headers environment variables are set, but headers are sent. @@ -203,9 +203,9 @@ curl -H "X-Forwarded-Proto: https" -H "X-Forwarded-Host: example.com" -H "X-Forw By default, forwarded headers are **enabled** and **all proxies are trusted**. This means any client can spoof `X-Forwarded-*` headers. This is suitable for development but should be restricted in production by configuring `KnownProxies` or `KnownNetworks`. -### Scenario 2: Trust All Proxies (Explicit) +### Scenario 2: Trust all proxies (explicit) -Explicitly enable trust all proxies (same as default, but explicit configuration). This scenario can be tested with the same Group A configuration - the behavior is identical. +Explicitly enable trust all proxies. This is the same as the default but set explicitly. Test this scenario with the same Group A configuration. The behavior is identical. **Test with curl (using Group A configuration above):** @@ -245,7 +245,7 @@ curl -H "X-Forwarded-Proto: https" -H "X-Forwarded-Host: example.com" -H "X-Forw The `scheme` is `https` (from `X-Forwarded-Proto`), `host` is `example.com` (from `X-Forwarded-Host`), and `remoteIpAddress` is `203.0.113.50` (from `X-Forwarded-For`) because all proxies are trusted. The `rawHeaders` are empty because the middleware consumed them. -### Scenario 8: Proxy Chain (Multiple X-Forwarded-For Values) +### Scenario 8: Proxy chain (multiple X-Forwarded-For values) Test how ServiceControl handles multiple proxies in the chain. @@ -287,7 +287,7 @@ curl -H "X-Forwarded-Proto: https" -H "X-Forwarded-Host: example.com" -H "X-Forw The `X-Forwarded-For` header contains multiple IPs representing the proxy chain. When `TrustAllProxies` is `true`, `ForwardLimit` is set to `null` (no limit), so the middleware processes all IPs and returns the original client IP (`203.0.113.50`). -### Scenario 11: Partial Headers (Proto Only) +### Scenario 11: Partial headers (proto only) Test that each forwarded header is processed independently. Only sending `X-Forwarded-Proto` should update the scheme while leaving host and remoteIpAddress unchanged. @@ -329,7 +329,7 @@ curl -H "X-Forwarded-Proto: https" http://localhost:33633/debug/request-info | j Only the `scheme` changed to `https`. The `host` remains `localhost:33333` and `remoteIpAddress` remains `::1` because those headers weren't sent. Each header is processed independently. -### Scenario 13: Multiple X-Forwarded-Proto and X-Forwarded-Host Values +### Scenario 13: Multiple X-Forwarded-Proto and X-Forwarded-Host values Test how ServiceControl handles multiple values in `X-Forwarded-Proto` and `X-Forwarded-Host` headers, which can occur in multi-proxy environments where each proxy adds its own values. @@ -373,7 +373,7 @@ When `TrustAllProxies` is `true`, `ForwardLimit` is set to `null` (no limit), so --- -## Group B: KnownProxies (Localhost) Configuration +## Group B: KnownProxies (localhost) configuration **Restart the instance with this configuration, then run all tests in this group (Scenarios 3, 9, 14).** @@ -402,11 +402,11 @@ dotnet run > [!NOTE] > Setting `KNOWNPROXIES` automatically disables `TRUSTALLPROXIES`. Both IPv4 (`127.0.0.1`) and IPv6 (`::1`) loopback addresses are included since curl may use either. -### Scenario 3: Known Proxies Only +### Scenario 3: Known proxies only Only accept forwarded headers from specific IP addresses. -**Test with curl (from localhost - should work):** +**Test with curl (from localhost, should work):** ```cmd rem ServiceControl (Primary) @@ -444,7 +444,7 @@ curl -H "X-Forwarded-Proto: https" -H "X-Forwarded-Host: example.com" -H "X-Forw Headers are applied because the request comes from localhost, which is in the known proxies list. The `rawHeaders` are empty because the middleware consumed them. -### Scenario 9: Proxy Chain with Known Proxies (ForwardLimit = 1) +### Scenario 9: Proxy chain with known proxies (ForwardLimit = 1) Test how ServiceControl handles multiple proxies when `TrustAllProxies` is `false`. In this case, `ForwardLimit` remains at its default of `1`, so only the last proxy IP is processed. @@ -486,7 +486,7 @@ curl -H "X-Forwarded-Proto: https" -H "X-Forwarded-Host: example.com" -H "X-Forw When `TrustAllProxies` is `false`, `ForwardLimit` remains at its default of `1`. The middleware only processes the rightmost IP from the chain (`192.168.1.1`). The remaining IPs (`203.0.113.50, 10.0.0.1`) stay in the `X-Forwarded-For` header. Compare this to Scenario 8 where `TrustAllProxies = true` returns the original client IP. -### Scenario 14: Multiple Header Values with Known Proxies (ForwardLimit = 1) +### Scenario 14: Multiple header values with known proxies (ForwardLimit = 1) Test how ServiceControl handles multiple `X-Forwarded-Proto` and `X-Forwarded-Host` values when `TrustAllProxies` is `false`. In this case, `ForwardLimit` remains at its default of `1`, so only the rightmost value is processed. @@ -530,7 +530,7 @@ When `TrustAllProxies` is `false`, `ForwardLimit` remains at its default of `1`. --- -## Group C: KnownNetworks (Localhost) Configuration +## Group C: KnownNetworks (localhost) configuration **Restart the instance with this configuration (Scenario 4).** @@ -559,7 +559,7 @@ dotnet run > [!NOTE] > Both IPv4 (`127.0.0.0/8`) and IPv6 (`::1/128`) loopback networks are included since curl may use either. -### Scenario 4: Known Networks (CIDR) +### Scenario 4: Known networks (CIDR) Trust all proxies within a network range. @@ -603,7 +603,7 @@ Headers are applied because the request comes from localhost, which falls within --- -## Group D: Untrusted Proxy Configuration +## Group D: Untrusted proxy configuration **Restart the instance with this configuration (Scenario 5).** @@ -629,7 +629,7 @@ set MONITORING_FORWARDEDHEADERS_KNOWNNETWORKS= dotnet run ``` -### Scenario 5: Unknown Proxy Rejected +### Scenario 5: Unknown proxy rejected Configure a known proxy that doesn't match the request source to verify headers are ignored. @@ -673,7 +673,7 @@ Headers are **ignored** because the request comes from localhost (`::1`), which --- -## Group E: Untrusted Network Configuration +## Group E: Untrusted network configuration **Restart the instance with this configuration (Scenario 6).** @@ -699,7 +699,7 @@ set MONITORING_FORWARDEDHEADERS_KNOWNNETWORKS=10.0.0.0/8,192.168.0.0/16 dotnet run ``` -### Scenario 6: Unknown Network Rejected +### Scenario 6: Unknown network rejected Configure a known network that doesn't match the request source to verify headers are ignored. @@ -743,7 +743,7 @@ Headers are **ignored** because the request comes from localhost (`::1`), which --- -## Group F: Disabled Configuration +## Group F: Disabled configuration **Restart the instance with this configuration (Scenario 7).** @@ -769,7 +769,7 @@ set MONITORING_FORWARDEDHEADERS_KNOWNNETWORKS= dotnet run ``` -### Scenario 7: Forwarded Headers Disabled +### Scenario 7: Forwarded headers disabled Completely disable forwarded headers processing. @@ -813,7 +813,7 @@ Headers are ignored because forwarded headers processing is disabled entirely. N --- -## Group G: Combined Proxies and Networks Configuration +## Group G: Combined proxies and networks configuration **Restart the instance with this configuration (Scenario 10).** @@ -839,7 +839,7 @@ set MONITORING_FORWARDEDHEADERS_KNOWNNETWORKS=127.0.0.0/8,::1/128 dotnet run ``` -### Scenario 10: Combined Known Proxies and Networks +### Scenario 10: Combined known proxies and networks Test using both `KnownProxies` and `KnownNetworks` together. @@ -883,7 +883,7 @@ Headers are applied because the request comes from localhost (`::1`), which fall --- -## Group H: IPv4 Only Configuration +## Group H: IPv4 only configuration **Restart the instance with this configuration (Scenario 12).** @@ -912,7 +912,7 @@ dotnet run > [!NOTE] > Only IPv4 `127.0.0.1` is configured, not IPv6 `::1`. -### Scenario 12: IPv4/IPv6 Mismatch +### Scenario 12: IPv4/IPv6 mismatch Demonstrates a common misconfiguration where only IPv4 localhost is configured but curl uses IPv6. This scenario shows why you should include both `127.0.0.1` and `::1` in your configuration. @@ -952,12 +952,12 @@ curl -H "X-Forwarded-Proto: https" -H "X-Forwarded-Host: example.com" -H "X-Forw } ``` -Headers are **ignored** because the request comes from `::1` (IPv6), but only `127.0.0.1` (IPv4) is in the known proxies list. This is a common gotcha - always include both IPv4 and IPv6 loopback addresses when testing locally, or use CIDR notation like `127.0.0.0/8` and `::1/128`. +Headers are **ignored** because the request comes from `::1` (IPv6), but only `127.0.0.1` (IPv4) is in the known proxies list. This is a common mistake. Always include both IPv4 and IPv6 loopback addresses when testing locally, or use CIDR notation like `127.0.0.0/8` and `::1/128`. > [!NOTE] > If your output shows headers were applied, curl is using IPv4. The behavior depends on your system's DNS resolution for `localhost`. -## Debug Endpoint +## Debug endpoint The `/debug/request-info` endpoint is only available in Development environment. It returns: @@ -995,11 +995,11 @@ The `/debug/request-info` endpoint is only available in Development environment. | `configuration` | `knownProxies` | List of trusted proxy IP addresses | | `configuration` | `knownNetworks` | List of trusted CIDR network ranges | -### Key Diagnostic Questions +### Key diagnostic questions -1. **Were headers applied?** - If `rawHeaders` are empty but `processed` values changed, the middleware consumed and applied them -2. **Why weren't headers applied?** - If `rawHeaders` still contain values, the middleware didn't trust the caller. Check `knownProxies` and `knownNetworks` in `configuration` -3. **Is forwarded headers enabled?** - Check `configuration.enabled` +1. Were headers applied? If `rawHeaders` are empty but `processed` values changed, the middleware consumed and applied them +2. Why weren't headers applied? If `rawHeaders` still contain values, the middleware didn't trust the caller. Check `knownProxies` and `knownNetworks` in `configuration` +3. Is forwarded headers processing enabled? Check `configuration.enabled` ## Cleanup @@ -1027,7 +1027,7 @@ set MONITORING_FORWARDEDHEADERS_KNOWNPROXIES= set MONITORING_FORWARDEDHEADERS_KNOWNNETWORKS= ``` -## Unit Tests +## Unit tests Unit tests for the `ForwardedHeadersSettings` configuration class are located at: @@ -1035,7 +1035,7 @@ Unit tests for the `ForwardedHeadersSettings` configuration class are located at src/ServiceControl.UnitTests/Infrastructure/Settings/ForwardedHeadersSettingsTests.cs ``` -## Acceptance Tests +## Acceptance tests Acceptance tests for end-to-end forwarded headers behavior are located at: @@ -1046,10 +1046,10 @@ src/ServiceControl.Monitoring.AcceptanceTests/Security/ForwardedHeaders/ ``` > [!NOTE] -> Scenario 12 (IPv4/IPv6 Mismatch) is not covered by acceptance tests because the test server's IP address (IPv4 vs IPv6) cannot be controlled reliably. The "untrusted proxy" behavior is already validated by Scenarios 5 and 6. +> Scenario 12 (IPv4/IPv6 mismatch) is not covered by acceptance tests because the test server's IP address (IPv4 vs IPv6) cannot be controlled reliably. The "untrusted proxy" behavior is already validated by Scenarios 5 and 6. -## See Also +## See also -- [Hosting Guide](https://docs.particular.net/servicecontrol/security/hosting-guide) - Configuration reference for forwarded headers -- [Reverse Proxy Testing](reverseproxy-testing.md) - Testing with a real reverse proxy (NGINX) -- [Testing Architecture](testing-architecture.md) - Overview of testing patterns in this repository +- [Hosting Guide](https://docs.particular.net/servicecontrol/security/hosting-guide): Configuration reference for forwarded headers +- [Local testing with NGINX reverse proxy](reverseproxy-testing.md): Testing with a real reverse proxy (NGINX) +- [Testing Architecture](testing-architecture.md): Overview of testing patterns in this repository diff --git a/docs/handling-unavailable-runtime-dependencies.md b/docs/handling-unavailable-runtime-dependencies.md index f89bb6c0f7..1d7a82b180 100644 --- a/docs/handling-unavailable-runtime-dependencies.md +++ b/docs/handling-unavailable-runtime-dependencies.md @@ -1,20 +1,19 @@ # Unavailable runtime dependencies -ServiceControl instances (Main, Audit, and Monitoring) have various runtime dependencies (persistence, transport, etc) and they are expected to handle their unavailability in a predictable way. Causes of dependencies being unavailable include: +ServiceControl instances (Main, Audit, and Monitoring) have runtime dependencies such as persistence and transport. They must handle the unavailability of these dependencies in a predictable way. Causes of unavailable dependencies include: -* invalid instance configuration e.g. connection string, secrets +* invalid instance configuration, for example connection string or secrets * network outages -* invalid network configuration e.g. firewall misconfiguration -* data store failures e.g. broken indexes, DB process failures -* missing and/or invalid permissions -* uninitialized state e.g. missing indexes, missing queues -* etc. +* invalid network configuration, for example firewall misconfiguration +* data store failures, for example broken indexes or database process failures +* missing or invalid permissions +* uninitialized state, for example missing indexes or missing queues -Such cases will render some (or all) of the functionalities offered by the platform as unavailable. +These cases make some or all of the functionality of the platform unavailable. ## How ServiceControl handles unavailable runtime dependencies -ServiceControl instances handle unavailable runtime dependencies in two different ways, depending on the time at which the failure scenario is detected: +ServiceControl instances handle unavailable runtime dependencies in one of two ways, depending on when they detect the failure: -* Failures detected during startup -> instance stops immediately -* Failures detected after a successful startup -> instance does not stop and is expected to try and recover from the failure once a dependency becomes available again +* A failure detected during startup stops the instance immediately. +* A failure detected after a successful startup does not stop the instance. The instance is expected to try to recover once the dependency becomes available again. diff --git a/docs/https-testing.md b/docs/https-testing.md index feedc268d3..3395c3e193 100644 --- a/docs/https-testing.md +++ b/docs/https-testing.md @@ -1,20 +1,20 @@ -# Local Testing with Direct HTTPS +# Local testing with direct HTTPS -This guide provides scenario-based tests for ServiceControl's direct HTTPS features. Use this to verify Kestrel HTTPS behavior without a reverse proxy. +This guide provides scenario-based tests for the direct HTTPS features of ServiceControl. Use it to verify Kestrel HTTPS behavior without a reverse proxy. > [!NOTE] -> HTTP to HTTPS redirection (`RedirectHttpToHttps`) is designed for reverse proxy scenarios where the proxy forwards HTTP requests to ServiceControl. When running with direct HTTPS, ServiceControl only binds to a single port (HTTPS). To test HTTP to HTTPS redirection, see [Reverse Proxy Testing](reverseproxy-testing.md). +> HTTP to HTTPS redirection (`RedirectHttpToHttps`) is designed for reverse proxy scenarios where the proxy forwards HTTP requests to ServiceControl. When running with direct HTTPS, ServiceControl only binds to a single port (HTTPS). To test HTTP to HTTPS redirection, see [Local testing with NGINX reverse proxy](reverseproxy-testing.md). -## Instance Reference +## Instance reference -| Instance | Project Directory | Default Port | Environment Variable Prefix | App.config Key Prefix | +| Instance | Project directory | Default port | Environment variable prefix | App.config key prefix | |---------------------------|---------------------------------|--------------|-----------------------------|-------------------------| | ServiceControl (Primary) | `src\ServiceControl` | 33333 | `SERVICECONTROL_` | `ServiceControl/` | | ServiceControl.Audit | `src\ServiceControl.Audit` | 44444 | `SERVICECONTROL_AUDIT_` | `ServiceControl.Audit/` | | ServiceControl.Monitoring | `src\ServiceControl.Monitoring` | 33633 | `MONITORING_` | `Monitoring/` | > [!NOTE] -> Environment variables must include the instance prefix (e.g., `SERVICECONTROL_HTTPS_ENABLED` for the primary instance). +> Environment variables must include the instance prefix (for example `SERVICECONTROL_HTTPS_ENABLED` for the primary instance). ## Prerequisites @@ -23,7 +23,7 @@ This guide provides scenario-based tests for ServiceControl's direct HTTPS featu - curl (included with Windows 10/11, Git Bash, or WSL) - (Optional) For formatted JSON output: `npm install -g json` then pipe curl output through `| json` -## Enabling Debug Logs +## Enabling debug logs To enable detailed logging for troubleshooting, set the `LogLevel` environment variable before starting each instance: @@ -40,7 +40,7 @@ set MONITORING_LOGLEVEL=Debug **Valid log levels:** `Trace`, `Debug`, `Information` (or `Info`), `Warning` (or `Warn`), `Error`, `Critical` (or `Fatal`), `None` (or `Off`) -Debug logs will show detailed HTTPS configuration and certificate loading information. +Debug logs show detailed HTTPS configuration and certificate loading information. ### Installing mkcert @@ -77,16 +77,16 @@ After installing, run `mkcert -install` to install the local CA in your system t ## Setup -### Step 1: Create the Local Development Folder +### Step 1: Create the local development folder -Create a `.local` folder in the repository root (this folder is gitignored): +Create a `.local` folder in the repository root (Git ignores this folder): ```bash mkdir .local mkdir .local/certs ``` -### Step 2: Generate PFX Certificates +### Step 2: Generate PFX certificates Kestrel requires certificates in PFX format. Use mkcert to generate them: @@ -101,20 +101,20 @@ cd .local/certs mkcert -p12-file localhost.pfx -pkcs12 localhost 127.0.0.1 ::1 servicecontrol servicecontrol-audit servicecontrol-monitor ``` -When prompted for a password, you can use an empty password by pressing Enter, or set a password (e.g., `changeit`) and note it for the configuration step. +When mkcert prompts for a password, press Enter to use an empty password, or set a password (for example `changeit`) and note it for the configuration step. -## Test Scenarios +## Test scenarios All scenarios use environment variables for configuration. > [!NOTE] -> The `RemoteInstances` setting on the primary ServiceControl instance needs the correct schema. e.g.; `https://localhost:44444/api/` +> The `RemoteInstances` setting on the primary ServiceControl instance needs the correct scheme, for example `https://localhost:44444/api/` -### Test Grouping by Configuration +### Test grouping by configuration -Both scenarios use the same HTTPS configuration, so you only need to start the service once to run all tests. +Both scenarios use the same HTTPS configuration, so start each instance once and run all tests. -## HTTPS Enabled Configuration +## HTTPS enabled configuration **Start the instance once, then run all tests (Scenarios 1, 2).** @@ -150,7 +150,7 @@ set MONITORING_FORWARDEDHEADERS_ENABLED=false dotnet run ``` -### Scenario 1: Basic HTTPS Connectivity +### Scenario 1: Basic HTTPS connectivity Verify that HTTPS is working with a valid certificate. @@ -178,9 +178,9 @@ curl --ssl-no-revoke -v https://localhost:33633/ 2>&1 | findstr /C:"HTTP/" /C:"S < HTTP/1.1 200 OK ``` -The request succeeds over HTTPS. The exact SSL output varies by curl version and platform, but you should see `HTTP/1.1 200 OK` confirming success. +The exact SSL output varies by curl version and platform. `HTTP/1.1 200 OK` confirms success. -### Scenario 2: HTTP Disabled (HTTPS Only) +### Scenario 2: HTTP disabled (HTTPS only) Verify that HTTP requests fail when only HTTPS is enabled. @@ -218,7 +218,7 @@ Ensure the `CertificatePath` is an absolute path and the file exists. If you set a password when generating the PFX, ensure it matches `CertificatePassword` in the config. -### Certificate errors in browser/curl +### Certificate errors in the browser or curl 1. Ensure mkcert's root CA is installed: `mkcert -install` 2. Restart your browser after installing the root CA @@ -271,8 +271,8 @@ set MONITORING_HTTPS_HSTSINCLUDESUBDOMAINS= set MONITORING_FORWARDEDHEADERS_ENABLED= ``` -## See Also +## See also -- [Hosting Guide](https://docs.particular.net/servicecontrol/security/hosting-guide) - Detailed configuration reference for all deployment scenarios -- [Reverse Proxy Testing](reverseproxy-testing.md) - Testing with a reverse proxy (NGINX) -- [Forwarded Headers Testing](forward-headers-testing.md) - Testing forwarded headers without a reverse proxy +- [Hosting Guide](https://docs.particular.net/servicecontrol/security/hosting-guide): Detailed configuration reference for all deployment scenarios +- [Local testing with NGINX reverse proxy](reverseproxy-testing.md): Testing with a reverse proxy (NGINX) +- [Local testing of forwarded headers without NGINX](forward-headers-testing.md): Testing forwarded headers without a reverse proxy diff --git a/docs/ingestion-pipeline.md b/docs/ingestion-pipeline.md index 2f86068966..d747db318d 100644 --- a/docs/ingestion-pipeline.md +++ b/docs/ingestion-pipeline.md @@ -35,14 +35,13 @@ messages already in flight finish and their receives commit. It then completes t waits for it to drain, and only then tears the transport infrastructure down, because batches still forward through its dispatcher. -A hard cancellation does not silently drop what is in flight. The assembler abandons the batch it +On a hard cancellation, ServiceControl still answers every receive in flight. The assembler abandons the batch it was building and whatever is left in the message channel, any batch no writer picked up is -abandoned, and a writer fails the batch it was holding. Every one of those receives is answered, -so they are redelivered rather than left waiting for a shutdown that has already happened. +abandoned, and a writer fails the batch it was holding. The transport redelivers those messages instead of leaving them waiting for a shutdown that has already happened. ## Settings -Named per instance, and read from that instance's settings root +Each instance reads its settings from its own settings root (`ServiceControl/...` and `ServiceControl.Audit/...`). | Setting | Default | Range | What it does | @@ -75,7 +74,7 @@ with smaller ones. ### Batch timeout -Zero means a partial batch is written rather than waited on, which is what the ingestion did before +Zero means a partial batch is written without waiting, which is what the ingestion did before the setting existed. A non-zero value trades latency for fewer, larger writes: at volume it costs nothing, because a full batch never waits, and at a trickle it delays each message by up to the timeout. Start at 100ms if a storage is clearly happier with larger batches. It cannot make a batch @@ -83,13 +82,12 @@ larger than the transport's concurrency, only fuller. ### Parallel writers -Only raise this for a storage whose writes are safe to interleave. Batches commit in whatever order -they finish, so this is not a free throughput knob, and the pipeline will not let you turn it on -where it is unsafe. +Raise this only for a storage whose writes are safe to interleave. Batches commit in whatever order +they finish, so this is not a free throughput knob. The pipeline does not let you turn it on where it is unsafe. -More than one writer also means more than one batch is being enriched and announced at a time, so a -custom `IEnrichImportedErrorMessages` or `IEnrichImportedAuditMessages` has to be thread safe. A -single writer used to serialise them. +More than one writer also means more than one batch is enriched and announced at a time, so a +custom `IEnrichImportedErrorMessages` or `IEnrichImportedAuditMessages` has to be thread-safe. A +single writer used to serialize them. ## Which storages take concurrent batches diff --git a/docs/load-testing.md b/docs/load-testing.md index 55d3f0f5c3..a16ac48585 100644 --- a/docs/load-testing.md +++ b/docs/load-testing.md @@ -1,26 +1,26 @@ -# Load Testing +# Load testing -The [ServiceControl Testing Tool](../tools/testing-tool/README.md) is a stateless .NET service that generates error load and real-world failure scenarios against a test ServiceControl instance, to validate error-ingestion performance. It ships with a web UI for manual control, built-in failure scenarios (third-party outage, timeout spike, poison message, and more), and OpenTelemetry telemetry. +The [ServiceControl Testing Tool](../tools/testing-tool/README.md) is a stateless .NET service that generates error load and real-world failure scenarios against a test ServiceControl instance, to validate error-ingestion performance. It has a web UI for manual control, built-in failure scenarios (third-party outage, timeout spike, poison message, and more), and OpenTelemetry telemetry. -This guide covers getting it up and running quickly. For the full reference — every scenario, API endpoint, configuration setting, container usage, and scaling notes — see the [testing tool README](../tools/testing-tool/README.md). +The [testing tool README](../tools/testing-tool/README.md) is the full reference: every scenario, API endpoint, configuration setting, container usage, and scaling notes. ## Quick start: full stack with Aspire -The Aspire AppHost brings up the whole system in a single command: the ServiceControl platform (Learning transport), the testing tool, and a complete observability stack (OTel Collector, Jaeger, Prometheus, and Grafana with a prebuilt dashboard). +The Aspire AppHost starts the whole system with a single command: the ServiceControl platform (Learning transport), the testing tool, and a complete observability stack (OTel Collector, Jaeger, Prometheus, and Grafana with a prebuilt dashboard). ```bash aspire run tools/testing-tool/TestingTool.AppHost/TestingTool.AppHost.csproj ``` -Aspire assigns dynamic ports — open the testing tool's web UI via the link in the Aspire dashboard. +Aspire assigns dynamic ports. Open the web UI of the testing tool through the link in the Aspire dashboard. Useful flags, passed after `--`: | Flag | Description | |---|---| -| `--tag ` | Test a specific ServiceControl image tag, e.g. a PR prerelease like `pr-1234` | +| `--tag ` | Test a specific ServiceControl image tag, for example a PR prerelease like `pr-1234` | | `--persistence ` | Persistence for the error instance: `RavenDb` (default), `SqlServer`, or `PostgreSql` | -| `--error-ingestion-scale-unit ` | Spin up `n` additional error-ingestion-only ServiceControl instances | +| `--error-ingestion-scale-unit ` | Start `n` additional error-ingestion-only ServiceControl instances | ## Run against an existing ServiceControl @@ -29,19 +29,19 @@ dotnet build tools/testing-tool/TestingTool.slnx --configuration Release dotnet run --project tools/testing-tool/TestingTool --configuration Release ``` -Open http://localhost:5290. The tool defaults to a ServiceControl instance at `http://localhost:33333`; point it elsewhere with the `TestingTool__ServiceControlApiUrl` environment variable. +Open http://localhost:5290. The tool defaults to a ServiceControl instance at `http://localhost:33333`; to use another instance, set the `TestingTool__ServiceControlApiUrl` environment variable. ## Generating load Everything is controllable from the web UI, or via the HTTP API: -- **Scenarios** — the five built-in failure scenarios (`third-party-outage`, `timeout-spike`, `poison-message`, `deserialization-failure`, `background-noise`), each producing naturally-grouped errors. Start with `POST /api/scenarios/{name}/start` (optional `rate` and `durationSeconds`). -- **Bypass writer** — high-throughput load that writes failed-message envelopes directly to the ServiceControl error queue, skipping the message handler (`POST /api/bypass/start`). -- **Jobs** — retry, archive, search, retention sweep, and custom-check-failure jobs exercise ServiceControl's recoverability, FTS search, and retention pipelines against the load. Jobs do not auto-start; kick them off from the UI or `/api/jobs`. -- **Release-test presets** — named presets mapped from the [testing scenarios](testing-scenarios.md), e.g. `POST /api/release-tests/ingestion-load/start`. +- Scenarios: the five built-in failure scenarios (`third-party-outage`, `timeout-spike`, `poison-message`, `deserialization-failure`, `background-noise`), each producing errors that group naturally. Start with `POST /api/scenarios/{name}/start` (optional `rate` and `durationSeconds`). +- Bypass writer: high-throughput load that writes failed-message envelopes directly to the ServiceControl error queue, skipping the message handler (`POST /api/bypass/start`). +- Jobs: retry, archive, search, retention sweep, and custom-check-failure jobs exercise ServiceControl's recoverability, FTS search, and retention pipelines against the load. Jobs do not start automatically. Start them from the UI or `/api/jobs`. +- Release-test presets: named presets mapped from the [testing scenarios](testing-scenarios.md), for example `POST /api/release-tests/ingestion-load/start`. ## Observing the run - The Aspire dashboard shows logs and traces for every service. -- Grafana (auto-provisioned; log in with `admin`/`admin`) ships a prebuilt "Testing Tool" dashboard — errors/sec by scenario, search latency p95, replay/archive rates, and errors raised vs ingested. -- Jaeger for trace analysis, Prometheus for metrics. The tool also exposes a Prometheus scraping endpoint at `/metrics`. +- Grafana (auto-provisioned; log in with `admin`/`admin`) has a prebuilt "Testing Tool" dashboard with errors/sec by scenario, search latency p95, replay/archive rates, and errors raised versus ingested. +- Jaeger provides trace analysis and Prometheus provides metrics. The tool also exposes a Prometheus scraping endpoint at `/metrics`. diff --git a/docs/multipleservicecontrolinstancescommunication.md b/docs/multipleservicecontrolinstancescommunication.md index e11c2ec95c..1506389da2 100644 --- a/docs/multipleservicecontrolinstancescommunication.md +++ b/docs/multipleservicecontrolinstancescommunication.md @@ -10,15 +10,15 @@ A `NewEndpointDetected` event is published when a node's Heartbeat component det ### MessageFailureResolvedByRetry -A `MessageFailureResolvedByRetry` event is published when an audit message is detected that contains ServiceControl retry headers. The primary instance subscribes to this event on order to be able to mark the failed message record as successfully retried. +A `MessageFailureResolvedByRetry` event is published when an audit message is detected that contains ServiceControl retry headers. The primary instance subscribes to this event in order to mark the failed message record as successfully retried. ## Scatter-gather HTTP interactions -The second category of communication patterns is HTTP-based scatter-gather. In this pattern the primary instance executes the request locally and in addition to that, fans it out to all registered secondary instances. Then the primary instance combines all the responses and forwards it back to the client. +The second category of communication patterns is HTTP-based scatter-gather. In this pattern the primary instance executes the request locally and also sends it to all registered secondary instances. The primary instance then combines all the responses and returns the result to the client. ### GetKnownEndpointsApi -This API is used by ServiceInsight to show endpoint-based filtering options. +This API is used by ServiceInsight to show endpoint-based filtering options. ### GetSagaByIdApi @@ -26,7 +26,7 @@ This API is used by ServiceInsight's saga view. ### ScatterGatherApiMessageView -This category of API calls group all return messages based on certain criteria such as ID, correlation ID or other. It includes the following API calls: +These API calls return all messages that match criteria such as ID or correlation ID. They include: * `GetAllMessagesApi` * `GetAllMessagesForEndpointApi` @@ -34,8 +34,8 @@ This category of API calls group all return messages based on certain criteria s * `SearchApi` * `SearchEndpointApi` -Apart from simply aggregating the results from the secondary instances, these calls manipulate certain bits of the response content, namely: - +In addition to aggregating the results from the secondary instances, these calls change parts of the response content: + * Rewrite the returned message body URL to include the ID of node that contains a given message record * Add an attribute containing the ID of the node that contains a given message record @@ -49,4 +49,4 @@ When a message is being retried, ServiceInsight includes the node ID in the retr ### GetBodyByIdApi -When the ServiceInsight fetches the body of a message it uses the URL provided by ServiceControl which includes the ID of nodes that contains the message record. +When ServiceInsight fetches the body of a message, it uses the URL provided by ServiceControl. The URL includes the ID of the node that contains the message record. diff --git a/docs/multiversion-multiinstance-smoke-test.md b/docs/multiversion-multiinstance-smoke-test.md index b50cd3fe58..4c12b4788e 100644 --- a/docs/multiversion-multiinstance-smoke-test.md +++ b/docs/multiversion-multiinstance-smoke-test.md @@ -1,33 +1,35 @@ ## Setup 1. Install ServiceControl version 2. -1. Add 1 instance of ServiceControl v2 with MSMQ. It would help to use a name relating it to v3, e.g. `v2_SC` - - Configure a unique error queue name. It would help to use a name relating it to v2, e.g. `v2_error` - - Configure a unique audit queue name. It would help to use a name relating it to v2, e.g. `v2_audit` -1. Install ServiceControl version 3 -1. Add 1 instance of ServiceControl v3 with MSMQ. It would help to use a name relating it to v3, e.g. `v3_SC` - - Configure a unique error queue name. It would help to use a name relating it to v3, e.g. `v3_error` - - Configure a unique audit queue name. It would help to use a name relating it to v3, e.g. `v3_audit` -1. Download or checkout the [FaultTolerance](https://docs.particular.net/samples/faulttolerance/) sample. +1. Add one instance of ServiceControl v2 with MSMQ. Use a name that relates to v2, for example `v2_SC`. + - Configure a unique error queue name. Use a name that relates to v2, for example `v2_error`. + - Configure a unique audit queue name. Use a name that relates to v2, for example `v2_audit`. +1. Install ServiceControl version 3. +1. Add one instance of ServiceControl v3 with MSMQ. Use a name that relates to v3, for example `v3_SC`. + - Configure a unique error queue name. Use a name that relates to v3, for example `v3_error`. + - Configure a unique audit queue name. Use a name that relates to v3, for example `v3_audit`. +1. Download or check out the [FaultTolerance](https://docs.particular.net/samples/faulttolerance/) sample. 1. Edit the configuration of the sample project - Reconfigure the endpoint to use the MSMQ transport - -## V2 remote notifies V3 master about successful retry + +## V2 remote notifies V3 master about successful retry + 1. Configure ServiceControl v3 as Remote. 1. Restart v3 SC -1. Configure ServiceControl v2 as Master. +1. Configure ServiceControl v2 as Master. 1. Restart v2 SC 1. Edit the configuration of the sample project - - Configure the error queue to the unique error queue assigned to v2 instance of SC - - Configure auditing to the unique audit queue assigned to v3 instance of SC. -1. Run the sample, sending error messages to the v2 instance of SC -1. Set the sample to successfully process messages + - Configure the error queue to the unique error queue assigned to the v2 instance of SC + - Configure auditing to the unique audit queue assigned to the v3 instance of SC +1. Run the sample. It sends error messages to the v2 instance of SC. +1. Set the sample to process messages successfully 1. Connect ServiceInsight to the v2 instance of ServiceControl 1. Retry one or more failed messages in ServiceInsight -1. Confirm the message(s) successfully processed in the sample -1. Confirm the message status is Resolved in ServiceInsight +1. Confirm that the sample processed the messages successfully +1. Confirm that the message status is Resolved in ServiceInsight + +## Reset -## Reset 1. Stop the v2 instance of SC 1. Remove the database directory of the v2 instance of SC 1. Start the v2 instance of SC @@ -36,16 +38,17 @@ 1. Start the v3 instance of SC ## V3 remote notifies V2 master about successful retry + 1. Configure ServiceControl v2 as Remote. 1. Restart v2 SC -1. Configure ServiceControl v3 as Master. +1. Configure ServiceControl v3 as Master. 1. Restart v3 SC 1. Edit the configuration of the sample project - - Configure the error queue to the unique error queue assigned to v3 instance of SC - - Configure auditing to the unique audit queue assigned to v2 instance of SC. -1. Run the sample, sending error messages to the v3 instance of SC -1. Set the sample to successfully process messages + - Configure the error queue to the unique error queue assigned to the v3 instance of SC + - Configure auditing to the unique audit queue assigned to the v2 instance of SC +1. Run the sample. It sends error messages to the v3 instance of SC. +1. Set the sample to process messages successfully 1. Connect ServiceInsight to the v3 instance of ServiceControl 1. Retry one or more failed messages in ServiceInsight -1. Confirm the message(s) successfully processed in the sample -1. Confirm the message status is Resolved in ServiceInsight +1. Confirm that the sample processed the messages successfully +1. Confirm that the message status is Resolved in ServiceInsight diff --git a/docs/packaging.md b/docs/packaging.md index 53a366ed41..28bc624912 100644 --- a/docs/packaging.md +++ b/docs/packaging.md @@ -1,8 +1,8 @@ # Packaging -Each product (ServiceControl, ServiceControl.Monitoring and ServiceControl.Audit), is packaged into its own versioned zip file in the `zip` folder. These zip files are included as resources in the ServiceControlInstaller.Engine project, to be used to create new app instances from both ServiceControl Management as well as the PowerShell module. +Each product (ServiceControl, ServiceControl.Monitoring, and ServiceControl.Audit) is packaged into its own versioned zip file in the `zip` folder. These zip files are included as resources in the ServiceControlInstaller.Engine project. ServiceControl Management and the PowerShell module use them to create new app instances. -The zip files are crafted to minimize duplication in order to control the overall file size of each installer. The zips for each app contain only the specific app code as well as persistence code unique to that application. +The zip files minimize duplication to control the overall file size of each installer. The zip file for each app contains only the specific app code and the persistence code unique to that application. - `ServiceControl.zip` - `ServiceControl.Audit.zip` @@ -12,19 +12,19 @@ The zip files are crafted to minimize duplication in order to control the overal ## The mechanics -The [Microsoft.Build.Artifacts](https://github.com/microsoft/MSBuildSdks/tree/main/src/Artifacts) package is used to define artifacts that are placed into the `deploy` folder when the solution is built. Each project that contributes artifacts has an `Artifact` definition in its project file. -To ensure proper build ordering, the `ServiceControlInstaller.Packaging` project needs to have a `ProjectReference` to every project that has an artifact definition. +The [Microsoft.Build.Artifacts](https://github.com/microsoft/MSBuildSdks/tree/main/src/Artifacts) package defines artifacts that the solution build places into the `deploy` folder. Each project that contributes artifacts has an `Artifact` definition in its project file. +To ensure the correct build order, the `ServiceControlInstaller.Packaging` project needs a `ProjectReference` to every project that has an artifact definition. -Every project that uses the artifacts then has to have a build ordering `ProjectReference` to the `ServiceControlInstaller.Packaging` project. The projects that use the artifacts are: +Every project that uses the artifacts needs a build-order `ProjectReference` to the `ServiceControlInstaller.Packaging` project. The projects that use the artifacts are: - The `ServiceControlInstaller.Engine` project to create the above-mentioned required zip files - The `Particular.PlatformSample.ServiceControl` project to create the Platform sample required NuGet package ## Assembly version mismatches -There can be an issue when the main instance folder and the selected transport/persister component each have a copy of the same assembly but reference different versions. At install time, one version or the other will be copied into the instance binary folder and things may break unexpectedly at runtime. +There can be an issue when the main instance folder and the selected transport/persister component each have a copy of the same assembly but reference different versions. At install time, one of the two versions is copied into the instance binary folder, and the instance may fail at runtime. -To prevent this, the unit test `DeploymentPackageTests.DuplicateAssemblyShouldHaveMatchingVersions` tests if duplicated assemblies might be deployed. If their versions match, then the test passes. If not then the test will fail with: +To prevent this, the unit test `DeploymentPackageTests.DuplicateAssemblyShouldHaveMatchingVersions` checks assemblies that could be deployed twice. The test passes if their versions match. Otherwise it fails with: ``` Component assembly version mismatch detected @@ -34,4 +34,4 @@ To prevent this, the unit test `DeploymentPackageTests.DuplicateAssemblyShouldHa ### How to resolve -The repo uses [NuGet central package management](https://learn.microsoft.com/en-us/nuget/consume-packages/central-package-management) to ensure that the same version of dependencies are used in each project. When a test fails with a version mismatch, add the package that provides the assembly to the `Versions to pin transitive references` ItemGroup in the `Directory.packages.props` file. \ No newline at end of file +The repo uses [NuGet central package management](https://learn.microsoft.com/en-us/nuget/consume-packages/central-package-management) so that each project uses the same version of a dependency. When the test fails with a version mismatch, add the package that provides the assembly to the `Versions to pin transitive references` ItemGroup in the `Directory.packages.props` file. \ No newline at end of file diff --git a/docs/retries-asq-transport.md b/docs/retries-asq-transport.md index c3482f9bb9..e3c6461d86 100644 --- a/docs/retries-asq-transport.md +++ b/docs/retries-asq-transport.md @@ -1,26 +1,28 @@ -# How ServiceControl Retries Works with regards to AzureStorageQueue Transport +# How ServiceControl retries work with the Azure Storage Queues transport -To get better understanding how retry mechanism works in ServiceControl refer to [How ServiceControl Retries Works] (https://github.com/Particular/ServiceControl/blob/master/docs/bulk-retries-design.md) +For how the retry mechanism works in ServiceControl, see [How ServiceControl retries work](https://github.com/Particular/ServiceControl/blob/master/docs/bulk-retries-design.md). -## When using one storage account - * Both endpoints exist in the same storage account as well as error queue - * Messages that are failed residing in error queue contains in `FailedQ` header name of the queue of the receiver - * ServiceControl while doing retry is calling send method with destination set to queue used in `FailedQ` header - * everything works as expected +## When using one storage account + +* Both endpoints and the error queue exist in the same storage account. +* A failed message in the error queue contains the queue name of the receiver in the `FailedQ` header. +* During a retry, ServiceControl calls the send method with the destination set to the queue in the `FailedQ` header. +* Retries work as expected. ## When using multiple storage accounts -This section contains analysis of multiple storage account support when doing retry. - -### Each endpoint resides in separate storage account - * If every account has it's own error queue (that is situated in corresponding storage accounts) - * An instance per storage account will be required - * Failed messages can be retried as the queue where it failed is in the same storage account as destination queue - * Each instance of SC can see only part of the conversation - - - * If there is one error queue on different storage account - * ServiceControl need to have only 1 instance connected to storage account that has error queue - * Failed messages can not be retried at this point of time, as SC would pass the value of FailedQ to Send method as destination. Operation would fail. As the SC uses v6 of ASQ transport in the end it calls: + +This section analyzes multiple storage account support for retries. + +### Each endpoint resides in a separate storage account + +* If every account has its own error queue, located in the same storage account: + * One ServiceControl instance per storage account is required. + * Failed messages can be retried, because the queue where the message failed is in the same storage account as the destination queue. + * Each ServiceControl instance sees only part of the conversation. + +* If there is one error queue in a different storage account: + * ServiceControl needs only one instance, connected to the storage account that has the error queue. + * Failed messages cannot be retried at this point in time. ServiceControl passes the value of `FailedQ` to the send method as the destination, and the operation fails. ServiceControl uses v6 of the ASQ transport, which ends in a call to https://github.com/Particular/NServiceBus.AzureStorageQueues/blob/6.2.1/src/Transport/AzureMessageQueueSender.cs#L41 -NOTE: For SC to work FailedQ field need to contains connection name after @. \ No newline at end of file +NOTE: For ServiceControl to work, the `FailedQ` field needs to contain the connection name after `@`. diff --git a/docs/reverseproxy-testing.md b/docs/reverseproxy-testing.md index 0e171c5420..a711c151a3 100644 --- a/docs/reverseproxy-testing.md +++ b/docs/reverseproxy-testing.md @@ -1,15 +1,15 @@ -# Local Testing with NGINX Reverse Proxy +# Local testing with NGINX reverse proxy -This guide provides scenario-based tests for ServiceControl instances behind an NGINX reverse proxy. Use this to verify: +This guide provides scenario-based tests for ServiceControl instances behind an NGINX reverse proxy. Use it to verify: - SSL/TLS termination at the reverse proxy - Forwarded headers handling (`X-Forwarded-For`, `X-Forwarded-Proto`, `X-Forwarded-Host`) - HTTP to HTTPS redirection - HSTS (HTTP Strict Transport Security) -## Instance Reference +## Instance reference -| Instance | Project Directory | Default Port | Hostname | Environment Variable Prefix | +| Instance | Project directory | Default port | Hostname | Environment variable prefix | |---------------------------|---------------------------------|--------------|------------------------------------|-----------------------------| | ServiceControl (Primary) | `src\ServiceControl` | 33333 | `servicecontrol.localhost` | `SERVICECONTROL_` | | ServiceControl.Audit | `src\ServiceControl.Audit` | 44444 | `servicecontrol-audit.localhost` | `SERVICECONTROL_AUDIT_` | @@ -22,7 +22,7 @@ This guide provides scenario-based tests for ServiceControl instances behind an - ServiceControl built locally (see [main README for instructions](../README.md#how-to-rundebug-locally)) - curl (included with Windows 10/11, Git Bash, or WSL) -## Enabling Debug Logs +## Enabling debug logs To enable detailed logging for troubleshooting, set the `LogLevel` environment variable before starting each instance: @@ -39,7 +39,7 @@ set MONITORING_LOGLEVEL=Debug **Valid log levels:** `Trace`, `Debug`, `Information` (or `Info`), `Warning` (or `Warn`), `Error`, `Critical` (or `Fatal`), `None` (or `Off`) -Debug logs will show detailed request processing information including forwarded headers handling and HTTPS redirection. +Debug logs show detailed request processing information including forwarded headers handling and HTTPS redirection. ### Installing mkcert @@ -59,16 +59,16 @@ After installing, run `mkcert -install` to install the local CA in your system t ## Setup -### Step 1: Create the Local Development Folder +### Step 1: Create the local development folder -Create a `.local` folder in the repository root (this folder is gitignored): +Create a `.local` folder in the repository root (Git ignores this folder): ```cmd mkdir .local mkdir .local\certs ``` -### Step 2: Generate SSL Certificates +### Step 2: Generate SSL certificates Use mkcert to generate trusted local development certificates: @@ -78,7 +78,7 @@ cd .local\certs mkcert -cert-file local-platform.pem -key-file local-platform-key.pem servicecontrol.localhost servicecontrol-audit.localhost servicecontrol-monitor.localhost localhost ``` -### Step 3: Create Docker Compose Configuration +### Step 3: Create Docker Compose configuration Create `.local/compose.yml`: @@ -95,7 +95,7 @@ services: - ./certs/local-platform-key.pem:/etc/nginx/certs/local-key.pem:ro ``` -### Step 4: Create NGINX Configuration +### Step 4: Create NGINX configuration Create `.local/nginx.conf`: @@ -249,7 +249,7 @@ http { } ``` -### Step 5: Configure Hosts File +### Step 5: Configure hosts file Add the following entries to your hosts file (`C:\Windows\System32\drivers\etc\hosts`): @@ -259,7 +259,7 @@ Add the following entries to your hosts file (`C:\Windows\System32\drivers\etc\h 127.0.0.1 servicecontrol-monitor.localhost ``` -### Step 6: Start the NGINX Reverse Proxy +### Step 6: Start the NGINX reverse proxy From the repository root: @@ -267,7 +267,7 @@ From the repository root: docker compose -f .local/compose.yml up -d ``` -### Step 7: Final Directory Structure +### Step 7: Final directory structure After completing the setup, your `.local` folder should look like: @@ -280,12 +280,12 @@ After completing the setup, your `.local` folder should look like: └── local-platform-key.pem ``` -## Test Scenarios +## Test scenarios > **Important:** ServiceControl must be running before testing. A 502 Bad Gateway error means NGINX cannot reach ServiceControl. -> **Note:** Use `TRUSTALLPROXIES=true` for local Docker testing. The NGINX container's IP address varies based on Docker's network configuration (e.g., `172.x.x.x`), making it impractical to specify a fixed `KNOWNPROXIES` value. +> **Note:** Use `TRUSTALLPROXIES=true` for local Docker testing. The NGINX container's IP address varies based on Docker's network configuration (for example `172.x.x.x`), making it impractical to specify a fixed `KNOWNPROXIES` value. -### Scenario 1: HTTPS Access +### Scenario 1: HTTPS access Verify that HTTPS is working through the reverse proxy. @@ -316,7 +316,7 @@ curl -k -v https://servicecontrol.localhost/api 2>&1 | findstr /C:"HTTP/" The request succeeds over HTTPS through the NGINX reverse proxy. -### Scenario 2: Forwarded Headers Processing +### Scenario 2: Forwarded headers processing Verify that forwarded headers are being processed correctly. @@ -368,7 +368,7 @@ The key indicators that forwarded headers are working: - `processed.host` is `servicecontrol.localhost` (from `X-Forwarded-Host`) - `rawHeaders` are empty because the middleware consumed them (trusted proxy) -### Scenario 3: HTTP to HTTPS Redirect +### Scenario 3: HTTP to HTTPS redirect Verify that HTTP requests are redirected to HTTPS. @@ -432,22 +432,22 @@ curl -k -v https://servicecontrol.localhost/api 2>&1 | findstr /i strict-transpo The HSTS header is present with the default max-age of 1 year. -## Testing Other Instances +## Testing other instances The scenarios above use ServiceControl (Primary). To test ServiceControl.Audit or ServiceControl.Monitoring: -1. Use the appropriate environment variable prefix (see Configuration Reference below) +1. Use the appropriate environment variable prefix (see Configuration reference below) 2. Use the corresponding project directory and hostname -| Instance | Project Directory | Hostname | Env Var Prefix | +| Instance | Project directory | Hostname | Env var prefix | |---------------------------|---------------------------------|------------------------------------|-------------------------| | ServiceControl (Primary) | `src\ServiceControl` | `servicecontrol.localhost` | `SERVICECONTROL_` | | ServiceControl.Audit | `src\ServiceControl.Audit` | `servicecontrol-audit.localhost` | `SERVICECONTROL_AUDIT_` | | ServiceControl.Monitoring | `src\ServiceControl.Monitoring` | `servicecontrol-monitor.localhost` | `MONITORING_` | -## Configuration Reference +## Configuration reference -| Environment Variable | Default | Description | +| Environment variable | Default | Description | |---------------------------------------------|------------|---------------------------------------------| | `{PREFIX}_FORWARDEDHEADERS_ENABLED` | `true` | Enable forwarded headers processing | | `{PREFIX}_FORWARDEDHEADERS_TRUSTALLPROXIES` | `true` | Trust all proxies | @@ -473,7 +473,7 @@ Where `{PREFIX}` is: docker compose -f .local/compose.yml down ``` -### Clear Environment Variables +### Clear environment variables After testing, clear the environment variables: @@ -497,7 +497,7 @@ $env:SERVICECONTROL_HTTPS_PORT = $null $env:SERVICECONTROL_HTTPS_ENABLEHSTS = $null ``` -### Remove Hosts Entries (Optional) +### Remove hosts entries (optional) If you no longer need the hostnames, remove these entries from your hosts file (`C:\Windows\System32\drivers\etc\hosts`): @@ -547,7 +547,7 @@ If using Docker Desktop on Windows with WSL2: The `/debug/request-info` endpoint is only available when running in Development environment (the default when using `dotnet run`). -## See Also +## See also -- [Hosting Guide](https://docs.particular.net/servicecontrol/security/hosting-guide) - Configuration reference for all deployment scenarios -- [Forwarded Headers Testing](forward-headers-testing.md) - Testing forwarded headers without a reverse proxy +- [Hosting Guide](https://docs.particular.net/servicecontrol/security/hosting-guide): Configuration reference for all deployment scenarios +- [Local testing of forwarded headers without NGINX](forward-headers-testing.md): Testing forwarded headers without a reverse proxy diff --git a/docs/telemetry.md b/docs/telemetry.md index e55820a22c..c9f6a98f33 100644 --- a/docs/telemetry.md +++ b/docs/telemetry.md @@ -1,12 +1,12 @@ # Telemetry -Instances can be configured to emit telemetry to aid in performance testing or troubleshooting performance-related issues. +Instances emit telemetry when configured to do so. Use it for performance testing and to troubleshoot performance issues. Both the error and the audit instance report their ingestion the same way. Exporting is configured with the standard [OpenTelemetry environment variables](https://opentelemetry.io/docs/specs/otel/protocol/exporter/#configuration-options), not with instance settings, so the same variables that configure any other OpenTelemetry process apply here. Setting `OTEL_EXPORTER_OTLP_ENDPOINT` is enough to turn metrics on. Both gRPC and HTTP endpoints are supported, and `OTEL_EXPORTER_OTLP_PROTOCOL` selects between them. -The signal-specific variables, `OTEL_EXPORTER_OTLP_METRICS_ENDPOINT` and its siblings, have no effect. The SDK only honours those under `UseOtlpExporter`, and instances use `AddOtlpExporter` so that OTLP applies to metrics without also being turned on for every other signal. +The signal-specific variables, `OTEL_EXPORTER_OTLP_METRICS_ENDPOINT` and its siblings, have no effect. The SDK honors those only under `UseOtlpExporter`, and instances use `AddOtlpExporter` so that OTLP applies to metrics without also being turned on for every other signal. -Logs are exported separately. Add `Otlp` to the instance's `LoggingProviders` setting, which is what turns the OTLP log exporter on, and it then reads the same environment variables for its endpoint. +Logs are exported separately. Add `Otlp` to the `LoggingProviders` setting of the instance to turn the OTLP log exporter on. The exporter reads its endpoint from the same environment variables. The instruments differ only in their prefix and in the categories a message can fall into, so the same dashboard works for both with the prefix swapped. What the batches being measured actually are is covered in [ingestion-pipeline.md](ingestion-pipeline.md). @@ -25,7 +25,7 @@ Meter `Particular.ServiceControl`. - `result` - How the failure was resolved: `retry` or `stored-poison` - `sc.error.ingestion.consecutive_batch_failures_total` - Consecutive batch failures -`ServiceControl/PrintMetrics` predates this and no longer has anything to print. +The `ServiceControl/PrintMetrics` setting no longer prints anything. ### Retry @@ -41,12 +41,12 @@ Every instrument carries `retry.type`, one of `all`, `endpoint`, `group`, `queue - `result` - `success`, `failed`, `empty` for a batch that had no messages left and was discarded, or `cancelled` if shutdown cut the staging short - `sc.retry.forward_duration_seconds` - Forwarding one batch back to the senders - `result` - `success`, `failed`, or `cancelled` if shutdown cut the forwarding short - - `mode` - `counting`, or `timeout` when recovering from a premature shutdown. Timeout mode only ends on the forwarder's 45 second idle timer, so its distribution has a floor at that value. + - `mode` - `counting`, or `timeout` when recovering from a premature shutdown. Timeout mode only ends on the forwarder's 45-second idle timer, so its distribution has a floor at that value. - `sc.retry.messages_total` - Messages moved through the pipeline - `result` - `staged`, `forwarded`, `skipped`, `staging_retried`, or `abandoned` for a message that hit the staging retry limit and was dropped from its batch. `abandoned` is the one to alert on: it is a message the user asked to retry that will not be retried. - `sc.retry.operations_in_progress` - Retry operations currently in progress - `retry.state` - `waiting`, `preparing` or `forwarding` -- `sc.retry.pending_bulk_requests` - Bulk retry requests queued behind each other, drained one per five second tick +- `sc.retry.pending_bulk_requests` - Bulk retry requests queued behind each other, drained one per five-second tick A retry that hangs never records a duration, so on the histograms alone a stuck operation looks identical to no traffic. `operations_in_progress` holding a non-zero value while the duration histograms stay flat is the stuck-operation signal. @@ -103,7 +103,7 @@ What the shapes mean when tuning: failure the watchdog acts on. A gauge climbing while health stays clear means batches are failing and being retried. -Example Grafana dashboard - https://github.com/andreasohlund/Docker/blob/main/otel-monitoring/grafana-platform-template.json +Example Grafana dashboard: https://github.com/andreasohlund/Docker/blob/main/otel-monitoring/grafana-platform-template.json ## Retention @@ -144,7 +144,7 @@ No telemetry is currently available. To emit and visualize RavenDB telemetry: 1. Install a RavenDB developer license (needed to get support for emitting telemetry) -2. [Enable and configure Raven to emit telemetry](https://ravendb.net/docs/article-page/6.2/csharp/server/administration/monitoring/open-telemetry) (the example below shows targeting a local OTEL collector) +2. [Enable and configure RavenDB to emit telemetry](https://ravendb.net/docs/article-page/6.2/csharp/server/administration/monitoring/open-telemetry) (the example below shows targeting a local OTEL collector) ``` environment: RAVEN_Monitoring_OpenTelemetry_Enabled: true @@ -156,10 +156,10 @@ To emit and visualize RavenDB telemetry: ## OTEL Collector -It's recommended to use a local [OTEL Collector](https://opentelemetry.io/docs/collector/) to collect, batch and export the metrics to the relevant observability backend being used. +Use a local [OTEL Collector](https://opentelemetry.io/docs/collector/) to collect, batch, and export the metrics to your observability backend. Example configuration: https://github.com/andreasohlund/Docker/tree/main/otel-monitoring ### Azure Monitor -User the [exporter for Azure Monitor](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/exporter/azuremonitorexporter/README.md) to push telemetry to application insights. +Use the [exporter for Azure Monitor](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/exporter/azuremonitorexporter/README.md) to push telemetry to Application Insights. diff --git a/docs/testing-persistence.md b/docs/testing-persistence.md index 954196b20c..7d59bece46 100644 --- a/docs/testing-persistence.md +++ b/docs/testing-persistence.md @@ -1,6 +1,6 @@ -# Local Testing of Persistence Providers +# Local testing of persistence providers -ServiceControl supports multiple persistence types +ServiceControl supports multiple persistence types: * RavenDB (default) * Microsoft SQL Server @@ -12,10 +12,10 @@ All persistence test projects can be run with `dotnet test` against the correspo The SQL Server and PostgreSQL suites give each test its own **schema** in one shared database, named `sc_test_` for persistence tests and `sc_at_` for acceptance tests, and drop it on teardown. This uses the same `Database/Schema` setting that is offered to customers, so every run exercises that feature. -Two consequences are worth knowing: +Two consequences follow: -* The connection string is used exactly as it is given, so **the database it names must already exist**. The test containers create one; a server you point the environment variable at will not. -* A test failure that leaves a process behind can leave a schema behind with it. `SELECT nspname FROM pg_namespace WHERE nspname LIKE 'sc\_%'` and `SELECT name FROM sys.schemas WHERE name LIKE 'sc[_]%'` will find any strays. +* The connection string is used exactly as it is given, so **the database it names must already exist**. The test containers create one. An existing server that you name in the environment variable does not. +* A test failure that leaves a process behind can leave a schema behind with it. `SELECT nspname FROM pg_namespace WHERE nspname LIKE 'sc\_%'` and `SELECT name FROM sys.schemas WHERE name LIKE 'sc[_]%'` find any leftover schemas. ## RavenDB @@ -32,13 +32,13 @@ Build that image locally before running SQL Server persistence tests: docker buildx build --platform=linux/amd64 --tag particular/servicecontrol-testing-sqlserver:latest ./src/Scripts/Docker/servicecontrol-testing-sqlserver ``` -If you want to use an existing SQL Server instance instead of a test container, set the `ServiceControl_Persistence_SqlServer_ConnectionString` environment variable to a valid SQL Server connection string. It must name a database that exists, not `master`, because the tests create their schemas in whatever database it points at. The test container creates a `ServiceControlTests` database for this. +To use an existing SQL Server instance instead of a test container, set the `ServiceControl_Persistence_SqlServer_ConnectionString` environment variable to a valid SQL Server connection string. It must name a database that exists, not `master`, because the tests create their schemas in whatever database it points at. The test container creates a `ServiceControlTests` database for this. ## PostgreSQL PostgreSQL persistence tests use [Testcontainers](https://testcontainers.com/) and start a `postgres:16-alpine` container automatically. -If you want to use an existing PostgreSQL instance instead of a test container, set: +To use an existing PostgreSQL instance instead of a test container, set: ```shell ServiceControl_Persistence_PostgreSql_ConnectionString diff --git a/docs/testing-scenarios.md b/docs/testing-scenarios.md index 9cb783cc07..a344192632 100644 --- a/docs/testing-scenarios.md +++ b/docs/testing-scenarios.md @@ -1,20 +1,20 @@ # Testing scenarios -A long (though not exhaustive) list, although not every change will merit running every single test. +This list is long but not exhaustive. Not every change needs every test. ## Instance management installation - [ ] Upgrade an existing instance of ServiceControl to the version being released - [ ] Create a new instance of ServiceControl - [ ] Check with both localhost and a custom hostname -- [ ] Upgrade an existing instance of ServiceControl to the version being released using the Powershell Scripts -- [ ] Create a new instance of ServiceControl using the Powershell Scripts +- [ ] Upgrade an existing instance of ServiceControl to the version being released using the PowerShell scripts +- [ ] Create a new instance of ServiceControl using the PowerShell scripts ## Functionality - [ ] Install and run ServiceControl Primary, Audit, and Monitoring instances - [ ] [Install the ServiceControl.SmokeTesting tool](https://github.com/Particular/ServiceControl.SmokeTest#installing) -- [ ] Start the smoke testing tool e.g.: `dotnet servicecontrol.smoketest rabbitmq` +- [ ] Start the smoke testing tool, for example `dotnet servicecontrol.smoketest rabbitmq` - [ ] Test Custom checks - Run `check-fail 1` and verify in [ServicePulse->Custom Checks] that a warning appeared - Run `check-pass 1` and verify in [ServicePulse->Custom Checks] that the warning is gone @@ -22,7 +22,7 @@ A long (though not exhaustive) list, although not every change will merit runnin - With the tool running verify in [ServicePulse->Heartbeats->Healthy Endpoints] that `Endpoints0` to `Endpoints5` and `Sender` endpoints are reported - Run `stop 1` and verify in [ServicePulse->Heartbeats->Healthy Endpoints] that `Endpoint1` has moved to [Unhealthy Endpoints] - [ ] Recoverability - - [ ] Retry single message + - [ ] Retry single message - Run `throw 1` and `send 1 1` commands - Run `recover 1` and retry the message from ServicePulse - [ ] Retry message group @@ -36,31 +36,31 @@ A long (though not exhaustive) list, although not every change will merit runnin - Go to [Service Pulse->Configuration->Create redirect] and add `Endpoint1` to `Endpoint2` redirect - Retry the failed message - [ ] Audits - - [ ] Generate messages e.g. `send 1 10` and check if these are accessible in ServicePulse - - [ ] Generate message messages with non-trivial content using `send-fulltext 1 10`. Open ServicePulse and check if a message can be found in the search box using one of the strings from the `LongString` property + - [ ] Generate messages, for example `send 1 10` and check if these are accessible in ServicePulse + - [ ] Generate messages with non-trivial content using `send-fulltext 1 10`. Open ServicePulse and check if a message can be found in the search box using one of the strings from the `LongString` property - [ ] Generate messages using `fanout` and check in the [Sequence Diagram] view in ServicePulse that the graph is properly visualized - [ ] Saga Auditing - [ ] Generate messages using `saga-audits 1` and check in the [Saga] view in ServicePulse that the graph is properly visualized - [ ] Integration Events - - [ ] Download [integration events sample](https://docs.particular.net/samples/servicecontrol/events-subscription/). Switch the sample to the appropriate transport. - - [ ] Generate a failing message in the `NServiceBusEndptoin` and validate that an integration event `MessageFailed` is received by the `EndpointsMonitor` + - [ ] Download [integration events sample](https://docs.particular.net/samples/servicecontrol/events-subscription/). Switch the sample to the appropriate transport. + - [ ] Generate a failing message in the `NServiceBusEndptoin` and validate that an integration event `MessageFailed` is received by the `EndpointsMonitor` - [ ] Monitoring - [ ] Navigate to the [Monitoring] tab in ServicePulse and verify that all 6 endpoints are visible - [ ] Verify that the failed messages indicator is rendered for endpoints with failed messages - - [ ] Navigate to the details of `Endpoint0` and verify that all the graphs are properly rendered + - [ ] Navigate to the details of `Endpoint0` and verify that all the graphs are properly rendered ## Chaos testing -Try to break ServiceControl instances by gracefully (CTRL+C) and ungracefully (kill) processes to validate if both storage and logic behavior correctly. This type of testing is very difficult to automate. +Try to break ServiceControl instances by stopping processes gracefully (CTRL+C) and ungracefully (kill). Validate that both storage and logic behave correctly. This type of testing is difficult to automate. -- [ ] Ingestion, have the smoketest tool or the load generator generator create a large number of messages: +- [ ] Ingestion, have the smoke test tool or the load generator create a large number of messages: - [ ] Gracefully stop (CTRL+C) processes - - [ ] Ungracefully (kill) processes + - [ ] Ungracefully (kill) processes - [ ] Retry groups, create a large retry group and interrupt these: - [ ] Gracefully stop (CTRL+C) processes - [ ] Ungracefully (kill) processes -## Performance/Load testing +## Performance and load testing Test the new version against the previous version. @@ -70,8 +70,8 @@ Test the new version against the previous version. - [ ] Test stability by: - [ ] Rebooting the machine and verifying that ServiceControl instances start in a reasonable amount of time and behave correctly - [ ] Stopping ServiceControl Windows services, verifying they stop as expected, and subsequently start in a reasonable amount of time and behave correctly - - [ ] Killing the hosting virtual machine from the Azure portal and verifying instances behave correctly after the reboot - - [ ] Trying to upgrade instances to newer versions while ingestion runs at full speed under load and verify the upgrade is successful + - [ ] Killing the hosting virtual machine from the Azure portal and verifying that instances behave correctly after the reboot + - [ ] Upgrading instances to newer versions while ingestion runs at full speed under load and verifying the upgrade is successful Review CPU/RAM utilization and disk IO. diff --git a/docs/testing.md b/docs/testing.md index 1048d032d8..a3140e48dd 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -1,6 +1,6 @@ # Testing -ServiceControl tests are designed to test different components and behaviors. This document outlines the tests in the repository and what they are meant to test. +ServiceControl tests cover different components and behaviors. ## Unit tests @@ -15,7 +15,7 @@ Packaging tests check: ## Installation engine tests -Installation engine tests run partial installations and checks: +Installation engine tests run partial installations and check: - That the generated configuration is correct. - That transport and persistence are correctly extracted. @@ -23,15 +23,15 @@ Installation engine tests run partial installations and checks: ## Persistence tests Persistence tests check assumptions at the persistence seam level by exercising each persister. -For local setup details, see [Local Testing of Persistence Providers](testing-persistence.md). +For local setup details, see [Local testing of persistence providers](testing-persistence.md). ## Transport tests -Transport tests are done by executing the transport test suite for each transport. +Transport tests run the transport test suite for each transport. ## Acceptance tests -Run ServiceControl full version and use the HTTP API to validate results. LearningTransport is used for all tests. +Acceptance tests run the full ServiceControl and use the HTTP API to validate results. LearningTransport is used for all tests. For how to write one that fails when it should, see [Writing acceptance tests](writing-acceptance-tests.md). @@ -61,7 +61,7 @@ Multi-instance tests validate the interaction between different ServiceControl i ## Container tests -Container images generated for all builds are pushed to the [GitHub container registry](https://docs.github.com/en/packages/working-with-a-github-packages-registry/working-with-the-container-registry). Once pushed, all images are tested by [spinning them all up for each supported transport](/src/container-integration-test/). +Container images generated for all builds are pushed to the [GitHub container registry](https://docs.github.com/en/packages/working-with-a-github-packages-registry/working-with-the-container-registry). Once pushed, all images are tested by [starting them all for each supported transport](/src/container-integration-test/). Containers built by a PR and stored on GitHub Container Registry can be tested locally: @@ -72,36 +72,36 @@ Containers built by a PR and stored on GitHub Container Registry can be tested l ```shell docker login ghcr.io ``` - you will be prompted for a username (your particular.net email) and a password (the token) - - ensure that you get a successful login message. - - Use `docker logout ghcr.io` once the following steps are complete and consider removing the token from github if its no longer needed -2. In the terminal, navigate to [`/docs/test-ghcr-tag`](/docs/test/ghcr-tag). + Docker prompts for a username (your particular.net email) and a password (the token). + - Confirm that the login succeeds. + - Use `docker logout ghcr.io` once the following steps are complete. Consider removing the token from GitHub if you no longer need it. +2. In the terminal, go to [`/docs/test-ghcr-tag`](/docs/test/ghcr-tag). 3. Edit the [`.env` file](/docs/test-ghcr-tag/.env) to specify the PR-based tag (in the form `pr-####`) to test. 4. Run `docker compose up -d`. -5. Services will be available at the following URLs: +5. Open the services at the following URLs: * [RabbitMQ Management](http://localhost:15672) (Login: `guest`/`guest`) * [RavenDB](http://localhost:8080) * [ServiceControl API](http://localhost:33333/api) * [Audit API](http://localhost:44444/api) * [Monitoring API](http://localhost:33633) * [ServicePulse (latest from Docker Hub)](http://localhost:9090) -6. Tear down services using `docker compose down`. +6. Stop the services using `docker compose down`. ## Container tests using Aspire -The [Particular.Aspire.Hosting.ServicePlatform](https://github.com/Particular/Particular.Aspire.Hosting.ServicePlatform) package integrates the Particular Platform with the Aspire hosting platform. This package configures environment variables to attach the platform. There is a single file apphost in [`test-ghcr-tag-aspire`](/docs/test-ghcr-tag-aspire) to start up serviceconrol from a prerelease container image. +The [Particular.Aspire.Hosting.ServicePlatform](https://github.com/Particular/Particular.Aspire.Hosting.ServicePlatform) package integrates the Particular Platform with the Aspire hosting platform. This package configures environment variables to attach the platform. A single-file apphost in [`test-ghcr-tag-aspire`](/docs/test-ghcr-tag-aspire) starts ServiceControl from a prerelease container image. Containers built by a PR and stored on GitHub Container Registry can be tested locally: -1. Set up your github container registry credentials as described in the [Container tests](#container-tests) section above. -2. Make sure you have the [Aspire CLI installed](https://aspire.dev/get-started/install-cli/). -3. Run `aspire update` to ensure that the testing AppHost file `docs/test-ghcr-tag-aspire/AppHost.cs` is running the latest aspire SDK and RabbitMQ integration package. -4. Run `aspire run docs/test-ghcr-tag-aspire/AppHost.cs -- tag` to start the application, where `tag` is the PR-based tag (in the form `pr-####`) to test. If no tag is provided, it will default to the `latest` tag. -5. Once running you can open the dashboard from the link in the terminal, this dashboard will provide the assigned ports for each service. +1. Set up your GitHub Container Registry credentials as described in the [Container tests](#container-tests) section above. +2. Install the [Aspire CLI](https://aspire.dev/get-started/install-cli/). +3. Run `aspire update` so that the testing AppHost file `docs/test-ghcr-tag-aspire/AppHost.cs` uses the latest Aspire SDK and RabbitMQ integration package. +4. Run `aspire run docs/test-ghcr-tag-aspire/AppHost.cs -- tag` to start the application, where `tag` is the PR-based tag (in the form `pr-####`) to test. Without a tag, the command uses the `latest` tag. +5. Open the dashboard from the link in the terminal. The dashboard shows the assigned ports for each service: * RabbitMQ Management (Login: `guest`/`guest`) * RavenDB * ServiceControl API * Audit API * Monitoring API * ServicePulse (latest from Docker Hub) -6. Aspire will automatically tear down the application when you exit the CLI process. \ No newline at end of file +6. Exit the CLI process. Aspire stops the application automatically. \ No newline at end of file diff --git a/docs/throughput-collection.md b/docs/throughput-collection.md index 246102de74..a463725d3b 100644 --- a/docs/throughput-collection.md +++ b/docs/throughput-collection.md @@ -1,32 +1,27 @@ -From version 5.4.0 a new `Licensing Component` feature was introduced to [collect usage data](https://docs.particular.net/servicepulse/usage) from within the Particular Platform. +Version 5.4.0 introduced the `Licensing Component` feature, which [collects usage data](https://docs.particular.net/servicepulse/usage) from within the Particular Platform. -Usage data is collected from 3 different sources: -- Audit instance(s) -- Monitoring instance -- Directly from the broker +The Error instance orchestrates the collection of usage data from three sources: -The Error instance is the orchestrator for collecting usage data from all the different sources. - -The Audit instance(s) is/are queried once a day to obtain usage data. The initial query will grab all available historic data. -The broker is queried once a day to obtain usage data. Depending on the broker, the initial query will grab the last 30 days of data. -The Monitoring instance uses its metrics calculations to send usage data to the Error instance every 5 minutes to a pre-defined satellite queue with the default name of `servicecontrol.throughput`. +- Audit instances are queried once a day. The initial query retrieves all available historic data. +- The broker is queried once a day. Depending on the broker, the initial query retrieves the last 30 days of data. +- The Monitoring instance uses its metrics calculations to send usage data to the Error instance every 5 minutes. It sends the data to a predefined satellite queue with the default name `servicecontrol.throughput`. ### Why is the "servicecontrol.throughput" queue not a sub queue of the Error instance? The usage collection queue needs to be known to the Monitoring instance. -At the time of creating this feature it was decided that having a queue name that is not dependent on the name of the Error instance means less setup for majority of customers since the feature would "just work" out of the box. +At the time of creating this feature it was decided that having a queue name that is not dependent on the name of the Error instance means less setup for most customers, because the feature would "just work". -If the queue name was based on the Error instance name (i.e. "ErrorInstanceQueueName.throughput") then **every** customer would have to make updates to their Monitoring instance config to set the correct queue name. +If the queue name was based on the Error instance name (for example "ErrorInstanceQueueName.throughput") then **every** customer would have to make updates to their Monitoring instance config to set the correct queue name. -The decision favoured simplicity of upgrade over existing ServiceControl queue name conventions, keeping in line with the tech lead preferences for [software that "just works"](https://github.com/Particular/Strategy/blob/master/tech-lead-preferences/it-just-works.md#it-just-works) and [convenience](https://github.com/Particular/Strategy/blob/master/tech-lead-preferences/usability.md#convenience). +The decision favored simplicity of upgrade over existing ServiceControl queue name conventions, consistent with the tech lead preferences for [software that "just works"](https://github.com/Particular/Strategy/blob/master/tech-lead-preferences/it-just-works.md#it-just-works) and [convenience](https://github.com/Particular/Strategy/blob/master/tech-lead-preferences/usability.md#convenience). -#### Why isn't SCMU used to ensure the names match? +#### Why isn't the ServiceControl Management Utility (SCMU) used to ensure the names match? -SCMU is a Windows only tool, plus the install of the Monitoring instance is separate to that of the Error instance. -Additionally, the Monitoring instance can be installed on a different server to the Error instance. +SCMU is a Windows-only tool, and the installation of the Monitoring instance is separate from that of the Error instance. +The Monitoring instance can also be installed on a different server than the Error instance. For similar reasons we do not configure the remote audit instances in SCMU. ### Can the "servicecontrol.throughput" queue be renamed? diff --git a/docs/writing-acceptance-tests.md b/docs/writing-acceptance-tests.md index f6e7f9e81a..b8d6404d4d 100644 --- a/docs/writing-acceptance-tests.md +++ b/docs/writing-acceptance-tests.md @@ -4,9 +4,9 @@ An acceptance test starts a full ServiceControl instance and drives it over the HTTP API, exactly as ServicePulse does. See [Testing](testing.md) for the suites and how to run them. -This is what makes them different from the other suites. Component logic belongs in unit tests, and storage belongs in persistence tests. Both of those are faster and easier to debug. An acceptance test is worth its cost when it proves something a user can see: that a journey through ServicePulse still works, against a real instance and a real persister. +Component logic belongs in unit tests, and storage belongs in persistence tests. Both are faster and easier to debug than acceptance tests. An acceptance test is worth its cost when it proves something a user can see: that a journey through ServicePulse still works, against a real instance and a real persister. -So the question to start from is not "which endpoint am I testing" but "what is someone trying to do". +So the question to start from is not "which endpoint am I testing" but "what is someone trying to do?" ### The transport is always LearningTransport @@ -33,7 +33,7 @@ await Define() .Run(); ``` -Each step logs `Advancing from X to Y`. If the scenario stops making progress, the log names the step it stopped on, instead of the test failing with a timeout that tells you nothing. `When_creating_a_usage_report_on_a_non_broker_transport` is the worked example, and its name is doing the job described above: it says which branch it covers, so the broker one can sit beside it without either being mistaken for the other. +Each step logs `Advancing from X to Y`. If the scenario stops making progress, the log names the step it stopped on, instead of the test failing with a timeout that tells you nothing. `When_creating_a_usage_report_on_a_non_broker_transport` is the worked example, and its name says which branch it covers, so the broker one can sit beside it without either being mistaken for the other. Not everything is a journey. Edge cases, such as posting a retry for an id that does not exist, are separate focused tests next to the scenario, not extra steps inside it. @@ -49,13 +49,13 @@ The report scenario asserts that a name the user redacted does not appear in the ## Expect the domain to have rules -When a scenario hangs, it is often the system telling you a rule you did not know about. The report scenario first recorded throughput for today, and then waited until the 90 second timeout. The reason is that a usage report counts only complete days, and ignores a partial one on purpose. +When a scenario hangs, it is often the system telling you a rule you did not know about. The report scenario first recorded throughput for today, and then waited until the 90-second timeout. The reason is that a usage report counts only complete days and ignores a partial one on purpose. So when a step will not go green, read the code it is waiting on before you add longer timeouts or retries. Once you find the rule, write it in the test as a comment, because the next person will assume what you assumed. ## Caveats -One idea connects all of these: **a test must fail if the thing it sets up does not take effect.** Below are the ways that has gone wrong in this suite. Two of them went unnoticed for years. +One idea connects all of these: **a test must fail if the thing it sets up does not take effect.** These are the ways it has gone wrong in this suite. Two of them went unnoticed for years. ### The double that is registered but never used @@ -71,8 +71,8 @@ builder.Services.AddSingleton(); First check how the collaborator asks for its dependency, because the two shapes behave differently when a test adds to them: -- **One instance**, such as `ReturnToSenderDequeuer` asking for a `ReturnToSender`. `CustomizeHostBuilder` runs after all production registration, so registering the same service type again replaces it with the double. -- **A collection**, such as `IEnumerable` or `GetServices()`. A later registration is *added to* the collection. The production implementation is still there, and still runs. +- One instance, such as `ReturnToSenderDequeuer` asking for a `ReturnToSender`. `CustomizeHostBuilder` runs after all production registration, so registering the same service type again replaces it with the double. +- A collection, such as `IEnumerable` or `GetServices()`. A later registration is *added to* the collection. The production implementation is still there, and still runs. ### The registration that only says it replaces another @@ -107,7 +107,7 @@ Registering the double correctly is only half the job. If the assertion would al ### The assertion that only one persister can satisfy -The suite runs against every persister. An assertion about how RavenDB happens to store or trim something says nothing about ServiceControl's behaviour. It also ends up on another persister's exclusion list, where it looks like a missing feature instead of a test that asks for too much. Assert what every persister has to do to be correct. +The suite runs against every persister. An assertion about how RavenDB happens to store or trim something says nothing about ServiceControl behavior. It also ends up on another persister's exclusion list, where it looks like a missing feature instead of a test that asks for too much. Assert what every persister has to do to be correct. ### The wait that does not cover what the assertion reads @@ -131,7 +131,7 @@ Persisters do not make a write visible everywhere at the same moment, and search This cuts across [the assertion that could not have failed](#the-assertion-that-could-not-have-failed), so be deliberate about where the wait stops and the assertion starts. -Wait for the loosest condition that makes the query answerable, not for the answer you expect. `Count == 2` never advances when a search matches three, so the interesting regression, matching too much, is reported as a 90 second timeout rather than as the assertion that would have named the extra row. `Count >= 2` advances as soon as there is enough to judge and lets the assertion do the judging, which turns that same regression into a failure in a few seconds reading `Extra (1): DeliveryFailed`. +Wait for the loosest condition that makes the query answerable, not for the answer you expect. `Count == 2` never advances when a search matches three, so the interesting regression, matching too much, is reported as a 90-second timeout rather than as the assertion that would have named the extra row. `Count >= 2` advances as soon as there is enough to judge and lets the assertion do the judging, which turns that same regression into a failure in a few seconds reading `Extra (1): DeliveryFailed`. Whatever the wait cannot avoid holding, record on the scenario context, because the runner prints the context when a scenario does not finish while a `TimeoutException` on its own says only that 90 seconds passed: @@ -147,8 +147,6 @@ Headers put into a dictionary nothing reads, constants nothing compares against, ## Before you open the PR -**Make it fail on purpose.** Break the thing the test protects, run it, and read the message. If it still passes, or if it fails with something another person cannot act on, the test is not finished yet. This one run checks every caveat listed above. - -**Run it on a second persister.** A test that passes on Raven and is never run on SqlServer or PostgreSQL will be excluded later by someone who knows less about it than you do now. - -**Cover a new route in the PR that adds it.** Nothing in the suite notices an uncovered route. +- Make it fail on purpose. Break the thing the test protects, run it, and read the message. If it still passes, or if it fails with something another person cannot act on, the test is not finished yet. This one run checks every caveat listed above. +- Run it on a second persister. A test that passes on RavenDB and is never run on SqlServer or PostgreSQL will be excluded later by someone who knows less about it than you do now. +- Cover a new route in the PR that adds it. Nothing in the suite notices an uncovered route.