From 68a22bcf5eb384d26c810659a62b9683aa414c1d Mon Sep 17 00:00:00 2001 From: Mark Lauter <10211200+marklauter@users.noreply.github.com> Date: Sun, 14 Jun 2026 01:23:06 -0400 Subject: [PATCH 1/4] Add Plumber.Diagnostics (OpenTelemetry), harden publish CI, lift coverage to > 95% (#8) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add Plumber.Diagnostics (OpenTelemetry) and rename Serilog extension Migrate the stale plumber.opentelemetry.extensions repo into the monorepo as Plumber.Diagnostics, modernized to .NET 10 / C# 14 and the repo conventions: - Library instruments via BCL ActivitySource/Meter with no OpenTelemetry SDK dependency; ctor-injected IOptions; TimeProvider-backed timing via RequestContext.Elapsed; BCL Activity.AddException for exception recording. - API: UseRequestTracing / UseRequestMetrics / UseRequestDiagnostics, AddPlumberDiagnostics, with PlumberDiagnostics exposing the source/meter names. - 30 tests via BCL ActivityListener/MeterListener (100% line, 91% branch, 100% method); arch-test class; serial execution for global diagnostic state. - Folded into Sample.Cli with host-free SDK console exporters. - Wired into Plumber.slnx, the publish workflow, and README. Rename Plumber.Serilog.Extensions -> Plumber.Serilog (assembly/namespace), keeping the published PackageId MSL.Plumber.Serilog.Extensions. Drop the redundant Microsoft.Extensions.Logging SimpleConsole from Sample.Cli; keep AddLogging so ILogger still resolves, with Serilog as the sole sink. Co-Authored-By: Claude Opus 4.8 (1M context) * Point README Diagnostics link at the renamed wiki page Co-Authored-By: Claude Opus 4.8 (1M context) * ci: upload coverage for all Plumber.* test projects The coverage artifact upload only listed Plumber.Tests and Sample.Cli.Tests. Add the missing Plumber.Testing.Tests, Plumber.Diagnostics.Tests, and Plumber.Serilog.Tests cobertura reports so coverage is captured for every Plumber.* library. Co-Authored-By: Claude Opus 4.8 (1M context) * Treat cancellation as a non-error in the diagnostics middleware (#3, #4) OperationCanceledException — how Plumber surfaces both caller cancellation and a pipeline timeout to the middleware — is an expected outcome, not a defect: - tracing: leave the span Unset, record no exception event; - metrics: keep it out of plumber.requests.errors and the duration histogram, and skip the RecordCustomMetrics hook (resolved outcomes only); - it still counts in plumber.requests.count as an attempt. ThrowOnException still governs propagation. RecordCustomMetrics moves off the blanket finally into the success and genuine-error paths so the hook never sees a cancellation as success=false. Coverage to 100% line/branch/method (added enrichment-on-cancel/error and null-response tests); diagnostics threshold raised 91->100. Co-Authored-By: Claude Opus 4.8 (1M context) * Untrack draft review doc committed by mistake docs/publish-workflow-remediation.md is an in-progress review artifact (like docs/otel-migration-findings.md) and was swept into the previous commit by git add -A. Remove it from tracking; the working file stays on disk. Co-Authored-By: Claude Opus 4.8 (1M context) * ci: harden publish workflow (gate on Release, metadata-driven pack) Implements items 1-5 of the publish-workflow remediation plan: - Gate job now builds and tests `-c Release`, matching the shipped configuration instead of validating Debug. - Replace the four hand-maintained pack steps with a single `dotnet pack Plumber.slnx`; mark Sample.Cli IsPackable=false so the solution pack emits only the four MSL.Plumber.* packages. - Validate the release tag resolves to a semver before pack/push. - Add a concurrency guard keyed on the release tag (no cancel-in-progress). - Pin the SDK via global.json (10.0.100, rollForward latestFeature). Trusted Publishing (OIDC) and job-setup de-duplication are deferred. Co-Authored-By: Claude Opus 4.8 (1M context) * ci: extract shared setup-dotnet composite action Implements item 6 of the remediation plan. Replace the duplicated setup-dotnet + NuGet cache steps across both workflows (four jobs) with a local composite action referenced as ./.github/actions/setup-dotnet. checkout stays per-job since a local composite can only resolve after the repo is checked out. Co-Authored-By: Claude Opus 4.8 (1M context) * ci: trigger tests on composite action and global.json changes The tests workflow now depends on the setup-dotnet composite action and the SDK pin in global.json, but the path filters gated only on the workflow file itself. Add both so changes to either re-run the suite. Co-Authored-By: Claude Opus 4.8 (1M context) * ci: fix readme casing, add CI determinism flag and publish dry-run Addresses three follow-up findings: - Pack the readme as README.md (was readme.md) in all four packable projects. The publish job packs on case-sensitive ubuntu, where the lowercase path would fail with NU5039 at release time; it only passed locally on case-insensitive Windows. - Set ContinuousIntegrationBuild=true in CI (gated on GITHUB_ACTIONS) for reproducible builds and normalized SourceLink paths in packed binaries. - Add a workflow_dispatch trigger to the publish workflow with a version input; guard the nuget push with `if: github.event_name == 'release'` so manual runs build/test/pack/upload without publishing. The paths-filter vs. required-status-check question is documented as open in the remediation plan; it depends on branch-protection settings. Co-Authored-By: Claude Opus 4.8 (1M context) * Sample.Cli: in-memory OTel exporter with a summary, not a console dump (#10, #11) #10: swap the OpenTelemetry console exporter for the in-memory exporter and print a compact Telemetry.Summarize() line after the run, so the demo console shows the report plus a one-line confirmation that span/metric collection worked, instead of interleaved exporter dumps. #11: comment the inline timing delegate to distinguish its role (it feeds the printed report payload) from the OTel span duration and the Serilog elapsed. Tests: TelemetrySummaryReflectsAPipelineRun drives the pipeline through the in-memory exporters; the assembly runs serially to avoid cross-test telemetry leakage. Sample.Cli branch coverage 38->47.5%, floor raised 38->45. Co-Authored-By: Claude Opus 4.8 (1M context) * docs: correct remediation plan status (items 1-9 implemented) The status line still read "items 1-6"; items 7-9 are also done and item 10 is open pending a branch-protection decision. Co-Authored-By: Claude Opus 4.8 (1M context) * better tests * nuget: sharpen package titles, descriptions, and tags Improve discoverability and clarity across the four publishable packages without touching PackageIds (upgrade continuity preserved): - Plumber: lead the title with the ASP.NET Core analogy and concrete surfaces (console/worker/serverless); add the Microsoft.Extensions familiarity hook; broaden tags (serverless, aws-lambda, azure-functions, background-service, di, cancellation). - Plumber.Testing: promote the WebApplicationFactory analogy into the title. - Plumber.Diagnostics: front-load the no-OpenTelemetry-SDK-dependency differentiator and name export backends; add distributed-tracing, activitysource, meter, instrumentation, monitoring tags. - Plumber.Serilog.Extensions: surface trace correlation (TraceId/SpanId) and add diagnostic-context, observability, trace-correlation tags. Co-Authored-By: Claude Opus 4.8 (1M context) * revert: keep Plumber.Serilog.Extensions namespace and assembly Undo the rename of the Serilog package from Plumber.Serilog.Extensions to Plumber.Serilog. The PackageId was always MSL.Plumber.Serilog.Extensions, so the rename changed only the assembly name and namespace — a source- and binary-breaking change for every existing consumer (their `using Plumber.Serilog.Extensions;` would break) for no functional benefit. Restores the project and test directory names, assembly, and namespace to Plumber.Serilog.Extensions, and updates the solution, sample, test project, and CI coverage path to match. The release is now a non-breaking minor bump (5.1.0): the only new public surface is the additive Plumber.Diagnostics package. Co-Authored-By: Claude Opus 4.8 (1M context) * ci: pass nuget key via env, pin SDK install via global.json - dotnet.publish.yml: pass NUGET_API_KEY through `env:` and reference it as "$NUGET_API_KEY" instead of interpolating the secret into the run command. - setup-dotnet action: install the SDK via global-json-file so global.json is the single source of truth (was dotnet-version: 10.0.x, which the action does not reconcile against global.json). Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .github/actions/setup-dotnet/action.yml | 18 ++ .github/workflows/dotnet.publish.yml | 61 ++-- .github/workflows/dotnet.tests.yml | 33 +-- Directory.Build.props | 7 +- Directory.Packages.props | 5 + Plumber.slnx | 2 + README.md | 2 +- docs/publish-workflow-remediation.md | 231 +++++++++++++++ global.json | 6 + samples/Sample.Cli.Tests/AssemblyInfo.cs | 4 + samples/Sample.Cli.Tests/PipelineTests.cs | 25 +- .../Sample.Cli.Tests/Sample.Cli.Tests.csproj | 3 +- samples/Sample.Cli/Pipeline.cs | 15 +- samples/Sample.Cli/Program.cs | 12 + samples/Sample.Cli/Sample.Cli.csproj | 6 +- samples/Sample.Cli/Telemetry.cs | 68 +++++ .../Plumber.Diagnostics.csproj | 56 ++++ src/Plumber.Diagnostics/PlumberDiagnostics.cs | 15 + .../RequestHandlerDiagnosticsExtensions.cs | 141 +++++++++ ...tMetricsMiddleware{TRequest, TResponse}.cs | 105 +++++++ .../RequestMetricsOptions.cs | 31 ++ ...tTracingMiddleware{TRequest, TResponse}.cs | 108 +++++++ .../RequestTracingOptions.cs | 47 +++ .../ServiceCollectionDiagnosticsExtensions.cs | 51 ++++ .../Plumber.Serilog.Extensions.csproj | 8 +- src/Plumber.Testing/Plumber.Testing.csproj | 8 +- src/Plumber/Plumber.csproj | 8 +- ...uestHandlerBuilder{TRequest, TResponse}.cs | 5 +- .../RequestHandler{TRequest, TResponse}.cs | 14 +- .../Architecture/ArchitectureTests.cs | 12 + .../Plumber.Diagnostics.Tests/AssemblyInfo.cs | 4 + .../Plumber.Diagnostics.Tests.csproj | 15 + .../RequestDiagnosticsAndGuardTests.cs | 84 ++++++ .../RequestMetricsTests.cs | 222 ++++++++++++++ .../RequestTracingTests.cs | 279 ++++++++++++++++++ .../TelemetryCollectors.cs | 81 +++++ .../Plumber.Diagnostics.Tests/TestPipeline.cs | 41 +++ .../Plumber.Serilog.Extensions.Tests.csproj | 4 - .../RequestHandlerSerilogExtensionsTests.cs | 97 ++++++ tests/Plumber.Tests/PlumberTests.cs | 53 ++++ 40 files changed, 1890 insertions(+), 97 deletions(-) create mode 100644 .github/actions/setup-dotnet/action.yml create mode 100644 docs/publish-workflow-remediation.md create mode 100644 global.json create mode 100644 samples/Sample.Cli.Tests/AssemblyInfo.cs create mode 100644 samples/Sample.Cli/Telemetry.cs create mode 100644 src/Plumber.Diagnostics/Plumber.Diagnostics.csproj create mode 100644 src/Plumber.Diagnostics/PlumberDiagnostics.cs create mode 100644 src/Plumber.Diagnostics/RequestHandlerDiagnosticsExtensions.cs create mode 100644 src/Plumber.Diagnostics/RequestMetricsMiddleware{TRequest, TResponse}.cs create mode 100644 src/Plumber.Diagnostics/RequestMetricsOptions.cs create mode 100644 src/Plumber.Diagnostics/RequestTracingMiddleware{TRequest, TResponse}.cs create mode 100644 src/Plumber.Diagnostics/RequestTracingOptions.cs create mode 100644 src/Plumber.Diagnostics/ServiceCollectionDiagnosticsExtensions.cs create mode 100644 tests/Plumber.Diagnostics.Tests/Architecture/ArchitectureTests.cs create mode 100644 tests/Plumber.Diagnostics.Tests/AssemblyInfo.cs create mode 100644 tests/Plumber.Diagnostics.Tests/Plumber.Diagnostics.Tests.csproj create mode 100644 tests/Plumber.Diagnostics.Tests/RequestDiagnosticsAndGuardTests.cs create mode 100644 tests/Plumber.Diagnostics.Tests/RequestMetricsTests.cs create mode 100644 tests/Plumber.Diagnostics.Tests/RequestTracingTests.cs create mode 100644 tests/Plumber.Diagnostics.Tests/TelemetryCollectors.cs create mode 100644 tests/Plumber.Diagnostics.Tests/TestPipeline.cs diff --git a/.github/actions/setup-dotnet/action.yml b/.github/actions/setup-dotnet/action.yml new file mode 100644 index 0000000..567eae8 --- /dev/null +++ b/.github/actions/setup-dotnet/action.yml @@ -0,0 +1,18 @@ +name: Setup .NET +description: Install the .NET SDK (pinned via global.json) and restore the NuGet package cache. + +runs: + using: composite + steps: + - name: Setup .NET + uses: actions/setup-dotnet@v5 + with: + global-json-file: global.json + + - name: Cache NuGet packages + uses: actions/cache@v5 + with: + path: ~/.nuget/packages + key: ${{ runner.os }}-nuget-${{ hashFiles('**/*.csproj', 'Directory.Packages.props') }} + restore-keys: | + ${{ runner.os }}-nuget- diff --git a/.github/workflows/dotnet.publish.yml b/.github/workflows/dotnet.publish.yml index 19cdddd..e3960a1 100644 --- a/.github/workflows/dotnet.publish.yml +++ b/.github/workflows/dotnet.publish.yml @@ -3,6 +3,16 @@ name: .NET Publish on: release: types: [published] + workflow_dispatch: + inputs: + version: + description: "Version to pack for a dry run (build/test/pack/upload only — no nuget push)" + required: false + default: "0.0.0-dryrun" + +concurrency: + group: publish-${{ github.event.release.tag_name || github.run_id }} + cancel-in-progress: false permissions: contents: read @@ -12,27 +22,16 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 - - name: Setup .NET - uses: actions/setup-dotnet@v5 - with: - dotnet-version: 10.0.x - - - name: Cache NuGet packages - uses: actions/cache@v5 - with: - path: ~/.nuget/packages - key: ${{ runner.os }}-nuget-${{ hashFiles('**/*.csproj', 'Directory.Packages.props') }} - restore-keys: | - ${{ runner.os }}-nuget- + - uses: ./.github/actions/setup-dotnet - name: Restore dependencies run: dotnet restore - name: Build - run: dotnet build -c Debug --no-restore + run: dotnet build -c Release --no-restore - name: Test - run: dotnet test --no-build + run: dotnet test -c Release --no-build publish: needs: test @@ -41,38 +40,25 @@ jobs: contents: read steps: - uses: actions/checkout@v6 - - name: Setup .NET - uses: actions/setup-dotnet@v5 - with: - dotnet-version: 10.0.x - - - name: Cache NuGet packages - uses: actions/cache@v5 - with: - path: ~/.nuget/packages - key: ${{ runner.os }}-nuget-${{ hashFiles('**/*.csproj', 'Directory.Packages.props') }} - restore-keys: | - ${{ runner.os }}-nuget- + - uses: ./.github/actions/setup-dotnet - name: Resolve package version env: + EVENT: ${{ github.event_name }} TAG: ${{ github.event.release.tag_name }} + INPUT_VERSION: ${{ inputs.version }} run: | - version="${TAG#v}" + if [ "$EVENT" = "release" ]; then version="${TAG#v}"; else version="$INPUT_VERSION"; fi + [[ "$version" =~ ^[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?$ ]] \ + || { echo "::error::invalid version '$version' (event '$EVENT')"; exit 1; } echo "Publishing version $version" echo "PACKAGE_VERSION=$version" >> "$GITHUB_ENV" - name: Restore run: dotnet restore - - name: Pack Plumber - run: dotnet pack src/Plumber -c Release --no-restore -o nuget -p:PackageVersion=$PACKAGE_VERSION -p:Version=$PACKAGE_VERSION - - - name: Pack Plumber.Testing - run: dotnet pack src/Plumber.Testing -c Release --no-restore -o nuget -p:PackageVersion=$PACKAGE_VERSION -p:Version=$PACKAGE_VERSION - - - name: Pack Plumber.Serilog.Extensions - run: dotnet pack src/Plumber.Serilog.Extensions -c Release --no-restore -o nuget -p:PackageVersion=$PACKAGE_VERSION -p:Version=$PACKAGE_VERSION + - name: Pack + run: dotnet pack Plumber.slnx -c Release --no-restore -o nuget -p:PackageVersion=$PACKAGE_VERSION -p:Version=$PACKAGE_VERSION - name: Upload package artifacts uses: actions/upload-artifact@v7 @@ -82,4 +68,7 @@ jobs: retention-days: 30 - name: Publish to api.nuget.org - run: dotnet nuget push nuget/*.nupkg --skip-duplicate -k ${{ secrets.NUGET_API_KEY }} -s https://api.nuget.org/v3/index.json + if: github.event_name == 'release' + env: + NUGET_API_KEY: ${{ secrets.NUGET_API_KEY }} + run: dotnet nuget push nuget/*.nupkg --skip-duplicate -k "$NUGET_API_KEY" -s https://api.nuget.org/v3/index.json diff --git a/.github/workflows/dotnet.tests.yml b/.github/workflows/dotnet.tests.yml index d36ee5c..f556c02 100644 --- a/.github/workflows/dotnet.tests.yml +++ b/.github/workflows/dotnet.tests.yml @@ -3,10 +3,10 @@ name: .NET Tests on: push: branches: [ "main" ] - paths: [ "src/**", "tests/**", "samples/**", "Directory.Build.props", "Directory.Build.targets", "Directory.Packages.props", ".editorconfig", ".gitattributes", ".github/workflows/dotnet.tests.yml" ] + paths: [ "src/**", "tests/**", "samples/**", "Directory.Build.props", "Directory.Build.targets", "Directory.Packages.props", ".editorconfig", ".gitattributes", "global.json", ".github/workflows/dotnet.tests.yml", ".github/actions/setup-dotnet/action.yml" ] pull_request: branches: [ "main" ] - paths: [ "src/**", "tests/**", "samples/**", "Directory.Build.props", "Directory.Build.targets", "Directory.Packages.props", ".editorconfig", ".gitattributes", ".github/workflows/dotnet.tests.yml" ] + paths: [ "src/**", "tests/**", "samples/**", "Directory.Build.props", "Directory.Build.targets", "Directory.Packages.props", ".editorconfig", ".gitattributes", "global.json", ".github/workflows/dotnet.tests.yml", ".github/actions/setup-dotnet/action.yml" ] concurrency: group: ${{ github.workflow }}-${{ github.ref }} @@ -20,18 +20,7 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 - - name: Setup .NET - uses: actions/setup-dotnet@v5 - with: - dotnet-version: 10.0.x - - - name: Cache NuGet packages - uses: actions/cache@v5 - with: - path: ~/.nuget/packages - key: ${{ runner.os }}-nuget-${{ hashFiles('**/*.csproj', 'Directory.Packages.props') }} - restore-keys: | - ${{ runner.os }}-nuget- + - uses: ./.github/actions/setup-dotnet - name: Verify formatting run: dotnet format "Plumber.slnx" --severity info --verify-no-changes @@ -43,18 +32,7 @@ jobs: configuration: [ Debug, Release ] steps: - uses: actions/checkout@v6 - - name: Setup .NET - uses: actions/setup-dotnet@v5 - with: - dotnet-version: 10.0.x - - - name: Cache NuGet packages - uses: actions/cache@v5 - with: - path: ~/.nuget/packages - key: ${{ runner.os }}-nuget-${{ hashFiles('**/*.csproj', 'Directory.Packages.props') }} - restore-keys: | - ${{ runner.os }}-nuget- + - uses: ./.github/actions/setup-dotnet - name: Restore dependencies run: dotnet restore @@ -72,5 +50,8 @@ jobs: name: coverage-cobertura path: | tests/Plumber.Tests/coverage.cobertura.xml + tests/Plumber.Testing.Tests/coverage.cobertura.xml + tests/Plumber.Diagnostics.Tests/coverage.cobertura.xml + tests/Plumber.Serilog.Extensions.Tests/coverage.cobertura.xml samples/Sample.Cli.Tests/coverage.cobertura.xml if-no-files-found: error diff --git a/Directory.Build.props b/Directory.Build.props index b9b80d1..d681dc2 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -14,6 +14,11 @@ true + + + true + + @@ -26,7 +31,7 @@ true cobertura - 95,91,95 + 95,95,95 line,branch,method minimum diff --git a/Directory.Packages.props b/Directory.Packages.props index c75ffa9..f588f50 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -30,6 +30,11 @@ + + + + diff --git a/Plumber.slnx b/Plumber.slnx index 48ca23d..cc43bf7 100644 --- a/Plumber.slnx +++ b/Plumber.slnx @@ -17,10 +17,12 @@ + + diff --git a/README.md b/README.md index 68b7299..edf8bbd 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ The [wiki](https://github.com/marklauter/plumber/wiki) is the full documentation - **`MSL.Plumber.Pipeline`** — the core builder, handler, middleware, and request context. See [Pipeline](https://github.com/marklauter/plumber/wiki/Building-A-Pipeline). - **`MSL.Plumber.Pipeline.Testing`** — `PlumberApplicationFactory` for exercising a real pipeline in tests. See [Testing](https://github.com/marklauter/plumber/wiki/Testing). - **`MSL.Plumber.Serilog.Extensions`** — per-request Serilog request logging. See [Serilog Extensions](https://github.com/marklauter/plumber/wiki/Serilog-Extensions). -- **`MSL.Plumber.OpenTelemetry.Extensions`** — tracing and metrics middleware. *Planned.* +- **`MSL.Plumber.Diagnostics`** — per-request OpenTelemetry tracing and metrics middleware. See [Diagnostics](https://github.com/marklauter/plumber/wiki/Diagnostics). ## Install diff --git a/docs/publish-workflow-remediation.md b/docs/publish-workflow-remediation.md new file mode 100644 index 0000000..eed7713 --- /dev/null +++ b/docs/publish-workflow-remediation.md @@ -0,0 +1,231 @@ +--- +title: Publish Workflow Remediation Plan +summary: Prioritized fixes for dotnet.publish.yml — Release gating, metadata-driven packing, version validation, and CI hardening. +tags: [ci, remediation-plan] +created: 2026-06-13 +document.status: implemented +--- + +# Publish Workflow Remediation Plan + +Findings from a best-practices review of `.github/workflows/dotnet.publish.yml`. +Trusted Publishing (OIDC) is **deferred** — tracked separately as a cross-repo +initiative — and is intentionally out of scope here. + +**Status:** Items 1–9 implemented on branch `ci/publish-hardening`. Item 10 +(paths-filter vs. required status checks) is open — it depends on branch-protection +settings and is left to the maintainer. + +Ordered by priority. Items are independent unless a dependency is noted. + +--- + +## 1. Gate on the shipped configuration (Release), not Debug + +**Important.** + +**Problem.** The `test` job builds `-c Debug` and runs `dotnet test --no-build` +(defaults to Debug), but `publish` packs `-c Release`. The Release binaries that +are actually distributed are never compiled or tested at publish time. The +matrix coverage in `dotnet.tests.yml` does not help — that workflow does not run +on the `release` trigger. + +**Fix.** Build and test Release in the gate job: + +```yaml + - name: Build + run: dotnet build -c Release --no-restore + - name: Test + run: dotnet test -c Release --no-build +``` + +**Files.** `.github/workflows/dotnet.publish.yml`. + +**Verify.** Trigger a pre-release (or `workflow_dispatch` on a branch) and +confirm the gate job logs `Release` for both build and test. + +--- + +## 2. Derive the package set from project metadata, not the YAML + +**Important.** Depends on a one-line project change (2a) before the workflow +change (2b). + +**Problem.** Four hand-maintained `dotnet pack src/X` steps duplicate the list +of publishable projects. Adding a fifth library and forgetting a pack step +silently drops it from the release. + +**Fix.** + +- **2a.** Mark the sample as non-packable so a solution-wide pack only emits the + four libraries: + + ```xml + + false + ``` + + (`*.Tests` projects already set `IsPackable=false` via `Directory.Build.props`.) + +- **2b.** Replace the four pack steps with one: + + ```yaml + - name: Pack + run: dotnet pack Plumber.slnx -c Release --no-restore -o nuget -p:PackageVersion=$PACKAGE_VERSION -p:Version=$PACKAGE_VERSION + ``` + +**Files.** `samples/Sample.Cli/Sample.Cli.csproj`, +`.github/workflows/dotnet.publish.yml`. + +**Verify.** Run `dotnet pack Plumber.slnx -c Release -o /tmp/nuget` locally; +confirm exactly four `MSL.Plumber.*.nupkg` (plus matching `.snupkg`) appear and +no `Sample.Cli` package is produced. + +--- + +## 3. Validate the resolved version before packing + +**Worth doing.** + +**Problem.** `version="${TAG#v}"` strips a leading `v` and trusts the rest. A +malformed or empty tag flows into pack/push and produces a garbage version. + +**Fix.** Fail fast after resolving the version: + +```yaml + - name: Resolve package version + env: + TAG: ${{ github.event.release.tag_name }} + run: | + version="${TAG#v}" + [[ "$version" =~ ^[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?$ ]] \ + || { echo "::error::invalid version '$version' from tag '$TAG'"; exit 1; } + echo "Publishing version $version" + echo "PACKAGE_VERSION=$version" >> "$GITHUB_ENV" +``` + +(GitHub's default `bash` shell already runs with `-eo pipefail`, so no extra +`set` is needed.) + +**Files.** `.github/workflows/dotnet.publish.yml`. + +**Verify.** A tag like `release-foo` or `v1.2` should fail the job at this step. + +--- + +## 4. Add a concurrency guard to the publish workflow + +**Minor.** `--skip-duplicate` already makes re-runs idempotent, so this is +defense-in-depth. + +**Problem.** Overlapping release runs can race on pack/push. + +**Fix.** Add at the workflow top level (mirrors `dotnet.tests.yml`, but without +`cancel-in-progress` — a publish in flight should finish): + +```yaml +concurrency: + group: publish-${{ github.event.release.tag_name }} + cancel-in-progress: false +``` + +**Files.** `.github/workflows/dotnet.publish.yml`. + +--- + +## 5. Pin the SDK with global.json + +**Minor.** Affects both workflows. + +**Problem.** Both workflows float on `dotnet-version: 10.0.x`, so the SDK can +drift between the test gate and any local repro. + +**Fix.** Add a `global.json` at the repo root with a `rollForward` policy, e.g.: + +```json +{ + "sdk": { + "version": "10.0.100", + "rollForward": "latestFeature" + } +} +``` + +Keep `setup-dotnet` as-is; it honors `global.json` when present. Pick the exact +baseline version from the current CI SDK. + +**Files.** `global.json` (new). No workflow change required. + +**Verify.** `dotnet --version` in CI matches the pinned baseline (modulo +`rollForward`). + +--- + +## 6. De-duplicate job setup + +**Minor.** Maintainability. + +**Problem.** `setup-dotnet` + NuGet cache are copy-pasted across the `test` and +`publish` jobs (and across both workflows). + +**Fix.** Extract a local composite action +(`.github/actions/setup-dotnet/action.yml`) bundling `setup-dotnet` and the +NuGet cache, and reference it from each job as `uses: ./.github/actions/setup-dotnet`. +`actions/checkout` stays in each job — a local composite action can only be +resolved after the repo is checked out, so it cannot be folded in. + +**Files.** `.github/actions/setup-dotnet/action.yml` (new), both workflows. + +--- + +## Suggested sequencing + +1. **#1 and #2** together — they are the substantive correctness/safety fixes + and both touch the publish workflow's core steps. +2. **#3** — small, self-contained hardening. +3. **#4 and #5** — low-risk guards; bundle into the same PR. +4. **#6** — only if/when the duplication becomes a maintenance burden. + +All of the above can land in a single PR against `lauterm/otel` (or a dedicated +`ci/publish-hardening` branch) since they are confined to CI config plus one +`IsPackable` flag. + +--- + +## Follow-up findings (second pass) + +### 7. README filename casing breaks pack on Linux — **fixed** + +**Important.** All four packable projects packed ``, +but the file is `README.md`. The publish job packs on `ubuntu-latest` +(case-sensitive filesystem), so `dotnet pack` would fail with **NU5039** at +release time. It passed locally only because Windows is case-insensitive, and no +CI job packs before the release event — so the bug was invisible until a release. +Corrected the include path to `README.md` in all four `src/*.csproj`. + +### 8. No deterministic-build flag in CI — **fixed** + +**Worth doing.** SourceLink was wired up (`Microsoft.SourceLink.GitHub`, +`PublishRepositoryUrl`, `EmbedUntrackedSources`) but nothing set +`ContinuousIntegrationBuild=true`, so packed binaries were not reproducible and +SourceLink paths were not normalized. Added a `Directory.Build.props` property +group gated on `'$(GITHUB_ACTIONS)' == 'true'` — on in CI, off locally. + +### 9. Publish path cannot be dry-run — **fixed** + +**Worth doing.** The workflow only triggered on `release: published`, so pack/push +could not be exercised without cutting a real release (which is how #7 stayed +hidden). Added a `workflow_dispatch` trigger with a `version` input; the +`nuget push` step is now guarded by `if: github.event_name == 'release'`, so a +manual run builds, tests, packs, and uploads the artifact without publishing. + +### 10. paths-filter vs. required status checks — **open, needs decision** + +**Verify.** `dotnet.tests.yml` uses `paths:` filters. If its checks are marked +**required** in branch protection, a PR that touches only non-matching paths +never triggers the workflow, so the required check never reports and the PR can +be blocked indefinitely. Resolution depends on the repo's branch-protection +settings (not visible from the tree): either rely on GitHub's "skipped = +success" behavior for required checks, scope required checks accordingly, or add +an always-run companion job that reports a neutral/green status. Left to the +maintainer. diff --git a/global.json b/global.json new file mode 100644 index 0000000..512142d --- /dev/null +++ b/global.json @@ -0,0 +1,6 @@ +{ + "sdk": { + "version": "10.0.100", + "rollForward": "latestFeature" + } +} diff --git a/samples/Sample.Cli.Tests/AssemblyInfo.cs b/samples/Sample.Cli.Tests/AssemblyInfo.cs new file mode 100644 index 0000000..387efca --- /dev/null +++ b/samples/Sample.Cli.Tests/AssemblyInfo.cs @@ -0,0 +1,4 @@ +// TelemetrySummaryReflectsAPipelineRunAsync registers process-global OpenTelemetry listeners on the +// pipeline's ActivitySource and Meter. Running test classes in parallel would let other pipeline runs leak +// spans and metrics into its collections, so this assembly runs its tests serially. +[assembly: CollectionBehavior(DisableTestParallelization = true)] diff --git a/samples/Sample.Cli.Tests/PipelineTests.cs b/samples/Sample.Cli.Tests/PipelineTests.cs index 280dfe1..00dda63 100644 --- a/samples/Sample.Cli.Tests/PipelineTests.cs +++ b/samples/Sample.Cli.Tests/PipelineTests.cs @@ -1,4 +1,6 @@ +using OpenTelemetry.Metrics; using Plumber; +using System.Diagnostics; namespace Sample.Cli.Tests; @@ -51,7 +53,9 @@ public void PipelineRegistersMiddlewareInOrder() Assert.Collection( handler.Middleware, - // the Serilog request logger is internal to Plumber.Serilog.Extensions, so assert by type name + // the OpenTelemetry and Serilog middleware are internal to their extension packages, so assert by type name + m => Assert.StartsWith("RequestTracingMiddleware", m.MiddlewareType!.Name, StringComparison.Ordinal), + m => Assert.StartsWith("RequestMetricsMiddleware", m.MiddlewareType!.Name, StringComparison.Ordinal), m => Assert.StartsWith("RequestLoggerMiddleware", m.MiddlewareType!.Name, StringComparison.Ordinal), m => Assert.Equal(MiddlewareDescriptor.DelegateDisplayName, m.DisplayName), // the timing delegate m => Assert.Equal(typeof(ValidationMiddleware), m.MiddlewareType), @@ -60,6 +64,25 @@ public void PipelineRegistersMiddlewareInOrder() m => Assert.Equal(typeof(ReportMiddleware), m.MiddlewareType)); } + [Fact] + public async Task TelemetrySummaryReflectsAPipelineRunAsync() + { + List spans = []; + List metrics = []; + using var tracerProvider = Telemetry.CreateTracerProvider(spans); + using var meterProvider = Telemetry.CreateMeterProvider(metrics); + using var handler = Pipeline.Build([]); + + _ = await handler.InvokeAsync("Hello, World!", TestContext.Current.CancellationToken); + + _ = meterProvider.ForceFlush(); + var summary = Telemetry.Summarize(spans, metrics); + + Assert.Contains("Plumber.HandleRequest", summary, StringComparison.Ordinal); + Assert.Contains("plumber.requests.count", summary, StringComparison.Ordinal); + Assert.Contains("plumber.requests.duration", summary, StringComparison.Ordinal); + } + [Fact] public async Task ShortCircuitStillRecordsElapsedAsync() { diff --git a/samples/Sample.Cli.Tests/Sample.Cli.Tests.csproj b/samples/Sample.Cli.Tests/Sample.Cli.Tests.csproj index ded2939..b4c029f 100644 --- a/samples/Sample.Cli.Tests/Sample.Cli.Tests.csproj +++ b/samples/Sample.Cli.Tests/Sample.Cli.Tests.csproj @@ -4,7 +4,8 @@ [Sample.Cli]* - 80,40,80 + + 80,45,80 $(NoWarn);CA1515 diff --git a/samples/Sample.Cli/Pipeline.cs b/samples/Sample.Cli/Pipeline.cs index 00d3794..311cac6 100644 --- a/samples/Sample.Cli/Pipeline.cs +++ b/samples/Sample.Cli/Pipeline.cs @@ -2,6 +2,7 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Plumber; +using Plumber.Diagnostics; using Plumber.Serilog.Extensions; using Serilog; using Serilog.Formatting.Compact; @@ -18,19 +19,16 @@ public static RequestHandlerBuilder CreateBuilder(string[] a new($"{TokenizerOptions.SectionName}:{nameof(TokenizerOptions.RemoveEmptyEntries)}", TokenizerOptions.Defaults.RemoveEmptyEntries.ToString()), new($"{TokenizerOptions.SectionName}:{nameof(TokenizerOptions.TrimEntries)}", TokenizerOptions.Defaults.TrimEntries.ToString()), ])) - .ConfigureLogging(logging => logging - .SetMinimumLevel(LogLevel.Information) - .AddSimpleConsole(o => - { - o.SingleLine = true; - o.IncludeScopes = false; - })) + // Register the Microsoft.Extensions.Logging stack so middleware can take ILogger, but add no + // console provider — structured console output is Serilog's job (AddSerilogRequestLogging below). + .ConfigureLogging(logging => logging.SetMinimumLevel(LogLevel.Information)) .ConfigureServices((services, configuration) => { var options = configuration.GetSection(TokenizerOptions.SectionName).Get() ?? TokenizerOptions.Defaults; _ = services .AddSerilogRequestLogging(logger => logger.WriteTo.Console(new CompactJsonFormatter())) + .AddPlumberDiagnostics() .AddSingleton(options) .AddSingleton(); }); @@ -39,7 +37,10 @@ public static RequestHandlerBuilder CreateBuilder(string[] a Justification = "fluent .Use() returns the same handler instance; caller disposes")] public static RequestHandler Configure(RequestHandler handler) => handler + .UseRequestDiagnostics() .UseSerilogRequestLogging() + // Inline timer feeds the report payload's Elapsed (which the CLI prints). The OpenTelemetry span + // duration and the Serilog event measure observability timing separately; this one shapes output. .Use(async (context, next) => { var start = DateTime.UtcNow; diff --git a/samples/Sample.Cli/Program.cs b/samples/Sample.Cli/Program.cs index 32bcddf..e8a2315 100644 --- a/samples/Sample.Cli/Program.cs +++ b/samples/Sample.Cli/Program.cs @@ -1,9 +1,17 @@ +using OpenTelemetry.Metrics; using Sample.Cli; +using System.Diagnostics; var input = args.Length > 0 ? string.Join(' ', args) : await Console.In.ReadToEndAsync(); +// Host-free OpenTelemetry: the in-memory exporters collect into these lists while the providers' listeners +// are active, then Telemetry.Summarize prints a compact confirmation instead of dumping every span/metric. +List spans = []; +List metrics = []; +using var tracerProvider = Telemetry.CreateTracerProvider(spans); +using var meterProvider = Telemetry.CreateMeterProvider(metrics); using var handler = Pipeline.Build(args); var report = await handler.InvokeAsync(input); @@ -24,4 +32,8 @@ Console.WriteLine($"tokens: [{string.Join(", ", report.Tokens)}]"); Console.WriteLine($"words: {report.WordCount}"); Console.WriteLine($"elapsed: {report.Elapsed.TotalMilliseconds:F2}ms"); + +// Flush the metrics so the in-memory exporter has them, then show what OpenTelemetry collected. +_ = meterProvider.ForceFlush(); +Console.WriteLine(Telemetry.Summarize(spans, metrics)); return 0; diff --git a/samples/Sample.Cli/Sample.Cli.csproj b/samples/Sample.Cli/Sample.Cli.csproj index 844b09f..b46d057 100644 --- a/samples/Sample.Cli/Sample.Cli.csproj +++ b/samples/Sample.Cli/Sample.Cli.csproj @@ -2,18 +2,22 @@ Exe + + false + - + + diff --git a/samples/Sample.Cli/Telemetry.cs b/samples/Sample.Cli/Telemetry.cs new file mode 100644 index 0000000..2549d98 --- /dev/null +++ b/samples/Sample.Cli/Telemetry.cs @@ -0,0 +1,68 @@ +using OpenTelemetry; +using OpenTelemetry.Metrics; +using OpenTelemetry.Resources; +using OpenTelemetry.Trace; +using Plumber.Diagnostics; +using System.Diagnostics; + +namespace Sample.Cli; + +/// +/// Host-free OpenTelemetry bootstrap: builds the tracer and meter providers that collect the pipeline's +/// spans and metrics into the supplied lists. The providers are built eagerly so their listeners are active +/// for the middleware's and , and +/// the caller disposes them at exit. The in-memory exporter keeps the demo console legible — instead of +/// dumping every span and metric, prints a one-line confirmation that collection worked. +/// +internal static class Telemetry +{ + public const string ServiceName = "Sample.Cli"; + + public static TracerProvider CreateTracerProvider(ICollection exportedSpans) => + Sdk.CreateTracerProviderBuilder() + .ConfigureResource(resource => resource.AddService(ServiceName)) + .AddSource(PlumberDiagnostics.ActivitySourceName) + .AddInMemoryExporter(exportedSpans) + .Build(); + + public static MeterProvider CreateMeterProvider(ICollection exportedMetrics) => + Sdk.CreateMeterProviderBuilder() + .ConfigureResource(resource => resource.AddService(ServiceName)) + .AddMeter(PlumberDiagnostics.MeterName) + .AddInMemoryExporter(exportedMetrics) + .Build(); + + /// + /// Renders a compact summary of the spans and metrics the in-memory exporters collected, so a sample run + /// shows that OpenTelemetry collection worked without the full exporter dump. + /// + public static string Summarize(IReadOnlyCollection spans, IReadOnlyCollection metrics) + { + List lines = [$"opentelemetry: {spans.Count} span(s) collected via the in-memory exporter"]; + lines.AddRange(spans.Select(span => $" span {span.OperationName} ({span.Status})")); + lines.AddRange(metrics.Select(DescribeMetric)); + return string.Join(Environment.NewLine, lines); + } + + private static string DescribeMetric(Metric metric) + { + if (metric.MetricType == MetricType.Histogram) + { + long samples = 0; + foreach (ref readonly var point in metric.GetMetricPoints()) + { + samples += point.GetHistogramCount(); + } + + return $" metric {metric.Name} ({samples} sample(s))"; + } + + long sum = 0; + foreach (ref readonly var point in metric.GetMetricPoints()) + { + sum += point.GetSumLong(); + } + + return $" metric {metric.Name} = {sum}"; + } +} diff --git a/src/Plumber.Diagnostics/Plumber.Diagnostics.csproj b/src/Plumber.Diagnostics/Plumber.Diagnostics.csproj new file mode 100644 index 0000000..69eb9f9 --- /dev/null +++ b/src/Plumber.Diagnostics/Plumber.Diagnostics.csproj @@ -0,0 +1,56 @@ + + + + MSL.Plumber.Diagnostics + Plumber.Diagnostics: OpenTelemetry Tracing and Metrics for Plumber Pipelines + Mark Lauter + OpenTelemetry tracing and metrics for the Plumber pipeline framework, with no OpenTelemetry SDK dependency: per-request spans and metrics are emitted through the in-box ActivitySource and Meter primitives, so you export to any backend — OTLP, Jaeger, Prometheus, or the console — with your own SDK setup. Configurable per-request enrichment. + Copyright (c) 2024 Mark Lauter + https://github.com/marklauter/Plumber + https://github.com/marklauter/Plumber + git + middleware;pipeline;plumber;opentelemetry;otel;observability;tracing;distributed-tracing;metrics;telemetry;instrumentation;monitoring;activitysource;meter;dotnet;msl-armory + en-US + true + snupkg + README.md + LICENSE + plumber.png + + + + + + + + + + + all + runtime; build; native; contentfiles; analyzers; buildtransitive + + + + + + <_Parameter1>$(AssemblyName).Tests + + + + + + True + \ + + + True + README.md + + + True + \ + + + + diff --git a/src/Plumber.Diagnostics/PlumberDiagnostics.cs b/src/Plumber.Diagnostics/PlumberDiagnostics.cs new file mode 100644 index 0000000..8a349bc --- /dev/null +++ b/src/Plumber.Diagnostics/PlumberDiagnostics.cs @@ -0,0 +1,15 @@ +namespace Plumber.Diagnostics; + +/// +/// Names of the and +/// the middleware emits through. Pass these to the OpenTelemetry SDK so it collects Plumber telemetry, e.g. +/// AddSource(PlumberDiagnostics.ActivitySourceName) and AddMeter(PlumberDiagnostics.MeterName). +/// +public static class PlumberDiagnostics +{ + /// The name of the the tracing middleware starts spans on. + public const string ActivitySourceName = "Plumber.Diagnostics"; + + /// The name of the the metrics middleware records instruments on. + public const string MeterName = "Plumber.Diagnostics"; +} diff --git a/src/Plumber.Diagnostics/RequestHandlerDiagnosticsExtensions.cs b/src/Plumber.Diagnostics/RequestHandlerDiagnosticsExtensions.cs new file mode 100644 index 0000000..6f35bda --- /dev/null +++ b/src/Plumber.Diagnostics/RequestHandlerDiagnosticsExtensions.cs @@ -0,0 +1,141 @@ +using Microsoft.Extensions.Options; + +namespace Plumber.Diagnostics; + +/// +/// RequestHandlerDiagnosticsExtensions provides extension methods for registering the OpenTelemetry +/// tracing and metrics middleware with the . +/// +public static class RequestHandlerDiagnosticsExtensions +{ + /// + /// UseRequestTracing registers the tracing middleware with the . + /// + /// The pipeline request type. + /// The pipeline response type. + /// The to register the middleware with. + /// The same for chaining. + /// + /// Options are resolved from the request handler's service provider as ; register + /// them with , or omit registration + /// to use the defaults baked into . + /// + public static RequestHandler UseRequestTracing( + this RequestHandler handler) + where TRequest : class + { + ArgumentNullException.ThrowIfNull(handler); + return handler.Use>(); + } + + /// + /// UseRequestTracing registers the tracing middleware with the + /// and applies the supplied inline configuration. + /// + /// The pipeline request type. + /// The pipeline response type. + /// The to register the middleware with. + /// An action that configures for this registration. + /// The same for chaining. + /// + /// The configured options are passed directly to the middleware and take precedence over any registered in the service provider. + /// + public static RequestHandler UseRequestTracing( + this RequestHandler handler, + Action> configureOptions) + where TRequest : class + { + ArgumentNullException.ThrowIfNull(handler); + ArgumentNullException.ThrowIfNull(configureOptions); + + var options = new RequestTracingOptions(); + configureOptions(options); + return handler.Use>(Options.Create(options)); + } + + /// + /// UseRequestMetrics registers the metrics middleware with the . + /// + /// The pipeline request type. + /// The pipeline response type. + /// The to register the middleware with. + /// The same for chaining. + /// + /// Options are resolved from the request handler's service provider as ; register + /// them with , or omit registration + /// to use the defaults baked into . + /// + public static RequestHandler UseRequestMetrics( + this RequestHandler handler) + where TRequest : class + { + ArgumentNullException.ThrowIfNull(handler); + return handler.Use>(); + } + + /// + /// UseRequestMetrics registers the metrics middleware with the + /// and applies the supplied inline configuration. + /// + /// The pipeline request type. + /// The pipeline response type. + /// The to register the middleware with. + /// An action that configures for this registration. + /// The same for chaining. + /// + /// The configured options are passed directly to the middleware and take precedence over any registered in the service provider. + /// + public static RequestHandler UseRequestMetrics( + this RequestHandler handler, + Action> configureOptions) + where TRequest : class + { + ArgumentNullException.ThrowIfNull(handler); + ArgumentNullException.ThrowIfNull(configureOptions); + + var options = new RequestMetricsOptions(); + configureOptions(options); + return handler.Use>(Options.Create(options)); + } + + /// + /// UseRequestDiagnostics registers both the tracing and metrics middleware with the . + /// + /// The pipeline request type. + /// The pipeline response type. + /// The to register the middleware with. + /// The same for chaining. + public static RequestHandler UseRequestDiagnostics( + this RequestHandler handler) + where TRequest : class + { + ArgumentNullException.ThrowIfNull(handler); + return handler + .UseRequestTracing() + .UseRequestMetrics(); + } + + /// + /// UseRequestDiagnostics registers both the tracing and metrics middleware with the + /// and applies the supplied inline configuration to each. + /// + /// The pipeline request type. + /// The pipeline response type. + /// The to register the middleware with. + /// An action that configures for the tracing middleware. + /// An action that configures for the metrics middleware. + /// The same for chaining. + public static RequestHandler UseRequestDiagnostics( + this RequestHandler handler, + Action> configureTracingOptions, + Action> configureMetricsOptions) + where TRequest : class + { + ArgumentNullException.ThrowIfNull(handler); + ArgumentNullException.ThrowIfNull(configureTracingOptions); + ArgumentNullException.ThrowIfNull(configureMetricsOptions); + return handler + .UseRequestTracing(configureTracingOptions) + .UseRequestMetrics(configureMetricsOptions); + } +} diff --git a/src/Plumber.Diagnostics/RequestMetricsMiddleware{TRequest, TResponse}.cs b/src/Plumber.Diagnostics/RequestMetricsMiddleware{TRequest, TResponse}.cs new file mode 100644 index 0000000..c3f927e --- /dev/null +++ b/src/Plumber.Diagnostics/RequestMetricsMiddleware{TRequest, TResponse}.cs @@ -0,0 +1,105 @@ +using Microsoft.Extensions.Options; +using System.Diagnostics.Metrics; + +namespace Plumber.Diagnostics; + +internal sealed class RequestMetricsMiddleware + where TRequest : class +{ + private static readonly Meter Meter = new(PlumberDiagnostics.MeterName); + + private static readonly Counter RequestCounter = Meter.CreateCounter( + "plumber.requests.count", + description: "Counts the number of requests processed"); + + private static readonly Histogram RequestDuration = Meter.CreateHistogram( + "plumber.requests.duration", + unit: "ms", + description: "Measures the duration of requests"); + + private static readonly Counter ErrorCounter = Meter.CreateCounter( + "plumber.requests.errors", + description: "Counts the number of request errors"); + + private readonly RequestMiddleware next; + private readonly bool addDefaultMetrics; + private readonly Action, bool>? recordCustomMetrics; + private readonly bool throwOnException; + + public RequestMetricsMiddleware( + RequestMiddleware next, + IOptions> options) + { + ArgumentNullException.ThrowIfNull(next); + ArgumentNullException.ThrowIfNull(options); + + var value = options.Value; + this.next = next; + addDefaultMetrics = value.AddDefaultMetrics; + recordCustomMetrics = value.RecordCustomMetrics; + throwOnException = value.ThrowOnException; + } + + public async Task InvokeAsync(RequestContext context) + { + ArgumentNullException.ThrowIfNull(context); + + var requestType = typeof(TRequest).Name; + if (addDefaultMetrics) + { + // Counted up front as an attempt, so a request that later cancels or fails still counts here: + // plumber.requests.count is throughput, and (count - errors) is not skewed by cancellation. + RequestCounter.Add(1, new KeyValuePair("request.type", requestType)); + } + + try + { + context.CancellationToken.ThrowIfCancellationRequested(); + await next(context); + } + catch (OperationCanceledException) + { + // Cancellation — caller-initiated or a Plumber timeout, indistinguishable at this layer — is an + // expected outcome, not a defect: it stays out of plumber.requests.errors and the duration + // histogram, and the custom-metrics hook (resolved outcomes only) does not fire. ThrowOnException + // still governs propagation. + if (throwOnException) + { + throw; + } + + return; + } + catch (Exception) + { + if (addDefaultMetrics) + { + ErrorCounter.Add(1, new KeyValuePair("request.type", requestType)); + RequestDuration.Record( + context.Elapsed.TotalMilliseconds, + new KeyValuePair("request.type", requestType), + new KeyValuePair("success", false)); + } + + recordCustomMetrics?.Invoke(context, false); + + if (throwOnException) + { + throw; + } + + return; + } + + if (addDefaultMetrics) + { + // RequestContext.Elapsed is measured from the injected TimeProvider, so timing stays testable. + RequestDuration.Record( + context.Elapsed.TotalMilliseconds, + new KeyValuePair("request.type", requestType), + new KeyValuePair("success", true)); + } + + recordCustomMetrics?.Invoke(context, true); + } +} diff --git a/src/Plumber.Diagnostics/RequestMetricsOptions.cs b/src/Plumber.Diagnostics/RequestMetricsOptions.cs new file mode 100644 index 0000000..ed4df70 --- /dev/null +++ b/src/Plumber.Diagnostics/RequestMetricsOptions.cs @@ -0,0 +1,31 @@ +namespace Plumber.Diagnostics; + +/// +/// RequestMetricsOptions lets you configure the OpenTelemetry metrics middleware. +/// +/// The pipeline request type. +/// The pipeline response type. +public sealed class RequestMetricsOptions + where TRequest : class +{ + /// + /// ThrowOnException lets you set whether the middleware should rethrow exceptions thrown by downstream middleware components. + /// + public bool ThrowOnException { get; set; } = true; + + /// + /// AddDefaultMetrics lets you configure whether default metrics should be recorded. + /// + /// + /// Default metrics include request count, duration, and error count. + /// + public bool AddDefaultMetrics { get; set; } = true; + + /// + /// RecordCustomMetrics lets you provide an action to record custom metrics for each request. + /// + /// + /// The boolean parameter indicates whether the request was successful (true) or resulted in an error (false). + /// + public Action, bool>? RecordCustomMetrics { get; set; } +} diff --git a/src/Plumber.Diagnostics/RequestTracingMiddleware{TRequest, TResponse}.cs b/src/Plumber.Diagnostics/RequestTracingMiddleware{TRequest, TResponse}.cs new file mode 100644 index 0000000..742cf6e --- /dev/null +++ b/src/Plumber.Diagnostics/RequestTracingMiddleware{TRequest, TResponse}.cs @@ -0,0 +1,108 @@ +using Microsoft.Extensions.Options; +using System.Diagnostics; + +namespace Plumber.Diagnostics; + +internal sealed class RequestTracingMiddleware + where TRequest : class +{ + private static readonly ActivitySource ActivitySource = new(PlumberDiagnostics.ActivitySourceName); + + private readonly RequestMiddleware next; + private readonly string operationName; + private readonly Action>? enrichSpan; + private readonly bool recordException; + private readonly bool throwOnException; + private readonly ActivityKind spanKind; + private readonly bool addDefaultAttributes; + + public RequestTracingMiddleware( + RequestMiddleware next, + IOptions> options) + { + ArgumentNullException.ThrowIfNull(next); + ArgumentNullException.ThrowIfNull(options); + + var value = options.Value; + this.next = next; + operationName = value.OperationName; + enrichSpan = value.EnrichSpan; + recordException = value.RecordException; + throwOnException = value.ThrowOnException; + spanKind = value.SpanKind; + addDefaultAttributes = value.AddDefaultAttributes; + } + + public async Task InvokeAsync(RequestContext context) + { + ArgumentNullException.ThrowIfNull(context); + + using var activity = ActivitySource.StartActivity(operationName, spanKind); + + // No listener sampled this source, so there's no span to enrich — skip straight through. + if (activity is null) + { + await next(context); + return; + } + + if (addDefaultAttributes) + { + _ = activity + .SetTag("request.id", context.Id) + .SetTag("request.type", typeof(TRequest).Name); + } + + try + { + context.CancellationToken.ThrowIfCancellationRequested(); + await next(context); + + if (addDefaultAttributes) + { + // RequestContext.Elapsed is measured from the injected TimeProvider, so timing stays testable. + _ = activity + .SetTag("request.elapsed_ms", context.Elapsed.TotalMilliseconds) + .SetTag("response.type", context.Response?.GetType().Name ?? "null"); + } + + enrichSpan?.Invoke(activity, context); + _ = activity.SetStatus(ActivityStatusCode.Ok); + } + catch (OperationCanceledException) + { + // Cancellation — caller-initiated or a Plumber timeout, indistinguishable at this layer — is an + // expected outcome, not a defect: leave the span Unset and record no exception event. The span's + // own Duration already captures timing. ThrowOnException still governs propagation. + enrichSpan?.Invoke(activity, context); + + if (throwOnException) + { + throw; + } + } + catch (Exception ex) + { + if (addDefaultAttributes) + { + _ = activity.SetTag("request.elapsed_ms", context.Elapsed.TotalMilliseconds); + } + + if (recordException) + { + // Activity.AddException (net9+) records an exception event with the OpenTelemetry + // semantic-convention tags, so the library needs no dependency on the OpenTelemetry SDK. + _ = activity + .SetStatus(ActivityStatusCode.Error, ex.Message) + .AddException(ex); + } + + enrichSpan?.Invoke(activity, context); + + if (throwOnException) + { + throw; + } + } + } +} diff --git a/src/Plumber.Diagnostics/RequestTracingOptions.cs b/src/Plumber.Diagnostics/RequestTracingOptions.cs new file mode 100644 index 0000000..82515e9 --- /dev/null +++ b/src/Plumber.Diagnostics/RequestTracingOptions.cs @@ -0,0 +1,47 @@ +using System.Diagnostics; + +namespace Plumber.Diagnostics; + +/// +/// RequestTracingOptions lets you configure the OpenTelemetry tracing middleware. +/// +/// The pipeline request type. +/// The pipeline response type. +public sealed class RequestTracingOptions + where TRequest : class +{ + private const string DefaultOperationName = "Plumber.HandleRequest"; + + /// + /// OperationName lets you configure the name of the operation/span. + /// + public string OperationName { get; set; } = DefaultOperationName; + + /// + /// EnrichSpan lets you provide an action to enrich the span with additional information per request. + /// + public Action>? EnrichSpan { get; set; } + + /// + /// RecordException lets you configure whether exceptions should be recorded on the span. + /// + public bool RecordException { get; set; } = true; + + /// + /// ThrowOnException lets you set whether the middleware should rethrow exceptions thrown by downstream middleware components. + /// + public bool ThrowOnException { get; set; } = true; + + /// + /// SpanKind lets you set the kind of span to be created. + /// + public ActivityKind SpanKind { get; set; } = ActivityKind.Internal; + + /// + /// AddDefaultAttributes lets you configure whether default attributes should be added to the span. + /// + /// + /// Default attributes include request ID, request type, elapsed time, and response type. + /// + public bool AddDefaultAttributes { get; set; } = true; +} diff --git a/src/Plumber.Diagnostics/ServiceCollectionDiagnosticsExtensions.cs b/src/Plumber.Diagnostics/ServiceCollectionDiagnosticsExtensions.cs new file mode 100644 index 0000000..e1dcc48 --- /dev/null +++ b/src/Plumber.Diagnostics/ServiceCollectionDiagnosticsExtensions.cs @@ -0,0 +1,51 @@ +using Microsoft.Extensions.DependencyInjection; + +namespace Plumber.Diagnostics; + +/// +/// ServiceCollectionDiagnosticsExtensions registers the options infrastructure the OpenTelemetry +/// tracing and metrics middleware resolve from the request handler's service provider. +/// +public static class ServiceCollectionDiagnosticsExtensions +{ + /// + /// Registers the and + /// options infrastructure so the parameterless + /// UseRequestTracing, UseRequestMetrics, and UseRequestDiagnostics + /// overloads resolve their dependency to the + /// configured values (or the baked-in defaults). + /// + /// The pipeline request type. + /// The pipeline response type. + /// The service collection to register with. + /// Optional configuration for . + /// Optional configuration for . + /// The same for chaining. + /// + /// This registers only Plumber's options; collecting the emitted telemetry is the consumer's job — register the + /// OpenTelemetry SDK separately and subscribe to and + /// via AddSource / AddMeter. + /// + public static IServiceCollection AddPlumberDiagnostics( + this IServiceCollection services, + Action>? configureTracing = null, + Action>? configureMetrics = null) + where TRequest : class + { + ArgumentNullException.ThrowIfNull(services); + + var tracing = services.AddOptions>(); + if (configureTracing is not null) + { + _ = tracing.Configure(configureTracing); + } + + var metrics = services.AddOptions>(); + if (configureMetrics is not null) + { + _ = metrics.Configure(configureMetrics); + } + + return services; + } +} diff --git a/src/Plumber.Serilog.Extensions/Plumber.Serilog.Extensions.csproj b/src/Plumber.Serilog.Extensions/Plumber.Serilog.Extensions.csproj index 8859c1b..88d53b9 100644 --- a/src/Plumber.Serilog.Extensions/Plumber.Serilog.Extensions.csproj +++ b/src/Plumber.Serilog.Extensions/Plumber.Serilog.Extensions.csproj @@ -2,14 +2,14 @@ MSL.Plumber.Serilog.Extensions - Plumber.Serilog.Extensions: Serilog Request Logging for Plumber Pipelines + Plumber.Serilog.Extensions: Serilog Structured Request Logging for Plumber Pipelines Mark Lauter - Serilog middleware for the Plumber pipeline framework: structured per-request logging with a Serilog DiagnosticContext, configurable level, message template, and enrichment. + Serilog middleware for the Plumber pipeline framework: structured per-request logging through a Serilog DiagnosticContext, with configurable level, message template, and enrichment. Completion events carry the active trace's TraceId and SpanId, so logs correlate with your Plumber.Diagnostics spans. Copyright (c) 2024 Mark Lauter https://github.com/marklauter/Plumber https://github.com/marklauter/Plumber git - middleware;pipeline;plumber;serilog;logging;log;request-logging;structured-logging;dotnet;msl-armory + middleware;pipeline;plumber;serilog;logging;structured-logging;request-logging;diagnostic-context;observability;trace-correlation;dotnet;msl-armory en-US true snupkg @@ -43,7 +43,7 @@ True \ - + True README.md diff --git a/src/Plumber.Testing/Plumber.Testing.csproj b/src/Plumber.Testing/Plumber.Testing.csproj index dca3563..9329554 100644 --- a/src/Plumber.Testing/Plumber.Testing.csproj +++ b/src/Plumber.Testing/Plumber.Testing.csproj @@ -2,14 +2,14 @@ MSL.Plumber.Pipeline.Testing - Plumber.Testing: In-Process Test Host for Plumber Pipelines + Plumber.Testing: WebApplicationFactory-Style Test Host for Plumber Pipelines Mark Lauter - Test host for Plumber middleware pipelines. PlumberApplicationFactory models the WebApplicationFactory pattern: bootstraps the application's real builder and pipeline, then lets tests override services, configuration, and logging before the pipeline is built. + WebApplicationFactory-style integration testing for Plumber pipelines. PlumberApplicationFactory bootstraps your application's real builder and pipeline, then lets tests override services, configuration, and logging before the pipeline is built. Copyright (c) 2024 Mark Lauter https://github.com/marklauter/Plumber https://github.com/marklauter/Plumber git - middleware;pipeline;plumber;testing;integration-testing;test-host;webapplicationfactory;dependency-injection;dotnet;msl-armory + middleware;pipeline;plumber;testing;integration-testing;integration-tests;test-host;test-fixture;webapplicationfactory;dependency-injection;dotnet;msl-armory en-US true snupkg @@ -34,7 +34,7 @@ True \ - + True README.md diff --git a/src/Plumber/Plumber.csproj b/src/Plumber/Plumber.csproj index 9b9ce44..f3ee603 100644 --- a/src/Plumber/Plumber.csproj +++ b/src/Plumber/Plumber.csproj @@ -2,14 +2,14 @@ MSL.Plumber.Pipeline - Plumber: Middleware Pipelines for Host-Free .NET + Plumber: ASP.NET Core-Style Middleware Pipelines for Console, Worker & Serverless .NET Mark Lauter - The ASP.NET Core middleware-pipeline model for host-free .NET — console apps, AWS Lambdas, Azure Functions, and queue consumers. Compose a request/response pipeline from delegate- and class-based middleware with dependency injection, configuration, logging, per-request scoping, timeouts, and cancellation. + The ASP.NET Core middleware-pipeline model, without the web host — for console apps, workers, AWS Lambda, Azure Functions, and queue consumers. Compose a request/response pipeline from delegate- and class-based middleware, built on the Microsoft.Extensions stack you already know: dependency injection, configuration, logging, per-request scoping, timeouts, and cancellation. Copyright (c) 2024 Mark Lauter https://github.com/marklauter/Plumber https://github.com/marklauter/Plumber git - middleware;pipeline;plumber;chain-of-responsibility;dependency-injection;console;queue;worker;request-handler;timeout;dotnet;msl-armory + middleware;middleware-pipeline;pipeline;plumber;chain-of-responsibility;dependency-injection;di;console;worker;background-service;serverless;aws-lambda;azure-functions;queue;request-handler;cancellation;timeout;dotnet;msl-armory en-US true snupkg @@ -49,7 +49,7 @@ True \ - + True README.md diff --git a/src/Plumber/RequestHandlerBuilder{TRequest, TResponse}.cs b/src/Plumber/RequestHandlerBuilder{TRequest, TResponse}.cs index a1a10c5..dcc1673 100644 --- a/src/Plumber/RequestHandlerBuilder{TRequest, TResponse}.cs +++ b/src/Plumber/RequestHandlerBuilder{TRequest, TResponse}.cs @@ -218,8 +218,9 @@ public RequestHandler Build(TimeSpan timeout) } catch { - // DI hasn't captured the configuration yet — dispose it ourselves so file watchers don't leak - (configuration as IDisposable)?.Dispose(); + // DI hasn't captured the configuration yet — dispose it ourselves so file watchers don't leak. + // ConfigurationBuilder.Build() always returns a ConfigurationRoot, which is IDisposable, so cast directly. + ((IDisposable)configuration).Dispose(); throw; } } diff --git a/src/Plumber/RequestHandler{TRequest, TResponse}.cs b/src/Plumber/RequestHandler{TRequest, TResponse}.cs index 910d73d..3844bae 100644 --- a/src/Plumber/RequestHandler{TRequest, TResponse}.cs +++ b/src/Plumber/RequestHandler{TRequest, TResponse}.cs @@ -41,8 +41,9 @@ internal RequestHandler( { // We own the provider, but no instance escapes the ctor to call Dispose, so tear it down here. // Sync Dispose is correct: an async-only singleton could only exist if the throwing resolution - // had already created one, which is pathological. - (Services as IDisposable)?.Dispose(); + // had already created one, which is pathological. BuildServiceProvider always returns an + // IDisposable provider, so cast directly rather than guard a case that cannot occur. + ((IDisposable)Services).Dispose(); throw; } @@ -417,8 +418,9 @@ private static RequestMiddleware Compile( call = Expression.Convert(call, typeof(Task)); } - // preserve the "returned null" diagnostic from the previous reflection-based dispatch - var nullMessage = $"{method.DeclaringType?.FullName}.{method.Name} returned null."; + // preserve the "returned null" diagnostic from the previous reflection-based dispatch. + // DeclaringType is non-null for an InvokeAsync method resolved from a class via GetMethods. + var nullMessage = $"{method.DeclaringType!.FullName}.{method.Name} returned null."; var throwOnNull = Expression.Throw( Expression.New( typeof(InvalidOperationException).GetConstructor([typeof(string)])!, @@ -444,7 +446,9 @@ public void Dispose() if (ownsProvider) { - (Services as IDisposable)?.Dispose(); + // the owned provider always comes from ServiceCollection.BuildServiceProvider, which is IDisposable; + // cast directly to match the IAsyncDisposable cast in DisposeAsync rather than guard an impossible case + ((IDisposable)Services).Dispose(); } disposed = true; diff --git a/tests/Plumber.Diagnostics.Tests/Architecture/ArchitectureTests.cs b/tests/Plumber.Diagnostics.Tests/Architecture/ArchitectureTests.cs new file mode 100644 index 0000000..cf17f8e --- /dev/null +++ b/tests/Plumber.Diagnostics.Tests/Architecture/ArchitectureTests.cs @@ -0,0 +1,12 @@ +using System.Reflection; + +namespace Plumber.Diagnostics.Tests.Architecture; + +// Plumber.Diagnostics's architecture rules: the shared invariants from +// global::Architecture.Testing.ArchitectureTestsBase, with no assembly-specific additions. +public sealed class ArchitectureTests : global::Architecture.Testing.ArchitectureTestsBase +{ + protected override Assembly TargetAssembly => typeof(RequestTracingOptions<,>).Assembly; + + protected override string RootNamespace => "Plumber.Diagnostics"; +} diff --git a/tests/Plumber.Diagnostics.Tests/AssemblyInfo.cs b/tests/Plumber.Diagnostics.Tests/AssemblyInfo.cs new file mode 100644 index 0000000..6d5240d --- /dev/null +++ b/tests/Plumber.Diagnostics.Tests/AssemblyInfo.cs @@ -0,0 +1,4 @@ +// The tracing and metrics middleware emit through a process-global ActivitySource and Meter, and the +// test collectors subscribe by name. Parallel test classes would cross-contaminate each other's spans and +// measurements, so this assembly runs its tests serially. +[assembly: CollectionBehavior(DisableTestParallelization = true)] diff --git a/tests/Plumber.Diagnostics.Tests/Plumber.Diagnostics.Tests.csproj b/tests/Plumber.Diagnostics.Tests/Plumber.Diagnostics.Tests.csproj new file mode 100644 index 0000000..775738f --- /dev/null +++ b/tests/Plumber.Diagnostics.Tests/Plumber.Diagnostics.Tests.csproj @@ -0,0 +1,15 @@ + + + + [Plumber.Diagnostics]* + + + $(NoWarn);CA1515;CA1812 + + + + + + + + diff --git a/tests/Plumber.Diagnostics.Tests/RequestDiagnosticsAndGuardTests.cs b/tests/Plumber.Diagnostics.Tests/RequestDiagnosticsAndGuardTests.cs new file mode 100644 index 0000000..40f11a1 --- /dev/null +++ b/tests/Plumber.Diagnostics.Tests/RequestDiagnosticsAndGuardTests.cs @@ -0,0 +1,84 @@ +using Microsoft.Extensions.DependencyInjection; +using System.Diagnostics.CodeAnalysis; + +namespace Plumber.Diagnostics.Tests; + +public sealed class RequestDiagnosticsAndGuardTests +{ + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "fluent UseRequestDiagnostics/Use return the same handler instance; the using var owns disposal")] + public async Task RequestDiagnosticsRegistersBothTracingAndMetricsAsync() + { + using var spans = new ActivityCollector(PlumberDiagnostics.ActivitySourceName); + using var metrics = new MeterCollector(PlumberDiagnostics.MeterName); + using var handler = TestPipeline.CreateHandler(); + + _ = handler + .UseRequestDiagnostics() + .Use(TestPipeline.Succeeds); + + _ = await handler.InvokeAsync(new TestRequest(), TestContext.Current.CancellationToken); + + _ = Assert.Single(spans.Activities); + _ = Assert.Single(metrics.Measurements, m => m.Instrument == "plumber.requests.count"); + } + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "fluent UseRequestDiagnostics/Use return the same handler instance; the using var owns disposal")] + public async Task RequestDiagnosticsWithConfigurationAppliesBothOptionsAsync() + { + using var spans = new ActivityCollector(PlumberDiagnostics.ActivitySourceName); + using var metrics = new MeterCollector(PlumberDiagnostics.MeterName); + using var handler = TestPipeline.CreateHandler(); + + _ = handler + .UseRequestDiagnostics( + tracing => tracing.OperationName = "MonitorOperation", + metric => metric.AddDefaultMetrics = true) + .Use(TestPipeline.Succeeds); + + _ = await handler.InvokeAsync(new TestRequest(), TestContext.Current.CancellationToken); + + Assert.Equal("MonitorOperation", Assert.Single(spans.Activities).OperationName); + _ = Assert.Single(metrics.Measurements, m => m.Instrument == "plumber.requests.count"); + } + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP005:Return type should indicate that the value should be disposed", + Justification = "the guarded extension throws ArgumentNullException before constructing anything; no disposable is created")] + public void ExtensionsGuardAgainstNullHandler() + { + RequestHandler handler = null!; + + _ = Assert.Throws(() => handler.UseRequestTracing()); + _ = Assert.Throws(() => handler.UseRequestTracing(_ => { })); + _ = Assert.Throws(() => handler.UseRequestMetrics()); + _ = Assert.Throws(() => handler.UseRequestMetrics(_ => { })); + _ = Assert.Throws(() => handler.UseRequestDiagnostics()); + _ = Assert.Throws(() => handler.UseRequestDiagnostics(_ => { }, _ => { })); + } + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "the using var owns disposal; the guarded calls throw before returning a handler")] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP005:Return type should indicate that the value should be disposed", + Justification = "the guarded extension throws ArgumentNullException before constructing anything; no disposable is created")] + public void ExtensionsGuardAgainstNullConfiguration() + { + using var handler = TestPipeline.CreateHandler(); + + _ = Assert.Throws(() => handler.UseRequestTracing(null!)); + _ = Assert.Throws(() => handler.UseRequestMetrics(null!)); + _ = Assert.Throws(() => handler.UseRequestDiagnostics(null!, _ => { })); + _ = Assert.Throws(() => handler.UseRequestDiagnostics(_ => { }, null!)); + } + + [Fact] + public void AddPlumberDiagnosticsGuardsAgainstNullServices() + { + IServiceCollection services = null!; + _ = Assert.Throws(() => services.AddPlumberDiagnostics()); + } +} diff --git a/tests/Plumber.Diagnostics.Tests/RequestMetricsTests.cs b/tests/Plumber.Diagnostics.Tests/RequestMetricsTests.cs new file mode 100644 index 0000000..dfadd33 --- /dev/null +++ b/tests/Plumber.Diagnostics.Tests/RequestMetricsTests.cs @@ -0,0 +1,222 @@ +using Microsoft.Extensions.Options; +using System.Diagnostics.CodeAnalysis; + +namespace Plumber.Diagnostics.Tests; + +public sealed class RequestMetricsTests +{ + private const string CountInstrument = "plumber.requests.count"; + private const string DurationInstrument = "plumber.requests.duration"; + private const string ErrorInstrument = "plumber.requests.errors"; + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "fluent UseRequestMetrics/Use return the same handler instance; the using var owns disposal")] + public async Task RecordsCountAndDurationOnSuccessAsync() + { + using var metrics = new MeterCollector(PlumberDiagnostics.MeterName); + using var handler = TestPipeline.CreateHandler(); + + _ = handler + .UseRequestMetrics() + .Use(TestPipeline.Succeeds); + + _ = await handler.InvokeAsync(new TestRequest(), TestContext.Current.CancellationToken); + + var count = Assert.Single(metrics.Measurements, m => m.Instrument == CountInstrument); + Assert.Equal(1, count.Value); + Assert.Equal(nameof(TestRequest), count.Tags["request.type"]); + + var duration = Assert.Single(metrics.Measurements, m => m.Instrument == DurationInstrument); + Assert.Equal(true, duration.Tags["success"]); + Assert.DoesNotContain(metrics.Measurements, m => m.Instrument == ErrorInstrument); + } + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "fluent UseRequestMetrics/Use return the same handler instance; the using var owns disposal")] + public async Task RecordsErrorAndDurationOnExceptionAsync() + { + using var metrics = new MeterCollector(PlumberDiagnostics.MeterName); + using var handler = TestPipeline.CreateHandler(); + + var boom = new InvalidOperationException("boom"); + _ = handler + .UseRequestMetrics() + .Use((_, _) => throw boom); + + // ThrowOnException defaults to true, so the original exception surfaces to the caller unchanged. + var thrown = await Assert.ThrowsAsync( + () => handler.InvokeAsync(new TestRequest(), TestContext.Current.CancellationToken)); + + Assert.Same(boom, thrown); + _ = Assert.Single(metrics.Measurements, m => m.Instrument == ErrorInstrument); + var duration = Assert.Single(metrics.Measurements, m => m.Instrument == DurationInstrument); + Assert.Equal(false, duration.Tags["success"]); + } + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "fluent UseRequestMetrics/Use return the same handler instance; the using var owns disposal")] + public async Task RecordCustomMetricsReceivesSuccessTrueAsync() + { + using var handler = TestPipeline.CreateHandler(); + + bool? observed = null; + _ = handler + .UseRequestMetrics(options => options.RecordCustomMetrics = (_, success) => observed = success) + .Use(TestPipeline.Succeeds); + + _ = await handler.InvokeAsync(new TestRequest(), TestContext.Current.CancellationToken); + + Assert.True(observed); + } + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "fluent UseRequestMetrics/Use return the same handler instance; the using var owns disposal")] + public async Task RecordCustomMetricsReceivesSuccessFalseOnSwallowedExceptionAsync() + { + using var metrics = new MeterCollector(PlumberDiagnostics.MeterName); + using var handler = TestPipeline.CreateHandler(); + + bool? observed = null; + _ = handler + .UseRequestMetrics(options => + { + options.ThrowOnException = false; + options.RecordCustomMetrics = (_, success) => observed = success; + }) + .Use((_, _) => throw new InvalidOperationException("boom")); + + _ = await handler.InvokeAsync(new TestRequest(), TestContext.Current.CancellationToken); + + Assert.False(observed); + _ = Assert.Single(metrics.Measurements, m => m.Instrument == ErrorInstrument); + } + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "fluent UseRequestMetrics/Use return the same handler instance; the using var owns disposal")] + public async Task AddDefaultMetricsFalseOmitsDefaultsButRunsCustomOnSuccessAsync() + { + using var metrics = new MeterCollector(PlumberDiagnostics.MeterName); + using var handler = TestPipeline.CreateHandler(); + + var customRan = false; + _ = handler + .UseRequestMetrics(options => + { + options.AddDefaultMetrics = false; + options.RecordCustomMetrics = (_, _) => customRan = true; + }) + .Use(TestPipeline.Succeeds); + + _ = await handler.InvokeAsync(new TestRequest(), TestContext.Current.CancellationToken); + + Assert.True(customRan); + Assert.Empty(metrics.Measurements); + } + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "fluent UseRequestMetrics/Use return the same handler instance; the using var owns disposal")] + public async Task AddDefaultMetricsFalseOmitsDefaultsOnExceptionAsync() + { + using var metrics = new MeterCollector(PlumberDiagnostics.MeterName); + using var handler = TestPipeline.CreateHandler(); + + _ = handler + .UseRequestMetrics(options => + { + options.AddDefaultMetrics = false; + options.ThrowOnException = false; + }) + .Use((_, _) => throw new InvalidOperationException("boom")); + + _ = await handler.InvokeAsync(new TestRequest(), TestContext.Current.CancellationToken); + + Assert.Empty(metrics.Measurements); + } + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "fluent UseRequestMetrics/Use return the same handler instance; the using var owns disposal")] + public async Task ParameterlessMetricsResolvesOptionsFromDiAsync() + { + using var metrics = new MeterCollector(PlumberDiagnostics.MeterName); + using var handler = TestPipeline.CreateHandler(services => + services.AddPlumberDiagnostics( + configureMetrics: options => options.AddDefaultMetrics = true)); + + _ = handler + .UseRequestMetrics() + .Use(TestPipeline.Succeeds); + + _ = await handler.InvokeAsync(new TestRequest(), TestContext.Current.CancellationToken); + + _ = Assert.Single(metrics.Measurements, m => m.Instrument == CountInstrument); + } + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "fluent UseRequestMetrics/Use return the same handler instance; the using var owns disposal")] + public async Task CancellationCountsAsAttemptButNotErrorOrDurationAsync() + { + using var metrics = new MeterCollector(PlumberDiagnostics.MeterName); + using var handler = TestPipeline.CreateHandler(); + + _ = handler + .UseRequestMetrics() + .Use((_, _) => throw new OperationCanceledException()); + + // ThrowOnException defaults true, so the cancellation propagates unchanged. + _ = await Assert.ThrowsAsync( + () => handler.InvokeAsync(new TestRequest(), TestContext.Current.CancellationToken)); + + // Counted as an attempt, but kept out of errors and the duration histogram. + _ = Assert.Single(metrics.Measurements, m => m.Instrument == CountInstrument); + Assert.DoesNotContain(metrics.Measurements, m => m.Instrument == ErrorInstrument); + Assert.DoesNotContain(metrics.Measurements, m => m.Instrument == DurationInstrument); + } + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "fluent UseRequestMetrics/Use return the same handler instance; the using var owns disposal")] + public async Task CancellationWithThrowOnExceptionFalseSwallowsAndSkipsCustomMetricsAsync() + { + using var metrics = new MeterCollector(PlumberDiagnostics.MeterName); + using var handler = TestPipeline.CreateHandler(); + + bool? observed = null; + _ = handler + .UseRequestMetrics(options => + { + options.ThrowOnException = false; + options.RecordCustomMetrics = (_, success) => observed = success; + }) + .Use((_, _) => throw new OperationCanceledException()); + + var response = await handler.InvokeAsync(new TestRequest(), TestContext.Current.CancellationToken); + + // The custom hook fires only on resolved outcomes, so a cancellation leaves it untouched. + Assert.Null(response); + Assert.Null(observed); + _ = Assert.Single(metrics.Measurements, m => m.Instrument == CountInstrument); + Assert.DoesNotContain(metrics.Measurements, m => m.Instrument == ErrorInstrument); + Assert.DoesNotContain(metrics.Measurements, m => m.Instrument == DurationInstrument); + } + + [Fact] + public async Task MiddlewareGuardsAgainstNullArgumentsAsync() + { + var options = Options.Create(new RequestMetricsOptions()); + RequestMiddleware next = _ => Task.CompletedTask; + + _ = Assert.Throws(() => new RequestMetricsMiddleware(null!, options)); + _ = Assert.Throws(() => new RequestMetricsMiddleware(next, null!)); + + var middleware = new RequestMetricsMiddleware(next, options); + _ = await Assert.ThrowsAsync(() => middleware.InvokeAsync(null!)); + } +} diff --git a/tests/Plumber.Diagnostics.Tests/RequestTracingTests.cs b/tests/Plumber.Diagnostics.Tests/RequestTracingTests.cs new file mode 100644 index 0000000..4109d2e --- /dev/null +++ b/tests/Plumber.Diagnostics.Tests/RequestTracingTests.cs @@ -0,0 +1,279 @@ +using Microsoft.Extensions.Options; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; + +namespace Plumber.Diagnostics.Tests; + +public sealed class RequestTracingTests +{ + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "fluent UseRequestTracing/Use return the same handler instance; the using var owns disposal")] + public async Task TracksSuccessfulRequestWithDefaultAttributesAsync() + { + using var spans = new ActivityCollector(PlumberDiagnostics.ActivitySourceName); + using var handler = TestPipeline.CreateHandler(); + + _ = handler + .UseRequestTracing(options => options.OperationName = "TestOperation") + .Use(TestPipeline.Succeeds); + + var response = await handler.InvokeAsync(new TestRequest { Value = "hi" }, TestContext.Current.CancellationToken); + + Assert.True(response!.Success); + var activity = Assert.Single(spans.Activities); + Assert.Equal("TestOperation", activity.OperationName); + Assert.Equal(ActivityStatusCode.Ok, activity.Status); + Assert.NotNull(activity.GetTagItem("request.id")); + Assert.Equal(nameof(TestRequest), activity.GetTagItem("request.type")); + Assert.NotNull(activity.GetTagItem("request.elapsed_ms")); + Assert.Equal(nameof(TestResponse), activity.GetTagItem("response.type")); + } + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "fluent UseRequestTracing/Use return the same handler instance; the using var owns disposal")] + public async Task EnrichSpanAddsCustomTagAsync() + { + using var spans = new ActivityCollector(PlumberDiagnostics.ActivitySourceName); + using var handler = TestPipeline.CreateHandler(); + + _ = handler + .UseRequestTracing(options => options.EnrichSpan = (activity, context) => + activity.SetTag("request.value", context.Request.Value)) + .Use(TestPipeline.Succeeds); + + _ = await handler.InvokeAsync(new TestRequest { Value = "enriched" }, TestContext.Current.CancellationToken); + + var activity = Assert.Single(spans.Activities); + Assert.Equal("enriched", activity.GetTagItem("request.value")); + } + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "fluent UseRequestTracing/Use return the same handler instance; the using var owns disposal")] + public async Task SpanKindIsAppliedAsync() + { + using var spans = new ActivityCollector(PlumberDiagnostics.ActivitySourceName); + using var handler = TestPipeline.CreateHandler(); + + _ = handler + .UseRequestTracing(options => options.SpanKind = ActivityKind.Server) + .Use(TestPipeline.Succeeds); + + _ = await handler.InvokeAsync(new TestRequest(), TestContext.Current.CancellationToken); + + Assert.Equal(ActivityKind.Server, Assert.Single(spans.Activities).Kind); + } + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "fluent UseRequestTracing/Use return the same handler instance; the using var owns disposal")] + public async Task DownstreamExceptionIsRecordedAndRethrownAsync() + { + using var spans = new ActivityCollector(PlumberDiagnostics.ActivitySourceName); + using var handler = TestPipeline.CreateHandler(); + + var boom = new InvalidOperationException("boom"); + _ = handler + .UseRequestTracing() + .Use((_, _) => throw boom); + + // ThrowOnException defaults to true, so the original exception surfaces to the caller unchanged. + var thrown = await Assert.ThrowsAsync( + () => handler.InvokeAsync(new TestRequest(), TestContext.Current.CancellationToken)); + + Assert.Same(boom, thrown); + var activity = Assert.Single(spans.Activities); + Assert.Equal(ActivityStatusCode.Error, activity.Status); + Assert.Equal("boom", activity.StatusDescription); + Assert.Contains(activity.Events, e => e.Name == "exception"); + Assert.NotNull(activity.GetTagItem("request.elapsed_ms")); + } + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "fluent UseRequestTracing/Use return the same handler instance; the using var owns disposal")] + public async Task ThrowOnExceptionFalseSwallowsAndRecordsErrorAsync() + { + using var spans = new ActivityCollector(PlumberDiagnostics.ActivitySourceName); + using var handler = TestPipeline.CreateHandler(); + + _ = handler + .UseRequestTracing(options => + { + options.ThrowOnException = false; + options.EnrichSpan = (activity, _) => activity.SetTag("enriched", true); + }) + .Use((_, _) => throw new InvalidOperationException("boom")); + + var response = await handler.InvokeAsync(new TestRequest(), TestContext.Current.CancellationToken); + + Assert.Null(response); + var activity = Assert.Single(spans.Activities); + Assert.Equal(ActivityStatusCode.Error, activity.Status); + // EnrichSpan runs on the failure path too. + Assert.Equal(true, activity.GetTagItem("enriched")); + } + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "fluent UseRequestTracing/Use return the same handler instance; the using var owns disposal")] + public async Task RecordExceptionFalseLeavesStatusUnsetAndAddsNoEventAsync() + { + using var spans = new ActivityCollector(PlumberDiagnostics.ActivitySourceName); + using var handler = TestPipeline.CreateHandler(); + + _ = handler + .UseRequestTracing(options => + { + options.RecordException = false; + options.ThrowOnException = false; + }) + .Use((_, _) => throw new InvalidOperationException("boom")); + + _ = await handler.InvokeAsync(new TestRequest(), TestContext.Current.CancellationToken); + + var activity = Assert.Single(spans.Activities); + Assert.Equal(ActivityStatusCode.Unset, activity.Status); + Assert.DoesNotContain(activity.Events, e => e.Name == "exception"); + } + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "fluent UseRequestTracing/Use return the same handler instance; the using var owns disposal")] + public async Task AddDefaultAttributesFalseOmitsDefaultTagsAsync() + { + using var spans = new ActivityCollector(PlumberDiagnostics.ActivitySourceName); + using var handler = TestPipeline.CreateHandler(); + + _ = handler + .UseRequestTracing(options => options.AddDefaultAttributes = false) + .Use(TestPipeline.Succeeds); + + _ = await handler.InvokeAsync(new TestRequest(), TestContext.Current.CancellationToken); + + var activity = Assert.Single(spans.Activities); + Assert.Null(activity.GetTagItem("request.id")); + Assert.Null(activity.GetTagItem("request.type")); + Assert.Null(activity.GetTagItem("response.type")); + Assert.Equal(ActivityStatusCode.Ok, activity.Status); + } + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "fluent UseRequestTracing/Use return the same handler instance; the using var owns disposal")] + public async Task NoListenerShortCircuitsButStillProcessesRequestAsync() + { + // No ActivityCollector is subscribed, so ActivitySource.StartActivity returns null and the + // middleware must fall through to the next component without touching a span. + using var handler = TestPipeline.CreateHandler(); + + _ = handler + .UseRequestTracing() + .Use(TestPipeline.Succeeds); + + var response = await handler.InvokeAsync(new TestRequest(), TestContext.Current.CancellationToken); + + Assert.True(response!.Success); + } + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "fluent UseRequestTracing/Use return the same handler instance; the using var owns disposal")] + public async Task ParameterlessTracingResolvesOptionsFromDiAsync() + { + using var spans = new ActivityCollector(PlumberDiagnostics.ActivitySourceName); + using var handler = TestPipeline.CreateHandler(services => + services.AddPlumberDiagnostics( + configureTracing: options => options.OperationName = "FromDi")); + + // The parameterless overload resolves IOptions from DI, so the operation name configured via + // AddPlumberDiagnostics must flow through to the span. + _ = handler + .UseRequestTracing() + .Use(TestPipeline.Succeeds); + + _ = await handler.InvokeAsync(new TestRequest(), TestContext.Current.CancellationToken); + + Assert.Equal("FromDi", Assert.Single(spans.Activities).OperationName); + } + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "fluent UseRequestTracing/Use return the same handler instance; the using var owns disposal")] + public async Task CancellationLeavesSpanUnsetAndRethrowsAsync() + { + using var spans = new ActivityCollector(PlumberDiagnostics.ActivitySourceName); + using var handler = TestPipeline.CreateHandler(); + + _ = handler + .UseRequestTracing() + .Use((_, _) => throw new OperationCanceledException()); + + // Cancellation is not a defect: it propagates (ThrowOnException defaults true), but the span stays Unset. + _ = await Assert.ThrowsAsync( + () => handler.InvokeAsync(new TestRequest(), TestContext.Current.CancellationToken)); + + var activity = Assert.Single(spans.Activities); + Assert.Equal(ActivityStatusCode.Unset, activity.Status); + Assert.DoesNotContain(activity.Events, e => e.Name == "exception"); + } + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "fluent UseRequestTracing/Use return the same handler instance; the using var owns disposal")] + public async Task CancellationWithThrowOnExceptionFalseSwallowsAndLeavesSpanUnsetAsync() + { + using var spans = new ActivityCollector(PlumberDiagnostics.ActivitySourceName); + using var handler = TestPipeline.CreateHandler(); + + _ = handler + .UseRequestTracing(options => + { + options.ThrowOnException = false; + options.EnrichSpan = (activity, _) => activity.SetTag("enriched", true); + }) + .Use((_, _) => throw new OperationCanceledException()); + + var response = await handler.InvokeAsync(new TestRequest(), TestContext.Current.CancellationToken); + + Assert.Null(response); + var activity = Assert.Single(spans.Activities); + Assert.Equal(ActivityStatusCode.Unset, activity.Status); + // EnrichSpan runs on cancellation too, even though the span stays Unset. + Assert.Equal(true, activity.GetTagItem("enriched")); + } + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "fluent UseRequestTracing/Use return the same handler instance; the using var owns disposal")] + public async Task NullResponseRecordsResponseTypeNullAsync() + { + using var spans = new ActivityCollector(PlumberDiagnostics.ActivitySourceName); + using var handler = TestPipeline.CreateHandler(); + + // A middleware that completes without assigning a response leaves context.Response null. + _ = handler + .UseRequestTracing() + .Use((_, _) => Task.CompletedTask); + + var response = await handler.InvokeAsync(new TestRequest(), TestContext.Current.CancellationToken); + + Assert.Null(response); + Assert.Equal("null", Assert.Single(spans.Activities).GetTagItem("response.type")); + } + + [Fact] + public async Task MiddlewareGuardsAgainstNullArgumentsAsync() + { + var options = Options.Create(new RequestTracingOptions()); + RequestMiddleware next = _ => Task.CompletedTask; + + _ = Assert.Throws(() => new RequestTracingMiddleware(null!, options)); + _ = Assert.Throws(() => new RequestTracingMiddleware(next, null!)); + + var middleware = new RequestTracingMiddleware(next, options); + _ = await Assert.ThrowsAsync(() => middleware.InvokeAsync(null!)); + } +} diff --git a/tests/Plumber.Diagnostics.Tests/TelemetryCollectors.cs b/tests/Plumber.Diagnostics.Tests/TelemetryCollectors.cs new file mode 100644 index 0000000..08a31d4 --- /dev/null +++ b/tests/Plumber.Diagnostics.Tests/TelemetryCollectors.cs @@ -0,0 +1,81 @@ +using System.Collections.ObjectModel; +using System.Diagnostics; +using System.Diagnostics.Metrics; + +namespace Plumber.Diagnostics.Tests; + +/// +/// A captured metric measurement: the instrument name, the recorded value (long counters are widened to +/// double for uniform assertions), and the tags attached to the measurement. +/// +internal sealed record CapturedMeasurement(string Instrument, double Value, IReadOnlyDictionary Tags); + +/// +/// Subscribes a BCL to a single by name and +/// collects every stopped activity, so tests can assert on real spans without the OpenTelemetry SDK. +/// +internal sealed class ActivityCollector : IDisposable +{ + private readonly ActivityListener listener; + + public Collection Activities { get; } = []; + + public ActivityCollector(string sourceName) + { + listener = new ActivityListener + { + ShouldListenTo = source => source.Name == sourceName, + Sample = static (ref _) => ActivitySamplingResult.AllDataAndRecorded, + ActivityStopped = Activities.Add, + }; + + ActivitySource.AddActivityListener(listener); + } + + public void Dispose() => listener.Dispose(); +} + +/// +/// Subscribes a BCL to a single by name and collects every +/// measurement, so tests can assert on real metrics without the OpenTelemetry SDK. +/// +internal sealed class MeterCollector : IDisposable +{ + private readonly MeterListener listener; + + public Collection Measurements { get; } = []; + + public MeterCollector(string meterName) + { + listener = new MeterListener + { + InstrumentPublished = (instrument, l) => + { + if (instrument.Meter.Name == meterName) + { + l.EnableMeasurementEvents(instrument); + } + }, + }; + + listener.SetMeasurementEventCallback( + (instrument, measurement, tags, _) => Measurements.Add(new CapturedMeasurement(instrument.Name, measurement, ToDictionary(tags)))); + listener.SetMeasurementEventCallback( + (instrument, measurement, tags, _) => Measurements.Add(new CapturedMeasurement(instrument.Name, measurement, ToDictionary(tags)))); + + listener.Start(); + } + + public void Dispose() => listener.Dispose(); + + private static Dictionary ToDictionary(ReadOnlySpan> tags) + { + var dictionary = new Dictionary(tags.Length); + foreach (var tag in tags) + { + dictionary[tag.Key] = tag.Value; + } + + return dictionary; + } +} diff --git a/tests/Plumber.Diagnostics.Tests/TestPipeline.cs b/tests/Plumber.Diagnostics.Tests/TestPipeline.cs new file mode 100644 index 0000000..2318737 --- /dev/null +++ b/tests/Plumber.Diagnostics.Tests/TestPipeline.cs @@ -0,0 +1,41 @@ +using Microsoft.Extensions.DependencyInjection; + +namespace Plumber.Diagnostics.Tests; + +internal sealed class TestRequest +{ + public string Value { get; init; } = string.Empty; +} + +internal sealed class TestResponse +{ + public bool Success { get; init; } +} + +internal static class TestPipeline +{ + /// + /// A terminal middleware factory that assigns a successful response and completes the pipeline. + /// + public static Func, RequestMiddleware> Succeeds { get; } = + next => context => + { + context.Response = new TestResponse { Success = true }; + return next(context); + }; + + /// + /// Builds a request handler for the test request/response pair. The default options infrastructure is + /// always registered so the parameterless UseRequestTracing, UseRequestMetrics, and + /// UseRequestDiagnostics overloads can resolve their IOptions dependency; tests can layer + /// further registration (e.g. configured AddPlumberDiagnostics) through . + /// + public static RequestHandler CreateHandler(Action? configureServices = null) => + RequestHandlerBuilder.Create() + .ConfigureServices((services, configuration) => + { + _ = services.AddPlumberDiagnostics(); + configureServices?.Invoke(services); + }) + .Build(); +} diff --git a/tests/Plumber.Serilog.Extensions.Tests/Plumber.Serilog.Extensions.Tests.csproj b/tests/Plumber.Serilog.Extensions.Tests/Plumber.Serilog.Extensions.Tests.csproj index ae16a2c..16e8f12 100644 --- a/tests/Plumber.Serilog.Extensions.Tests/Plumber.Serilog.Extensions.Tests.csproj +++ b/tests/Plumber.Serilog.Extensions.Tests/Plumber.Serilog.Extensions.Tests.csproj @@ -2,10 +2,6 @@ [Plumber.Serilog.Extensions]* - - 100,88,100 $(NoWarn);CA1515;CA1812 diff --git a/tests/Plumber.Serilog.Extensions.Tests/RequestHandlerSerilogExtensionsTests.cs b/tests/Plumber.Serilog.Extensions.Tests/RequestHandlerSerilogExtensionsTests.cs index 72aeb1d..e299bf2 100644 --- a/tests/Plumber.Serilog.Extensions.Tests/RequestHandlerSerilogExtensionsTests.cs +++ b/tests/Plumber.Serilog.Extensions.Tests/RequestHandlerSerilogExtensionsTests.cs @@ -1,5 +1,7 @@ +using Microsoft.Extensions.DependencyInjection; using Serilog.Events; using Serilog.Exceptions; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Globalization; @@ -178,4 +180,99 @@ public async Task AddSerilogRequestLoggingConfiguresOptionsThroughDiAsync() var e = sink.Events.First(); Assert.Equal("DONE {RequestId}", e.MessageTemplate.Text); } + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "fluent UseSerilogRequestLogging/Use return the same handler instance; the using var owns disposal")] + public async Task CompletionEventCarriesAmbientActivityTraceContextAsync() + { + // When a request runs inside an active span (the realistic case with Plumber.Diagnostics also wired up), + // the completion event must carry that span's trace and span ids so logs correlate with the trace. + using var source = new ActivitySource("Plumber.Serilog.Extensions.Tests.TraceCorrelation"); + using var listener = new ActivityListener + { + ShouldListenTo = s => s.Name == source.Name, + Sample = static (ref _) => ActivitySamplingResult.AllDataAndRecorded, + }; + ActivitySource.AddActivityListener(listener); + + var sink = new TestSink(); + using var handler = RequestHandlerBuilder.Create() + .ConfigureServices((services, _) => services + .AddSerilogRequestLogging(logger => logger.MinimumLevel.Debug().WriteTo.Sink(sink))) + .Build(); + + _ = handler + .UseSerilogRequestLogging() + .Use(); + + using var activity = source.StartActivity("request"); + Assert.NotNull(activity); // the listener samples AllDataAndRecorded, so a recording activity must exist + + _ = await handler.InvokeAsync("Hello", TestContext.Current.CancellationToken); + + var e = sink.Events.First(); + Assert.Equal(activity.TraceId, Assert.NotNull(e.TraceId)); + Assert.Equal(activity.SpanId, Assert.NotNull(e.SpanId)); + } + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "the completion event must NOT borrow trace context from an unrelated span; the using var owns disposal")] + public async Task CompletionEventOmitsTraceContextWhenNoActivityIsActiveAsync() + { + // No ambient activity: the completion event must leave trace/span ids unset rather than fabricate them. + Assert.Null(Activity.Current); + + var sink = new TestSink(); + using var handler = RequestHandlerBuilder.Create() + .ConfigureServices((services, _) => services + .AddSerilogRequestLogging(logger => logger.MinimumLevel.Debug().WriteTo.Sink(sink))) + .Build(); + + _ = handler + .UseSerilogRequestLogging() + .Use(); + + _ = await handler.InvokeAsync("Hello", TestContext.Current.CancellationToken); + + var e = sink.Events.First(); + Assert.Null(e.TraceId); + Assert.Null(e.SpanId); + } + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP005:Return type should indicate that the value should be disposed", + Justification = "the guarded extension throws ArgumentNullException before constructing anything; no disposable is created")] + public void UseSerilogRequestLoggingGuardsAgainstNullArguments() + { + RequestHandler handler = null!; + + _ = Assert.Throws(() => handler.UseSerilogRequestLogging()); + _ = Assert.Throws(() => handler.UseSerilogRequestLogging(_ => { })); + } + + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP004:Don't ignore created IDisposable", + Justification = "the using var owns disposal; the guarded call throws before returning a handler")] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP005:Return type should indicate that the value should be disposed", + Justification = "the guarded extension throws ArgumentNullException before constructing anything; no disposable is created")] + public void UseSerilogRequestLoggingGuardsAgainstNullConfiguration() + { + using var handler = RequestHandlerBuilder.Create().Build(); + + _ = Assert.Throws(() => handler.UseSerilogRequestLogging(null!)); + } + + [Fact] + public void AddSerilogRequestLoggingGuardsAgainstNullArguments() + { + IServiceCollection nullServices = null!; + _ = Assert.Throws( + () => nullServices.AddSerilogRequestLogging(_ => { })); + + var services = new ServiceCollection(); + _ = Assert.Throws( + () => services.AddSerilogRequestLogging(null!)); + } } diff --git a/tests/Plumber.Tests/PlumberTests.cs b/tests/Plumber.Tests/PlumberTests.cs index 25560e0..8389e90 100644 --- a/tests/Plumber.Tests/PlumberTests.cs +++ b/tests/Plumber.Tests/PlumberTests.cs @@ -1,5 +1,6 @@ using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Time.Testing; using Plumber.Tests.Middleware; using System.Diagnostics.CodeAnalysis; @@ -54,6 +55,33 @@ public void BuildDisposesProviderWhenTimeProviderResolutionThrows() Assert.True(created.Disposed, "the owned provider must be disposed when TimeProvider resolution throws in the ctor"); } + [Fact] + [SuppressMessage("IDisposableAnalyzers.Correctness", "IDISP013:Await in using", Justification = "IDisposable analyzer is misjudging the context")] + public async Task ConfigureLoggingCallbacksRunAndRegisterLoggingAsync() + { + var callbackRan = false; + ILoggerFactory? resolved = null; + + using var handler = RequestHandlerBuilder.Create() + .ConfigureLogging(logging => + { + callbackRan = true; + _ = logging.SetMinimumLevel(LogLevel.Warning); + }) + .Build() + .Use((context, next) => + { + // logging infrastructure is only registered when a ConfigureLogging callback is present + resolved = context.Services.GetRequiredService(); + return next(context); + }); + + _ = await handler.InvokeAsync("request", TestContext.Current.CancellationToken); + + Assert.True(callbackRan, "the ConfigureLogging callback must run during Build"); + Assert.NotNull(resolved); + } + private sealed class TrackingDisposable : IDisposable { public bool Disposed { get; private set; } @@ -208,6 +236,31 @@ public void TryGetValueFalseOnTypeMismatchDoesNotThrow() Assert.False(context.TryGetValue("key", out var value)); Assert.Null(value); } + + [Fact] + public void DataReturnsTheSameDictionaryAcrossAccesses() + { + using var services = new ServiceCollection().BuildServiceProvider(); + var context = new RequestContext("request", Ulid.NewUlid(), TimeProvider.System, services, CancellationToken.None); + + // first access creates the backing dictionary; later accesses must return that same instance so + // middleware can accumulate shared per-request state rather than writing into a fresh map each time + var first = context.Data; + first["key"] = "value"; + var second = context.Data; + + Assert.Same(first, second); + Assert.Equal("value", second["key"]); + } + + [Fact] + public void ConstructorThrowsWhenServicesIsNull() + { + var ex = Assert.Throws( + () => _ = new RequestContext("request", Ulid.NewUlid(), TimeProvider.System, null!, CancellationToken.None)); + + Assert.Equal("services", ex.ParamName); + } } public sealed class PlumberTests From d6a412656a1faf218fdecb014ce88ab294e0ebbd Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 25 Jun 2026 17:20:44 -0400 Subject: [PATCH 2/4] Bump the nuget-minor-and-patch group with 1 update (#10) Bumps Microsoft.NET.Test.Sdk from 18.6.0 to 18.7.0 --- updated-dependencies: - dependency-name: Microsoft.NET.Test.Sdk dependency-version: 18.7.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: nuget-minor-and-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Directory.Packages.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index f588f50..fee2e8f 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -39,7 +39,7 @@ - + From cf0dee8f4bd705cccb9091953a9dc8406a395fe4 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 25 Jun 2026 17:21:05 -0400 Subject: [PATCH 3/4] Bump actions/checkout from 6 to 7 in the actions group (#9) Bumps the actions group with 1 update: [actions/checkout](https://github.com/actions/checkout). Updates `actions/checkout` from 6 to 7 - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](https://github.com/actions/checkout/compare/v6...v7) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major dependency-group: actions ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/dotnet.publish.yml | 4 ++-- .github/workflows/dotnet.tests.yml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/dotnet.publish.yml b/.github/workflows/dotnet.publish.yml index e3960a1..e785d70 100644 --- a/.github/workflows/dotnet.publish.yml +++ b/.github/workflows/dotnet.publish.yml @@ -21,7 +21,7 @@ jobs: test: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - uses: ./.github/actions/setup-dotnet - name: Restore dependencies @@ -39,7 +39,7 @@ jobs: permissions: contents: read steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - uses: ./.github/actions/setup-dotnet - name: Resolve package version diff --git a/.github/workflows/dotnet.tests.yml b/.github/workflows/dotnet.tests.yml index f556c02..50a79ee 100644 --- a/.github/workflows/dotnet.tests.yml +++ b/.github/workflows/dotnet.tests.yml @@ -19,7 +19,7 @@ jobs: format: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - uses: ./.github/actions/setup-dotnet - name: Verify formatting @@ -31,7 +31,7 @@ jobs: matrix: configuration: [ Debug, Release ] steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - uses: ./.github/actions/setup-dotnet - name: Restore dependencies From 5c30e9874cbef72c84dbdd6a85de21aa0d7a8e4b Mon Sep 17 00:00:00 2001 From: Mark Lauter <10211200+marklauter@users.noreply.github.com> Date: Thu, 25 Jun 2026 17:23:01 -0400 Subject: [PATCH 4/4] docs: refresh README migration pointer and MS-style prose (#11) - Point the migration note at all earlier versions and note v5 is non-breaking. - Rewrite figurative lines into plain Microsoft-style prose; keep the MSL Armory branding. Co-authored-by: Claude Opus 4.8 (1M context) --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index edf8bbd..7fa4d56 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ *Another weapon from the MSL Armory* -Middleware pipelines for host-free .NET projects. The same shape ASP.NET Core gives web apps — request, response, a chain of steps with DI and configuration — for console apps, AWS Lambdas, Azure Functions, queue consumers, file processors, and anything else that lives outside a host. +Middleware pipelines for host-free .NET projects. Plumber brings the ASP.NET Core middleware model — a request, a response, and a chain of steps with dependency injection and configuration — to .NET projects that run without a host: console apps, AWS Lambdas, Azure Functions, queue consumers, file processors, and similar workloads. The [wiki](https://github.com/marklauter/plumber/wiki) is the full documentation: concepts, a tutorial, per-type reference, and deployment recipes. @@ -48,14 +48,14 @@ var greeting = await handler.InvokeAsync("World"); Console.WriteLine(greeting); // Hello, World! ``` -That's the whole shape: a builder, a built handler, one or more middleware, and an `InvokeAsync` call. Each invocation gets its own DI scope and cancellation token. +A Plumber application has four pieces: a builder, the handler it builds, one or more middleware, and an `InvokeAsync` call. Each invocation gets its own dependency injection scope and cancellation token. ## Where to go next - **New to middleware pipelines?** Start with [Concepts](https://github.com/marklauter/plumber/wiki/Concepts), then the [Tutorial](https://github.com/marklauter/plumber/wiki/Tutorial). - **Know the shape already?** Jump into [Building a pipeline](https://github.com/marklauter/plumber/wiki/Building-A-Pipeline), [Middleware](https://github.com/marklauter/plumber/wiki/Middleware), and [Request lifecycle](https://github.com/marklauter/plumber/wiki/Request-Lifecycle). - **Looking for a specific scenario?** Browse the recipes — AWS Lambda, Azure Functions, queue consumers, webhooks, and more — from the [wiki home](https://github.com/marklauter/plumber/wiki). -- **Migrating from v2 or v3?** See [Migration](https://github.com/marklauter/plumber/wiki/Migration). +- **Migrating from an earlier version?** See [Migration](https://github.com/marklauter/plumber/wiki/Migration). v5 makes no breaking changes; v4 and v3 are covered with before-and-after examples. --- [Repository](https://github.com/marklauter/plumber) · [NuGet — Pipeline](https://www.nuget.org/packages/MSL.Plumber.Pipeline/) · [NuGet — Testing](https://www.nuget.org/packages/MSL.Plumber.Pipeline.Testing/) · [MIT License](https://github.com/marklauter/plumber/blob/main/LICENSE) · [Report an issue](https://github.com/marklauter/plumber/issues)