From d67d54674b3505b4d139a6657e757a6c44a55419 Mon Sep 17 00:00:00 2001 From: Prasanna Sundaravaradan Date: Thu, 13 Aug 2026 13:36:24 -0500 Subject: [PATCH] Wire up OpenTelemetry (traces + metrics) for the core service ARCHITECTURE.md already documented OpenTelemetry as the intended observability approach; it was never actually wired in (README was recently corrected to say so after a grep found zero references to it anywhere in the codebase). This closes that gap. - ASP.NET Core auto-instrumentation for both traces and metrics. - OTLP exporter added only when OTEL_EXPORTER_OTLP_ENDPOINT is set (read through IConfiguration, same as EnvironmentSecretProvider) -- a self-hoster who sets nothing gets no attempted exports. Verified both states directly: ran the service with the env var unset and confirmed clean logs across a request cycle, then with it pointed at a throwaway local HTTP listener and OTEL_EXPORTER_OTLP_PROTOCOL=http/protobuf, confirmed real POST /v1/metrics and /v1/traces requests arriving on the configured export interval. First attempt (before forcing the protocol) came in as HTTP/2 and failed against the plain HTTP/1.1 listener, confirming gRPC is the SDK's actual default -- switchable via the same standard env var, no RiftCache-specific config needed. - New CacheMetrics: hit/miss counters + a live entry-count gauge via System.Diagnostics.Metrics, which the OTel SDK picks up once CacheMetrics.MeterName is registered with .AddMeter(...). - New MeteredCacheStore, an ICacheStore decorator recording hits/misses around GetAsync. Decorator rather than injecting CacheMetrics directly into InMemoryCacheStore: CacheMetrics depends on InMemoryCacheStore for the gauge, so the reverse dependency would be circular. CacheEvictionService keeps injecting the concrete InMemoryCacheStore directly and is unaffected. - Tests use MeterListener filtered by exact Meter *instance* (CacheMetrics now exposes it internally) rather than just MeterName, since multiple tests create their own CacheMetrics with the same meter name and xUnit can run different test classes in parallel -- name-only filtering would leak measurements across tests. README/ROADMAP updates deliberately left out of this PR: they'd describe both this and the still-unmerged http-client-resilience PR as done, and I don't control merge order. Following up once both are confirmed merged into main. Co-Authored-By: Claude Sonnet 5 --- src/RiftCache/Caching/CacheMetrics.cs | 31 ++++++ src/RiftCache/Caching/MeteredCacheStore.cs | 35 +++++++ src/RiftCache/Program.cs | 35 ++++++- src/RiftCache/RiftCache.csproj | 6 ++ .../Caching/CacheMetricsTests.cs | 82 ++++++++++++++++ .../Caching/MeteredCacheStoreTests.cs | 96 +++++++++++++++++++ .../TestSupport/FakeCacheStore.cs | 39 ++++++++ 7 files changed, 323 insertions(+), 1 deletion(-) create mode 100644 src/RiftCache/Caching/CacheMetrics.cs create mode 100644 src/RiftCache/Caching/MeteredCacheStore.cs create mode 100644 tests/RiftCache.Tests/Caching/CacheMetricsTests.cs create mode 100644 tests/RiftCache.Tests/Caching/MeteredCacheStoreTests.cs create mode 100644 tests/RiftCache.Tests/TestSupport/FakeCacheStore.cs diff --git a/src/RiftCache/Caching/CacheMetrics.cs b/src/RiftCache/Caching/CacheMetrics.cs new file mode 100644 index 0000000..8cef037 --- /dev/null +++ b/src/RiftCache/Caching/CacheMetrics.cs @@ -0,0 +1,31 @@ +using System.Diagnostics.Metrics; + +namespace RiftCache.Caching; + +/// +/// Cache hit/miss counters and a live entry-count gauge, published via System.Diagnostics.Metrics +/// so the OpenTelemetry SDK picks them up once MeterName is registered with .AddMeter(...). +/// +public sealed class CacheMetrics +{ + public const string MeterName = "RiftCache.Caching"; + + private readonly Meter _meter; + private readonly Counter _hits; + private readonly Counter _misses; + + public CacheMetrics(InMemoryCacheStore store) + { + _meter = new Meter(MeterName); + _hits = _meter.CreateCounter("riftcache.cache.hits", description: "Cache reads that found a live entry."); + _misses = _meter.CreateCounter("riftcache.cache.misses", description: "Cache reads that found nothing, or an expired entry."); + _meter.CreateObservableGauge("riftcache.cache.entries", () => store.Count, description: "Entries currently held in the in-memory store."); + } + + /// Exposed for tests to filter a MeterListener by exact instance rather than just MeterName, since multiple CacheMetrics instances (one per test) share that name. + internal Meter Meter => _meter; + + public void RecordHit() => _hits.Add(1); + + public void RecordMiss() => _misses.Add(1); +} diff --git a/src/RiftCache/Caching/MeteredCacheStore.cs b/src/RiftCache/Caching/MeteredCacheStore.cs new file mode 100644 index 0000000..bed07b7 --- /dev/null +++ b/src/RiftCache/Caching/MeteredCacheStore.cs @@ -0,0 +1,35 @@ +namespace RiftCache.Caching; + +/// +/// ICacheStore decorator that records hit/miss metrics around GetAsync. Kept separate from +/// InMemoryCacheStore rather than injecting CacheMetrics directly into it, since CacheMetrics +/// itself depends on InMemoryCacheStore (for the entry-count gauge) -- injecting it the other way +/// too would create a circular dependency. +/// +internal sealed class MeteredCacheStore(ICacheStore inner, CacheMetrics metrics) : ICacheStore +{ + public async Task GetAsync(string key, CancellationToken token = default) + { + var value = await inner.GetAsync(key, token).ConfigureAwait(false); + + if (value is null) + { + metrics.RecordMiss(); + } + else + { + metrics.RecordHit(); + } + + return value; + } + + public Task SetAsync(string key, byte[] value, DateTimeOffset? absoluteExpiration, TimeSpan? slidingExpiration, CancellationToken token = default) => + inner.SetAsync(key, value, absoluteExpiration, slidingExpiration, token); + + public Task RefreshAsync(string key, CancellationToken token = default) => + inner.RefreshAsync(key, token); + + public Task RemoveAsync(string key, CancellationToken token = default) => + inner.RemoveAsync(key, token); +} diff --git a/src/RiftCache/Program.cs b/src/RiftCache/Program.cs index 565a1cb..6b48781 100644 --- a/src/RiftCache/Program.cs +++ b/src/RiftCache/Program.cs @@ -1,4 +1,7 @@ using Microsoft.Extensions.Options; +using OpenTelemetry.Metrics; +using OpenTelemetry.Resources; +using OpenTelemetry.Trace; using RiftCache.Caching; using RiftCache.Endpoints; using RiftCache.Options; @@ -13,9 +16,39 @@ builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); -builder.Services.AddSingleton(sp => sp.GetRequiredService()); +builder.Services.AddSingleton(); +builder.Services.AddSingleton(sp => + new MeteredCacheStore(sp.GetRequiredService(), sp.GetRequiredService())); builder.Services.AddHostedService(); +// Only export via OTLP when an endpoint is actually configured -- otherwise a self-hoster who set +// nothing gets zero telemetry noise (no attempted connections, no connection-refused warnings). +// Read through IConfiguration, not Environment.GetEnvironmentVariable, for the same reason +// EnvironmentSecretProvider does: picks up env vars, user secrets, and config files uniformly. +var otlpEndpoint = builder.Configuration["OTEL_EXPORTER_OTLP_ENDPOINT"]; + +builder.Services.AddOpenTelemetry() + .ConfigureResource(resource => resource.AddService("RiftCache")) + .WithTracing(tracing => + { + tracing.AddAspNetCoreInstrumentation(); + + if (!string.IsNullOrEmpty(otlpEndpoint)) + { + tracing.AddOtlpExporter(); + } + }) + .WithMetrics(metrics => + { + metrics.AddAspNetCoreInstrumentation(); + metrics.AddMeter(CacheMetrics.MeterName); + + if (!string.IsNullOrEmpty(otlpEndpoint)) + { + metrics.AddOtlpExporter(); + } + }); + var app = builder.Build(); var serverOptions = app.Services.GetRequiredService>().Value; diff --git a/src/RiftCache/RiftCache.csproj b/src/RiftCache/RiftCache.csproj index 7e27f69..287fcbe 100644 --- a/src/RiftCache/RiftCache.csproj +++ b/src/RiftCache/RiftCache.csproj @@ -11,4 +11,10 @@ + + + + + + diff --git a/tests/RiftCache.Tests/Caching/CacheMetricsTests.cs b/tests/RiftCache.Tests/Caching/CacheMetricsTests.cs new file mode 100644 index 0000000..c88f7c7 --- /dev/null +++ b/tests/RiftCache.Tests/Caching/CacheMetricsTests.cs @@ -0,0 +1,82 @@ +using System.Diagnostics.Metrics; +using RiftCache.Caching; +using RiftCache.Providers.Persistence; +using RiftCache.Tests.TestSupport; + +namespace RiftCache.Tests.Caching; + +public class CacheMetricsTests +{ + [Fact] + public void RecordHit_PublishesToHitsCounter() + { + var metrics = new CacheMetrics(new InMemoryCacheStore(new FakePersistenceProvider(), TimeProvider.System)); + + // Filters by exact Meter instance, not just CacheMetrics.MeterName -- multiple tests + // create their own CacheMetrics (and therefore their own Meter with the same name), and + // xUnit can run different test classes in parallel, so name-only filtering would leak + // measurements across tests. + using var listener = new MeterListener(); + var measurements = new List(); + listener.InstrumentPublished = (instrument, l) => + { + if (ReferenceEquals(instrument.Meter, metrics.Meter) && instrument.Name == "riftcache.cache.hits") + { + l.EnableMeasurementEvents(instrument); + } + }; + listener.SetMeasurementEventCallback((_, measurement, _, _) => measurements.Add(measurement)); + listener.Start(); + + metrics.RecordHit(); + metrics.RecordHit(); + + Assert.Equal([1, 1], measurements); + } + + [Fact] + public void RecordMiss_PublishesToMissesCounter() + { + var metrics = new CacheMetrics(new InMemoryCacheStore(new FakePersistenceProvider(), TimeProvider.System)); + + using var listener = new MeterListener(); + var measurements = new List(); + listener.InstrumentPublished = (instrument, l) => + { + if (ReferenceEquals(instrument.Meter, metrics.Meter) && instrument.Name == "riftcache.cache.misses") + { + l.EnableMeasurementEvents(instrument); + } + }; + listener.SetMeasurementEventCallback((_, measurement, _, _) => measurements.Add(measurement)); + listener.Start(); + + metrics.RecordMiss(); + + Assert.Equal([1], measurements); + } + + [Fact] + public async Task EntriesGauge_ReportsLiveStoreCount() + { + var store = new InMemoryCacheStore(new FakePersistenceProvider(), TimeProvider.System); + await store.SetAsync("a", [1], null, null); + await store.SetAsync("b", [2], null, null); + var metrics = new CacheMetrics(store); + + using var listener = new MeterListener(); + var measurements = new List(); + listener.InstrumentPublished = (instrument, l) => + { + if (ReferenceEquals(instrument.Meter, metrics.Meter) && instrument.Name == "riftcache.cache.entries") + { + l.EnableMeasurementEvents(instrument); + } + }; + listener.SetMeasurementEventCallback((_, measurement, _, _) => measurements.Add(measurement)); + listener.Start(); + listener.RecordObservableInstruments(); + + Assert.Equal([2], measurements); + } +} diff --git a/tests/RiftCache.Tests/Caching/MeteredCacheStoreTests.cs b/tests/RiftCache.Tests/Caching/MeteredCacheStoreTests.cs new file mode 100644 index 0000000..17f1413 --- /dev/null +++ b/tests/RiftCache.Tests/Caching/MeteredCacheStoreTests.cs @@ -0,0 +1,96 @@ +using System.Diagnostics.Metrics; +using RiftCache.Caching; +using RiftCache.Providers.Persistence; +using RiftCache.Tests.TestSupport; + +namespace RiftCache.Tests.Caching; + +public class MeteredCacheStoreTests +{ + [Fact] + public async Task GetAsync_ValueFound_ReturnsItAndRecordsHit() + { + var inner = new FakeCacheStore { NextGetResult = [1, 2, 3] }; + var metrics = new CacheMetrics(new InMemoryCacheStore(new FakePersistenceProvider(), TimeProvider.System)); + var store = new MeteredCacheStore(inner, metrics); + + using var listener = new MeterListener(); + var hits = new List(); + listener.InstrumentPublished = (instrument, l) => + { + if (ReferenceEquals(instrument.Meter, metrics.Meter) && instrument.Name == "riftcache.cache.hits") + { + l.EnableMeasurementEvents(instrument); + } + }; + listener.SetMeasurementEventCallback((_, measurement, _, _) => hits.Add(measurement)); + listener.Start(); + + var result = await store.GetAsync("key"); + + Assert.Equal(new byte[] { 1, 2, 3 }, result); + Assert.Equal([1], hits); + } + + [Fact] + public async Task GetAsync_ValueMissing_ReturnsNullAndRecordsMiss() + { + var inner = new FakeCacheStore { NextGetResult = null }; + var metrics = new CacheMetrics(new InMemoryCacheStore(new FakePersistenceProvider(), TimeProvider.System)); + var store = new MeteredCacheStore(inner, metrics); + + using var listener = new MeterListener(); + var misses = new List(); + listener.InstrumentPublished = (instrument, l) => + { + if (ReferenceEquals(instrument.Meter, metrics.Meter) && instrument.Name == "riftcache.cache.misses") + { + l.EnableMeasurementEvents(instrument); + } + }; + listener.SetMeasurementEventCallback((_, measurement, _, _) => misses.Add(measurement)); + listener.Start(); + + var result = await store.GetAsync("key"); + + Assert.Null(result); + Assert.Equal([1], misses); + } + + [Fact] + public async Task SetAsync_DelegatesToInner() + { + var store = CreateStore(out var inner); + + await store.SetAsync("key", [1], null, null); + + Assert.Equal(1, inner.SetCallCount); + } + + [Fact] + public async Task RefreshAsync_DelegatesToInnerAndReturnsItsResult() + { + var store = CreateStore(out var inner); + inner.NextRefreshResult = false; + + Assert.False(await store.RefreshAsync("key")); + Assert.Equal(1, inner.RefreshCallCount); + } + + [Fact] + public async Task RemoveAsync_DelegatesToInnerAndReturnsItsResult() + { + var store = CreateStore(out var inner); + inner.NextRemoveResult = false; + + Assert.False(await store.RemoveAsync("key")); + Assert.Equal(1, inner.RemoveCallCount); + } + + private static MeteredCacheStore CreateStore(out FakeCacheStore inner) + { + inner = new FakeCacheStore(); + var metrics = new CacheMetrics(new InMemoryCacheStore(new FakePersistenceProvider(), TimeProvider.System)); + return new MeteredCacheStore(inner, metrics); + } +} diff --git a/tests/RiftCache.Tests/TestSupport/FakeCacheStore.cs b/tests/RiftCache.Tests/TestSupport/FakeCacheStore.cs new file mode 100644 index 0000000..39a4f56 --- /dev/null +++ b/tests/RiftCache.Tests/TestSupport/FakeCacheStore.cs @@ -0,0 +1,39 @@ +using RiftCache.Caching; + +namespace RiftCache.Tests.TestSupport; + +internal sealed class FakeCacheStore : ICacheStore +{ + public byte[]? NextGetResult { get; set; } + + public bool NextRefreshResult { get; set; } = true; + + public bool NextRemoveResult { get; set; } = true; + + public int SetCallCount { get; private set; } + + public int RefreshCallCount { get; private set; } + + public int RemoveCallCount { get; private set; } + + public Task GetAsync(string key, CancellationToken token = default) => + Task.FromResult(NextGetResult); + + public Task SetAsync(string key, byte[] value, DateTimeOffset? absoluteExpiration, TimeSpan? slidingExpiration, CancellationToken token = default) + { + SetCallCount++; + return Task.CompletedTask; + } + + public Task RefreshAsync(string key, CancellationToken token = default) + { + RefreshCallCount++; + return Task.FromResult(NextRefreshResult); + } + + public Task RemoveAsync(string key, CancellationToken token = default) + { + RemoveCallCount++; + return Task.FromResult(NextRemoveResult); + } +}