feat: NAMS Phase 9 -- observability (spans, metrics, health checks) (#138)

Instruments the NAMS backend with spans/metrics matching the engineering plan's own naming
(memory.backend.operations/.duration/.failures/.retries/.rate_limited/.unknown_write_outcomes/
.context_items/.context_truncated), using BCL diagnostics directly to keep AgentMemory.Nams at
zero sibling references (B9). Adds a framework-agnostic INamsHealthCheck distinguishing Unhealthy
from Degraded (rate-limited), backed by a new INamsClient.ListEntitiesAsync -- the one safe,
side-effect-free, no-precondition read this client can use as a connectivity probe. Reviewed every
log call site in AgentMemory.Nams/AgentMemory.AgentFramework.Nams for secret/PII/content leakage:
clean by design, no changes needed.

Author: joslat

docs/reviews/NAMS_Phase9_Observability_PlanningAndImplementationPlan.md

@@ -0,0 +1,92 @@
+# NAMS Phase 9 — Observability — Planning and Implementation Plan
+
+**Prepared:** 2026-07-17
+**Branch:** `nams/phase9-observability`
+**Purpose:** Executes engineering plan Phase 9 ("Observability and operations"). Per §12's recommended
+delivery sequence, this comes right after "Live sandbox proof" (today's earlier live validation +
+`NamsAgent` sample) and before the optional Phase 7 convergence.
+
+## 1. Design decisions
+
+- **BCL diagnostics directly, no `AgentMemory.Observability` dependency.** That package decorates the
+  direct backend's Core interfaces (`IMemoryService`, extractors) via `TryAddScoped` wrapping -- none of
+  which exist on the NAMS path. `AgentMemory.Nams` (B9) stays at zero sibling references by using
+  `System.Diagnostics.ActivitySource`/`System.Diagnostics.Metrics.Meter` directly -- pure BCL, not an
+  external package, so B9's boundary rule is untouched.
+- **Spans/metrics live at two layers.** `Neo4jNamsClientAdapter` (the actual HTTP-call boundary) emits
+  `memory.backend.operations`/`.duration`/`.failures`/`.retries`/`.rate_limited`, one per real REST call
+  (`resolve_conversation`, `get_context`, `store_turn`, `search_entities`, `list_entities`). `.context_items`/
+  `.context_truncated` are emitted from `NamsRecallService` (it alone knows about budget truncation) and
+  `.unknown_write_outcomes` from `NamsPersistenceService` (it alone classifies that outcome). All metric
+  tags are the plan's own low-cardinality set (`backend`, `operation`, `status`, `failure_kind`) -- never an
+  owner/workspace/conversation ID, memory text, or a prompt.
+- **Cancellation is not counted as a failure**, per the plan's own test list -- `Neo4jNamsClientAdapter`
+  records a distinct `status=cancelled` on the operations counter and never touches the failures counter for
+  caller-requested cancellation.
+- **A new `INamsClient.ListEntitiesAsync`** (`GET /v1/entities`, confirmed live) was added specifically to
+  give the health check a safe, side-effect-free, no-precondition probe -- `SearchEntitiesAsync` requires a
+  non-empty query (confirmed live: an empty query returns 400) and `GetContextAsync` requires an existing
+  conversation, so neither works as a default connectivity probe.
+- **Health check is a small, framework-agnostic interface** (`INamsHealthCheck`/`NamsHealthCheckResult`/
+  `NamsHealthStatus`), not an ASP.NET Core `IHealthCheck` -- avoids a new package dependency for every
+  consumer; a host that wants the ASP.NET Core shape adapts this thin result itself. Distinguishes
+  `Unhealthy` (auth/network/timeout/unexpected) from `Degraded` (rate-limited) per the plan's own
+  requirement ("status distinction between unhealthy and degraded/rate-limited"). Never performs a
+  destructive write, by construction (only ever calls the new read-only `ListEntitiesAsync`).
+- **Retry counting via a callback, not a metrics dependency in `NamsRetryPolicy`.** `ExecuteAsync` gained an
+  optional `Action? onRetry` parameter, invoked once per actual retry -- keeps the retry policy itself free
+  of any metrics-type dependency; the adapter supplies the callback.
+
+## 2. Explicitly out of scope
+
+- **Data-governance verification** (conversation/user deletion behavior, export, region/classification
+  restrictions) -- the plan's own list requires either live testing against real deletion behavior or an
+  organizational decision neither of which this PR can make alone.
+- **`docs/security/production-checklist.md`** -- reviewed, deliberately left untouched. It's entirely
+  direct-Neo4j-backend-scoped (threat-model TT-01 through TT-11) and NAMS isn't released yet (no preview,
+  Phase 12 not started) -- grafting a half-formed NAMS item onto it now would be premature.
+- **Phase 8 (MCP tools)** and further **Phase 10 test-matrix expansion** -- separate phases, next in this
+  session's queue.
+
+## 3. Log review (plan's own checklist item)
+
+Reviewed every `_logger.Log*` call site in `AgentMemory.Nams` and `AgentMemory.AgentFramework.Nams`
+(`grep` across both packages). None log an API key, recalled memory content, or a raw message body --
+only conversation/session/application IDs and static messages. `NamsClientExceptionMapper.Redact()` already
+scrubs the configured API key from any exception message before it can reach a log call (a Phase 2
+guarantee, unchanged here). No changes needed -- clean by design, not clean by omission.
+
+## 4. New/changed files
+
+- `src/AgentMemory.Nams/Observability/` (new folder): `NamsActivitySource.cs`, `NamsMetrics.cs`,
+  `NamsMetricTags.cs`, `NamsHealthStatus.cs`, `NamsHealthCheckResult.cs`, `INamsHealthCheck.cs`,
+  `NamsHealthCheck.cs`.
+- `src/AgentMemory.Nams/Client/INamsClient.cs` / `Neo4jNamsClientAdapter.cs` -- new `ListEntitiesAsync`;
+  `InvokeAsync` instrumented with spans/metrics per operation.
+- `src/AgentMemory.Nams/Client/NamsRetryPolicy.cs` -- new optional `onRetry` callback parameter.
+- `src/AgentMemory.Nams/Recall/NamsRecallService.cs` -- records `context_items`/`context_truncated`.
+- `src/AgentMemory.Nams/Persistence/NamsPersistenceService.cs` -- records `unknown_write_outcomes`.
+- `src/AgentMemory.Nams/NamsServiceCollectionExtensions.cs` -- registers `NamsMetrics`, `INamsHealthCheck`.
+- Test updates: every existing `INamsClient` test fake gained `ListEntitiesAsync`; every direct
+  `NamsRecallService`/`NamsPersistenceService`/`Neo4jNamsClientAdapter` construction site gained a
+  `NamsMetrics` argument.
+- New tests: `NamsMetricsTests.cs`, `NamsMetricTagsTests.cs`, `NamsHealthCheckTests.cs`,
+  `NamsObservabilityTests.cs` (+ `NamsObservabilityCollection.cs`, mirroring the existing
+  `AgentMemory.Tests.Unit.Observability.ObservabilityCollection` pattern for tests that attach a
+  process-wide `MeterListener`) -- verifies actual metric recording, not just that construction doesn't
+  throw: success/duration, failure+failure_kind, rate-limited, retries, cancellation-not-a-failure,
+  context items/truncation, unknown write outcomes. Plus 2 new `NamsRetryPolicy` tests for the `onRetry`
+  callback itself.
+
+## 5. Verification
+
+- `dotnet build AgentMemory.slnx -c Release` -- 0 warnings, 0 errors.
+- `dotnet test tests/AgentMemory.Tests.Unit` -- full suite green (189 Nams-filtered, ~30 net new).
+
+## 6. Definition of done
+
+- [x] Spans + metrics instrumented at the client/recall/persistence layers.
+- [x] Health check built and covered.
+- [x] Log review complete, no changes needed.
+- [x] Full unit suite green.
+- [ ] Self-reviewed, PR opened, CI green, merged to `main`.

src/AgentMemory.Nams/Client/INamsClient.cs

@@ -34,4 +34,13 @@ Task<IReadOnlyList<NamsEntity>> SearchEntitiesAsync(
         string? type,
         int limit,
         CancellationToken cancellationToken);
+
+    /// <summary>
+    /// Lists entities in the workspace with no query -- confirmed against the live NAMS REST API
+    /// (<c>GET /v1/entities</c>) as part of Phase 9. Unlike <see cref="SearchEntitiesAsync"/> (which requires
+    /// a non-empty query -- NAMS returns 400 for an empty one), this needs no precondition and no existing
+    /// conversation, making it the one safe, side-effect-free read this client can use for a connectivity
+    /// health probe.
+    /// </summary>
+    Task<IReadOnlyList<NamsEntity>> ListEntitiesAsync(int limit, CancellationToken cancellationToken);
 }

src/AgentMemory.Nams/Client/NamsRetryPolicy.cs

@@ -24,12 +24,16 @@ public NamsRetryPolicy(int maxAttempts, TimeSpan baseDelay, ILogger? logger = nu
     }
 
     /// <summary><paramref name="requestFactory"/> is invoked once per attempt -- an <see cref="HttpRequestMessage"/>
-    /// can only be sent once, so each retry needs a fresh instance.</summary>
+    /// can only be sent once, so each retry needs a fresh instance. <paramref name="onRetry"/> (Phase 9), if
+    /// supplied, is invoked once per actual retry -- after the transient response/exception is observed, right
+    /// before the retry delay -- so a caller can record a metric without this policy taking a hard dependency
+    /// on any metrics type.</summary>
     public async Task<HttpResponseMessage> ExecuteAsync(
         Func<HttpRequestMessage> requestFactory,
         HttpClient httpClient,
         bool isIdempotent,
-        CancellationToken cancellationToken)
+        CancellationToken cancellationToken,
+        Action? onRetry = null)
     {
         if (!isIdempotent)
             return await httpClient.SendAsync(requestFactory(), cancellationToken).ConfigureAwait(false);
@@ -47,6 +51,7 @@ public async Task<HttpResponseMessage> ExecuteAsync(
                     "Transient NAMS response {Status}; retry {Next}/{Max} after {Delay}.",
                     (int)response.StatusCode, attempt + 1, _maxAttempts, delay);
                 response.Dispose();
+                onRetry?.Invoke();
                 await Task.Delay(delay, cancellationToken).ConfigureAwait(false);
             }
             catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
@@ -58,6 +63,7 @@ public async Task<HttpResponseMessage> ExecuteAsync(
                 // Network failure, or a per-request timeout (a TaskCanceledException whose token was NOT the
                 // caller's -- caller cancellation is handled by the guarded catch above).
                 _logger?.LogDebug(ex, "Transient NAMS failure; retry {Next}/{Max}.", attempt + 1, _maxAttempts);
+                onRetry?.Invoke();
                 await Task.Delay(BackoffFor(attempt), cancellationToken).ConfigureAwait(false);
             }
         }

src/AgentMemory.Nams/Client/Neo4jNamsClientAdapter.cs

@@ -1,9 +1,11 @@
+using System.Diagnostics;
 using System.Net.Http.Json;
 using System.Text.Json;
 using System.Text.Json.Serialization;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 using AgentMemory.Nams.Domain;
+using AgentMemory.Nams.Observability;
 
 namespace AgentMemory.Nams.Client;
 
@@ -27,25 +29,30 @@ internal sealed class Neo4jNamsClientAdapter : INamsClient
     private readonly HttpClient _httpClient;
     private readonly NamsRetryPolicy _retryPolicy;
     private readonly string? _apiKeyForRedaction;
+    private readonly NamsMetrics _metrics;
 
-    public Neo4jNamsClientAdapter(HttpClient httpClient, IOptions<NamsOptions> options, ILogger<Neo4jNamsClientAdapter> logger)
+    public Neo4jNamsClientAdapter(
+        HttpClient httpClient, IOptions<NamsOptions> options, ILogger<Neo4jNamsClientAdapter> logger, NamsMetrics metrics)
     {
         _httpClient = httpClient;
         var namsOptions = options.Value;
         _retryPolicy = new NamsRetryPolicy(namsOptions.MaxRetryAttempts, namsOptions.InitialRetryDelay, logger);
         _apiKeyForRedaction = namsOptions.ApiKey;
+        _metrics = metrics;
     }
 
     public Task<NamsConversation> CreateConversationAsync(
         string? userId, IReadOnlyDictionary<string, string>? metadata, CancellationToken cancellationToken) =>
         InvokeAsync(
+            "resolve_conversation",
             () => BuildJsonRequest(HttpMethod.Post, "conversations", new CreateConversationRequestBody(userId, metadata)),
             isIdempotent: false,
             DeserializeAsync<NamsConversation>,
             cancellationToken);
 
     public Task<NamsContext> GetContextAsync(string conversationId, CancellationToken cancellationToken) =>
         InvokeAsync(
+            "get_context",
             () => new HttpRequestMessage(HttpMethod.Get, $"conversations/{Uri.EscapeDataString(conversationId)}/context"),
             isIdempotent: true,
             DeserializeAsync<NamsContext>,
@@ -56,6 +63,7 @@ public async Task<IReadOnlyList<NamsMessage>> AddMessagesAsync(
     {
         var path = $"conversations/{Uri.EscapeDataString(conversationId)}/messages/bulk";
         var response = await InvokeAsync(
+            "store_turn",
             () => BuildJsonRequest(HttpMethod.Post, path, new AddMessagesBulkRequestBody(messages)),
             isIdempotent: false,
             DeserializeAsync<AddMessagesBatchResponseBody>,
@@ -70,37 +78,82 @@ public async Task<IReadOnlyList<NamsEntity>> SearchEntitiesAsync(
         // idempotent for retry purposes (matches the engineering plan's retry matrix, which lists "Search" as
         // always-retryable regardless of the verb it happens to use).
         var response = await InvokeAsync(
+            "search_entities",
             () => BuildJsonRequest(HttpMethod.Post, "entities/search", new SearchEntitiesRequestBody(query, type, limit)),
             isIdempotent: true,
             DeserializeAsync<SearchEntitiesResponseBody>,
             cancellationToken).ConfigureAwait(false);
         return response.Entities;
     }
 
+    public async Task<IReadOnlyList<NamsEntity>> ListEntitiesAsync(int limit, CancellationToken cancellationToken)
+    {
+        var response = await InvokeAsync(
+            "list_entities",
+            () => new HttpRequestMessage(HttpMethod.Get, $"entities?limit={limit}"),
+            isIdempotent: true,
+            DeserializeAsync<ListEntitiesResponseBody>,
+            cancellationToken).ConfigureAwait(false);
+        return response.Entities;
+    }
+
     private async Task<T> InvokeAsync<T>(
+        string operationName,
         Func<HttpRequestMessage> requestFactory,
         bool isIdempotent,
         Func<Stream, CancellationToken, Task<T>> deserialize,
         CancellationToken cancellationToken)
     {
+        using var activity = NamsActivitySource.Instance.StartActivity($"agentmemory.nams.{operationName}");
+        var stopwatch = Stopwatch.StartNew();
         try
         {
-            using var response = await _retryPolicy.ExecuteAsync(requestFactory, _httpClient, isIdempotent, cancellationToken)
+            using var response = await _retryPolicy.ExecuteAsync(
+                requestFactory, _httpClient, isIdempotent, cancellationToken,
+                onRetry: () => _metrics.BackendRetries.Add(1, NamsMetricTags.Operation(operationName)))
                 .ConfigureAwait(false);
             var result = await NamsClientExceptionMapper.MapResponseAsync(response, deserialize, _apiKeyForRedaction, cancellationToken)
                 .ConfigureAwait(false);
-            return result.IsSuccess ? result.Value! : throw NamsClientExceptionMapper.ToException(result);
+            if (result.IsSuccess)
+            {
+                RecordOutcome(operationName, stopwatch.Elapsed, activity, status: "succeeded");
+                return result.Value!;
+            }
+
+            var failure = NamsClientExceptionMapper.ToException(result);
+            RecordFailure(operationName, stopwatch.Elapsed, activity, failure.FailureKind);
+            throw failure;
         }
         catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
         {
-            throw; // caller cancellation -- never wrap
+            // Cancellation is not counted as a service failure (Phase 9's own test list).
+            RecordOutcome(operationName, stopwatch.Elapsed, activity, status: "cancelled");
+            throw;
         }
         catch (Exception ex) when (ex is HttpRequestException or TaskCanceledException)
         {
-            throw NamsClientExceptionMapper.FromTransportException(ex, _apiKeyForRedaction);
+            var failure = NamsClientExceptionMapper.FromTransportException(ex, _apiKeyForRedaction);
+            RecordFailure(operationName, stopwatch.Elapsed, activity, failure.FailureKind);
+            throw failure;
         }
     }
 
+    private void RecordOutcome(string operationName, TimeSpan elapsed, Activity? activity, string status)
+    {
+        _metrics.BackendOperations.Add(1, NamsMetricTags.OperationStatus(operationName, status));
+        _metrics.BackendDurationMs.Record(elapsed.TotalMilliseconds, NamsMetricTags.Operation(operationName));
+        activity?.SetStatus(status == "succeeded" || status == "cancelled" ? ActivityStatusCode.Ok : ActivityStatusCode.Error);
+    }
+
+    private void RecordFailure(string operationName, TimeSpan elapsed, Activity? activity, NamsFailureKind failureKind)
+    {
+        RecordOutcome(operationName, elapsed, activity, status: "failed");
+        _metrics.BackendFailures.Add(1, NamsMetricTags.OperationFailureKind(operationName, failureKind));
+        if (failureKind == NamsFailureKind.RateLimited)
+            _metrics.BackendRateLimited.Add(1, NamsMetricTags.Operation(operationName));
+        activity?.SetStatus(ActivityStatusCode.Error, failureKind.ToString());
+    }
+
     private static HttpRequestMessage BuildJsonRequest<TBody>(HttpMethod method, string relativePath, TBody body) =>
         new(method, relativePath) { Content = JsonContent.Create(body, options: JsonOptions) };
 
@@ -128,4 +181,7 @@ private sealed record SearchEntitiesRequestBody(
     private sealed record SearchEntitiesResponseBody(
         [property: JsonPropertyName("entities")] IReadOnlyList<NamsEntity> Entities,
         [property: JsonPropertyName("searchType")] string? SearchType);
+
+    private sealed record ListEntitiesResponseBody(
+        [property: JsonPropertyName("entities")] IReadOnlyList<NamsEntity> Entities);
 }

src/AgentMemory.Nams/NamsServiceCollectionExtensions.cs

@@ -5,6 +5,7 @@
 using AgentMemory.Nams.Client;
 using AgentMemory.Nams.Identity;
 using AgentMemory.Nams.Internal;
+using AgentMemory.Nams.Observability;
 using AgentMemory.Nams.Persistence;
 using AgentMemory.Nams.Recall;
 
@@ -47,6 +48,9 @@ public static IServiceCollection AddNamsAgentMemory(
         // more than once -- each call just adds another equivalent configure/validate step).
         services.TryAddSingleton<NamsBackendDescriptor>();
 
+        services.TryAddSingleton<NamsMetrics>();
+        services.TryAddSingleton<INamsHealthCheck, NamsHealthCheck>();
+
         services.TryAddSingleton<INamsAccessTokenProvider, StaticApiKeyNamsAccessTokenProvider>();
         services.TryAddTransient<NamsAuthenticationHandler>();
         services.AddHttpClient<INamsClient, Neo4jNamsClientAdapter>((serviceProvider, httpClient) =>

src/AgentMemory.Nams/Observability/INamsHealthCheck.cs

@@ -0,0 +1,12 @@
+namespace AgentMemory.Nams.Observability;
+
+/// <summary>
+/// A lightweight, host-agnostic health probe for the NAMS backend (engineering plan Phase 9). Deliberately
+/// not an ASP.NET Core <c>IHealthCheck</c> -- this package takes no dependency on
+/// <c>Microsoft.Extensions.Diagnostics.HealthChecks</c>, so a host that wants one adapts this thin result
+/// itself. Never performs a destructive write (plan: "no destructive write probe by default").
+/// </summary>
+public interface INamsHealthCheck
+{
+    Task<NamsHealthCheckResult> CheckAsync(CancellationToken cancellationToken);
+}

src/AgentMemory.Nams/Observability/NamsActivitySource.cs

@@ -0,0 +1,19 @@
+using System.Diagnostics;
+
+namespace AgentMemory.Nams.Observability;
+
+/// <summary>
+/// Centralized <see cref="ActivitySource"/> for distributed tracing across NAMS backend operations
+/// (engineering plan Phase 9). Uses the BCL diagnostics APIs directly rather than taking a dependency on
+/// <c>AgentMemory.Observability</c> -- that package decorates the direct-Neo4j-backend's Core interfaces
+/// (<c>IMemoryService</c>, extractors, etc.), none of which this package can reference (B9: zero sibling
+/// <c>AgentMemory.*</c> references).
+/// </summary>
+public static class NamsActivitySource
+{
+    /// <summary>The name used when registering the activity source with OpenTelemetry.</summary>
+    public const string Name = "AgentMemory.Nams";
+
+    /// <summary>Shared instance for all NAMS backend operation spans.</summary>
+    public static readonly ActivitySource Instance = new(Name, "1.0.0");
+}