Repository navigation
Wire up OpenTelemetry (traces + metrics) for the core service - #10
Merged
Merged
Conversation
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.
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.