feat(otel): support signal-specific OTLP export#148
Conversation
yordis
commented
May 1, 2026
yordis
commented
- Operators need one observability path for logs and metrics instead of a metrics-only exporter story.
- OTLP deployments often route each signal to a different collector, so shared defaults need safe per-signal overrides.
- The public diagnostics docs need to match the built-in telemetry surface operators can actually configure.
PR SummaryMedium Risk Overview Reworks OTLP metrics export configuration to support shared/per-signal OTLP settings and a new runtime enable switch ( Updates diagnostics docs to reflect the new Reviewed by Cursor Bugbot for commit 8ec6ff4. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
WalkthroughReplaces the commercial OpenTelemetry exporter plugin docs and wiring with built-in OTLP support for logs and metrics, adds configuration helpers for shared and per-signal OTLP settings, integrates a Serilog OpenTelemetry sink, updates startup/metrics wiring, and adds tests covering shared vs per-signal behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant App as EventStore App
participant Config as IConfiguration
participant Logger as EventStoreLoggerConfiguration / OpenTelemetryLogger
participant Serilog as Serilog Pipeline
participant OTLP as OTLP Collector
rect rgba(200,220,255,0.5)
App->>Config: load `EventStore:OpenTelemetry` config
end
App->>Logger: Initialize(..., telemetryConfiguration)
Logger->>Config: OtlpLogsEnabled()? / GetOtlpExporterOptions(...)
Logger->>Serilog: AddOpenTelemetryLogger(options)
Serilog->>OTLP: Send log records via configured protocol/endpoint
OTLP->>OTLP: (collector receives/export)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 39 minutes and 44 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/EventStore.Core.XUnit.Tests/Metrics/OtlpMetricsConfigurationTests.cs (1)
56-73: ⚡ Quick winAdd explicit assertion for per-signal
Headersoverride precedence.This test currently validates shared-header fallback, but not the case where
EventStore:OpenTelemetry:Metrics:Otlp:Headersis set and should win over shared settings.✅ Targeted test diff
var configuration = new ConfigurationBuilder() .AddInMemoryCollection(new Dictionary<string, string> { ["EventStore:OpenTelemetry:Otlp:Endpoint"] = "http://shared:4317", ["EventStore:OpenTelemetry:Otlp:Headers"] = "key=shared", ["EventStore:OpenTelemetry:Metrics:Otlp:Endpoint"] = "http://metrics:4317", ["EventStore:OpenTelemetry:Metrics:Otlp:Protocol"] = "HttpProtobuf", + ["EventStore:OpenTelemetry:Metrics:Otlp:Headers"] = "key=metrics", }) .Build(); var options = configuration.GetOtlpExporterOptions(OpenTelemetryConfiguration.OtlpMetricsOtlpPrefix); options.Endpoint.Should().Be(new Uri("http://metrics:4317")); - options.Headers.Should().Be("key=shared"); + options.Headers.Should().Be("key=metrics"); options.Protocol.Should().Be(OtlpExportProtocol.HttpProtobuf);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/EventStore.Core.XUnit.Tests/Metrics/OtlpMetricsConfigurationTests.cs` around lines 56 - 73, In the PerSignalMetricsOtlpOverridesSharedSettings test, add a per-signal Headers entry to the in-memory configuration and assert it wins over the shared Headers; specifically, add ["EventStore:OpenTelemetry:Metrics:Otlp:Headers"] = "key=metrics" to the Dictionary<string,string> and change or add an assertion so options.Headers.Should().Be("key=metrics") while keeping the existing Endpoint and Protocol assertions (test method: PerSignalMetricsOtlpOverridesSharedSettings, options variable from GetOtlpExporterOptions).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/diagnostics/integrations.md`:
- Around line 57-63: Update the integrations documentation table and related
text to include the new runtime config switch
EventStore__OpenTelemetry__Metrics__Enabled (also referenced as
EventStore:OpenTelemetry:Metrics:Enabled) as a supported way to enable metrics
export; add a new row describing that key and its purpose, and update the
troubleshooting paragraph(s) that discuss metrics enablement to mention this
runtime-config switch alongside the shared OTLP endpoints and metricsconfig.json
fallback (also apply the same addition to the corresponding section referenced
at lines 78-82).
In `@src/EventStore.Core/Configuration/OpenTelemetryConfiguration.cs`:
- Around line 21-24: OtlpMetricsEnabled (in OpenTelemetryConfiguration) misses
the presence of the metrics-specific OTLP section, so add a check for the
existence of the EventStore:OpenTelemetry:Metrics:Otlp section; update the
expression in OtlpMetricsEnabled to also return true when
configuration.GetSection($"{OtlpMetricsPrefix}:Otlp").Exists(), keeping the
existing checks for metricsConfiguration.Otlp.Enabled, OtlpConfigPrefix section,
and $"{OtlpMetricsPrefix}:Enabled".
In `@src/EventStore.Core/Log/OpenTelemetryLogger.cs`:
- Around line 33-64: The current global call to
Filter.ByExcluding(Matching.FromSource("REGULAR-STATS-LOGGER")) is applied
before WriteTo.OpenTelemetry and thus affects all sinks; change the
configuration to scope the exclusion to only the OTLP sink by wrapping the sink
in WriteTo.Logger(lc =>
lc.Filter.ByExcluding(Matching.FromSource("REGULAR-STATS-LOGGER")).WriteTo.OpenTelemetry(...)),
keeping the same OpenTelemetry options and getConfigurationVariable delegate so
only the OpenTelemetry sink is filtered while other sinks (file, console, etc.)
remain unaffected.
---
Nitpick comments:
In `@src/EventStore.Core.XUnit.Tests/Metrics/OtlpMetricsConfigurationTests.cs`:
- Around line 56-73: In the PerSignalMetricsOtlpOverridesSharedSettings test,
add a per-signal Headers entry to the in-memory configuration and assert it wins
over the shared Headers; specifically, add
["EventStore:OpenTelemetry:Metrics:Otlp:Headers"] = "key=metrics" to the
Dictionary<string,string> and change or add an assertion so
options.Headers.Should().Be("key=metrics") while keeping the existing Endpoint
and Protocol assertions (test method:
PerSignalMetricsOtlpOverridesSharedSettings, options variable from
GetOtlpExporterOptions).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ac93b89c-acf5-4bfc-9a54-22ee1895318a
📒 Files selected for processing (11)
docs/diagnostics/integrations.mddocs/diagnostics/metrics.mdsrc/Directory.Packages.propssrc/EventStore.ClusterNode/Program.cssrc/EventStore.Core.XUnit.Tests/Metrics/OtlpMetricsConfigurationTests.cssrc/EventStore.Core.XUnit.Tests/OpenTelemetry/OpenTelemetryLoggerTests.cssrc/EventStore.Core/ClusterVNodeStartup.cssrc/EventStore.Core/Configuration/OpenTelemetryConfiguration.cssrc/EventStore.Core/EventStore.Core.csprojsrc/EventStore.Core/Log/EventStoreLoggerConfiguration.cssrc/EventStore.Core/Log/OpenTelemetryLogger.cs
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
yordis
force-pushed
the
yordis/feat-otlp-signal-export
branch
2 times, most recently
from
cc100b3 to
5074105
Compare
yordis
force-pushed
the
yordis/feat-otlp-signal-export
branch
from
5074105 to
14a8c59
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 14a8c59. Configure here.
yordis
force-pushed
the
yordis/feat-otlp-signal-export
branch
from
14a8c59 to
611f6e5
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/diagnostics/integrations.md`:
- Around line 81-85: The troubleshooting table is inaccurate: the
OtlpMetricsEnabled check uses
configuration.GetSection(OtlpMetricsOtlpPrefix).Exists(), so the presence of any
key under EventStore:OpenTelemetry:Metrics:Otlp (not just Endpoint) will enable
metrics export; update the "Metrics are not exported" row to reflect that
behavior by stating users should verify either
EventStore__OpenTelemetry__Metrics__Otlp__Endpoint is set or that any key exists
under EventStore__OpenTelemetry__Metrics__Otlp (and mention that this behavior
comes from OtlpMetricsEnabled using configuration.GetSection(...).Exists()).
In `@src/EventStore.Core/Log/OpenTelemetryLogger.cs`:
- Around line 42-51: In OpenTelemetryLogger.cs update the block that sets
options.Endpoint/options.Protocol for otlpExporterConfig so that when
otlpExporterConfig.Protocol == OtlpExportProtocol.HttpProtobuf you ensure the
endpoint includes the OTLP logs path (e.g. append "/v1/logs" when the configured
Endpoint has no path) before assigning options.Endpoint (or alternatively set
the Serilog sink's LogsEndpoint property instead of Endpoint); locate the code
around the switch on otlpExporterConfig.Protocol and add logic to normalize
otlpExporterConfig.Endpoint.AbsoluteUri (handling trailing slashes and avoiding
double slashes) or use the LogsEndpoint property so explicitly-configured base
URLs (like http://collector:4318) are converted to the correct
http://.../v1/logs target.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2b436451-58ee-4f91-908a-85f9f0c736b0
📒 Files selected for processing (11)
docs/diagnostics/integrations.mddocs/diagnostics/metrics.mdsrc/Directory.Packages.propssrc/EventStore.ClusterNode/Program.cssrc/EventStore.Core.XUnit.Tests/Metrics/OtlpMetricsConfigurationTests.cssrc/EventStore.Core.XUnit.Tests/OpenTelemetry/OpenTelemetryLoggerTests.cssrc/EventStore.Core/ClusterVNodeStartup.cssrc/EventStore.Core/Configuration/OpenTelemetryConfiguration.cssrc/EventStore.Core/EventStore.Core.csprojsrc/EventStore.Core/Log/EventStoreLoggerConfiguration.cssrc/EventStore.Core/Log/OpenTelemetryLogger.cs
✅ Files skipped from review due to trivial changes (4)
- src/EventStore.Core/EventStore.Core.csproj
- src/EventStore.ClusterNode/Program.cs
- src/Directory.Packages.props
- docs/diagnostics/metrics.md
🚧 Files skipped from review as they are similar to previous changes (3)
- src/EventStore.Core/Log/EventStoreLoggerConfiguration.cs
- src/EventStore.Core.XUnit.Tests/OpenTelemetry/OpenTelemetryLoggerTests.cs
- src/EventStore.Core.XUnit.Tests/Metrics/OtlpMetricsConfigurationTests.cs
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
yordis
force-pushed
the
yordis/feat-otlp-signal-export
branch
from
611f6e5 to
8ec6ff4
Compare
1 participant
