feat(telemetry): HTTP body logging, enricher refactor, and background job improvements#50
Merged
yilmaztayfun merged 2 commits intoMar 12, 2026
Conversation
… job improvements Telemetry: - Add configurable HTTP request/response body logging middleware (UseHttpBodyLogging) with Body options, path exclusion, and sensitive field/header redaction - Introduce HttpBodyLoggingOptions, LoggingEnricherOptions in TelemetryOptions - Replace AetherLogEnricherProcessor with EnricherLogProcessor; document Tracing.Headers as explicit list (no wildcard) - Update telemetry README with Logging.Body and Enrichers configuration Background jobs: - Set CreatedAt in BackgroundJobInfo constructor, ModifiedAt in EfCoreJobStore on update and status change - Integrate IJobScheduler.DeleteAsync in JobDispatcher on completion, failure, and cancellation; refactor idempotency check to use single job fetch Made-with: Cursor
Closed
5 tasks
Reviewer's GuideAdds configurable HTTP request/response body logging middleware to the telemetry stack, refactors log enrichment to use a new processor and tracing header options, and hardens background job processing with created/modified timestamps and scheduler cleanup on completion/failure/cancel. Sequence diagram for HTTP request logging with body middleware and enricher processorsequenceDiagram
actor User
participant Browser
participant AspNetCoreApp
participant HttpBodyLoggingMiddleware
participant NextMiddleware
participant ILogger
participant EnricherLogProcessor
participant OtelExporter
User->>Browser: Send HTTP request
Browser->>AspNetCoreApp: HTTP request
AspNetCoreApp->>HttpBodyLoggingMiddleware: Invoke(HttpContext)
HttpBodyLoggingMiddleware->>HttpBodyLoggingMiddleware: ShouldProcess(path, ExcludedPaths)
alt Path excluded
HttpBodyLoggingMiddleware->>NextMiddleware: Invoke(HttpContext)
NextMiddleware-->>HttpBodyLoggingMiddleware: Response
HttpBodyLoggingMiddleware-->>AspNetCoreApp: Response
else Path included
alt EnableRequestBody && allowed content type
HttpBodyLoggingMiddleware->>HttpBodyLoggingMiddleware: CaptureRequestBodyAsync
end
HttpBodyLoggingMiddleware->>NextMiddleware: Invoke(HttpContext)
NextMiddleware-->>HttpBodyLoggingMiddleware: Response
alt EnableResponseBody && allowed content type
HttpBodyLoggingMiddleware->>HttpBodyLoggingMiddleware: CaptureResponseBodyAsync
end
HttpBodyLoggingMiddleware->>HttpBodyLoggingMiddleware: Build scope
HttpBodyLoggingMiddleware->>HttpBodyLoggingMiddleware: AddEnricherProperties(CustomAttributes + Headers)
HttpBodyLoggingMiddleware->>ILogger: LogInformation("HTTP request/response logged ...")
ILogger-->>AspNetCoreApp: LogRecord
end
note over ILogger,EnricherLogProcessor: Logging pipeline processes LogRecord
ILogger->>EnricherLogProcessor: OnEnd(LogRecord)
alt Record already has RequestHeaders/RequestBody
EnricherLogProcessor->>EnricherLogProcessor: HasBodyOrHeaderEnrichment == true
EnricherLogProcessor-->>ILogger: Skip enrichment
else
EnricherLogProcessor->>EnricherLogProcessor: Add CustomAttributes
EnricherLogProcessor->>EnricherLogProcessor: Add header enrichers (RequestHeader.*, ResponseHeader.*) with redaction
EnricherLogProcessor-->>ILogger: Enriched LogRecord
end
ILogger->>OtelExporter: Export enriched log
OtelExporter-->>AspNetCoreApp: Ack
AspNetCoreApp-->>Browser: HTTP response
Sequence diagram for background job dispatch with scheduler cleanupsequenceDiagram
participant Scheduler as ExternalScheduler
participant JobDispatcher
participant Scope as DI_Scope
participant IUnitOfWorkManager
participant IJobStore
participant IJobScheduler
participant IUnitOfWork
participant JobHandler
Scheduler->>JobDispatcher: DispatchAsync(jobId, handlerName)
JobDispatcher->>Scope: CreateScope()
Scope-->>JobDispatcher: scope
JobDispatcher->>Scope: Resolve IUnitOfWorkManager
JobDispatcher->>Scope: Resolve IJobStore
JobDispatcher->>Scope: Resolve IJobScheduler
JobDispatcher->>IUnitOfWorkManager: BeginRequiresNew()
IUnitOfWorkManager-->>JobDispatcher: IUnitOfWork uow
JobDispatcher->>IJobStore: GetAsync(jobId)
IJobStore-->>JobDispatcher: BackgroundJobInfo jobInfo
alt jobInfo null
JobDispatcher->>IUnitOfWork: CommitAsync()
JobDispatcher-->>Scheduler: return
end
JobDispatcher->>JobDispatcher: IsJobAlreadyProcessed(jobInfo,...)
alt Already completed or cancelled
JobDispatcher->>IUnitOfWork: CommitAsync()
JobDispatcher->>IJobScheduler: TryDeleteFromSchedulerAsync(handlerName, jobInfo.JobName)
IJobScheduler-->>JobDispatcher: DeleteAsync result (ignored on failure)
JobDispatcher-->>Scheduler: return
else Not processed yet
JobDispatcher->>Scope: Resolve JobHandler
JobDispatcher->>IJobStore: UpdateStatusAsync(jobId, Running,...)
JobDispatcher->>IUnitOfWork: CommitAsync()
JobDispatcher->>IUnitOfWorkManager: BeginRequiresNew()
IUnitOfWorkManager-->>JobDispatcher: IUnitOfWork handlerUow
JobDispatcher->>JobHandler: HandleAsync(jobInfo)
alt Handler completes
JobHandler-->>JobDispatcher: success
JobDispatcher->>IJobStore: UpdateStatusAsync(jobId, Completed,...)
JobDispatcher->>IUnitOfWork: CommitAsync()
JobDispatcher->>IJobScheduler: TryDeleteFromSchedulerAsync(handlerName, jobInfo.JobName)
else Handler cancelled
JobHandler-->>JobDispatcher: OperationCanceledException
JobDispatcher->>JobDispatcher: MarkJobStatusAsync(Cancelled)
JobDispatcher->>IJobScheduler: TryDeleteFromSchedulerAsync(handlerName, jobInfo.JobName)
else Handler throws
JobHandler-->>JobDispatcher: Exception
JobDispatcher->>JobDispatcher: MarkJobStatusAsync(Failed)
JobDispatcher->>IJobScheduler: TryDeleteFromSchedulerAsync(handlerName, jobInfo.JobName)
end
JobDispatcher-->>Scheduler: return
end
Class diagram for updated telemetry logging options and processorsclassDiagram
class AetherTelemetryOptions {
+bool TracingEnabled
+bool MetricsEnabled
+bool LoggingEnabled
+AetherLoggingOptions Logging
+AetherTracingOptions Tracing
+AetherMetricsOptions Metrics
}
class AetherLoggingOptions {
+bool EnableOtlpExporter
+bool EnableConsoleExporter
+List~string~ ExcludedPaths
+LoggingEnricherOptions Enrichers
+HttpBodyLoggingOptions Body
}
class HttpBodyLoggingOptions {
<<sealed>>
+static IReadOnlyList~string~ DefaultSensitiveJsonFields
+static IReadOnlyList~string~ DefaultSensitiveHeaderNames
+bool EnableRequestBody
+bool EnableResponseBody
+int MaxBodyLengthToCapture
+List~string~ AllowedContentTypes
+List~string~ AdditionalSensitiveJsonFields
+List~string~ AdditionalSensitiveHeaderNames
}
class LoggingEnricherOptions {
<<sealed>>
+Dictionary~string,string~ CustomAttributes
+List~string~ Headers
}
class AetherTracingOptions {
<<sealed>>
+bool EnableAspNetCore
+bool EnableHttpClient
+List~string~ AdditionalSources
+List~string~ ExcludedPaths
+List~string~ Headers
}
class HttpBodyLoggingMiddleware {
<<sealed>>
-RequestDelegate _next
-ILogger_HttpBodyLoggingMiddleware_ _logger
-HttpBodyLoggingOptions _bodyOptions
-LoggingEnricherOptions _enricherOptions
-List~Regex~ _excludedPatterns
-HashSet~string~ _sensitiveJsonFields
-HashSet~string~ _sensitiveHeaderNames
+HttpBodyLoggingMiddleware(RequestDelegate next, ILogger_HttpBodyLoggingMiddleware_ logger, IOptions_AetherTelemetryOptions_ options)
+Task Invoke(HttpContext context)
-bool ShouldProcess(HttpContext context)
-bool CanReadRequestBody(HttpRequest request)
-bool CanReadResponseBody(HttpResponse response)
-bool IsAllowedContentType(string contentType)
-Task<(string Body,int Size,bool Truncated,bool Captured)> CaptureRequestBodyAsync(HttpRequest request)
-Task<(string Body,int Size,bool Truncated,bool Captured)> CaptureResponseBodyAsync(HttpResponse response, MemoryStream buffer)
-void LogBodies(HttpContext context, string requestBody, string responseBody, int requestBodySize, int responseBodySize, bool requestTruncated, bool responseTruncated, bool requestCaptured, bool responseCaptured, string requestHeadersJson, string responseHeadersJson)
-void AddEnricherProperties(List~KeyValuePair~string,object~~ scope, HttpContext context, string requestHeadersJson, string responseHeadersJson)
}
class EnricherLogProcessor {
<<sealed>>
-HashSet~string~ _sensitiveHeaderNames
+EnricherLogProcessor(AetherTelemetryOptions options, IHttpContextAccessor httpContextAccessor)
+void OnEnd(LogRecord record)
-static HashSet~string~ BuildSensitiveHeaderNames(AetherTelemetryOptions options)
-static bool HasBodyOrHeaderEnrichment(LogRecord record)
-static string NormalizeHeaderKey(string key)
}
class HttpBodyLoggingApplicationBuilderExtensions {
<<static>>
+IApplicationBuilder UseHttpBodyLogging(IApplicationBuilder app)
}
AetherTelemetryOptions --> AetherLoggingOptions : uses
AetherTelemetryOptions --> AetherTracingOptions : uses
AetherLoggingOptions --> LoggingEnricherOptions : has
AetherLoggingOptions --> HttpBodyLoggingOptions : has
HttpBodyLoggingMiddleware --> HttpBodyLoggingOptions : config
HttpBodyLoggingMiddleware --> LoggingEnricherOptions : config
EnricherLogProcessor --> AetherTelemetryOptions : config
EnricherLogProcessor --> HttpBodyLoggingOptions : redaction
HttpBodyLoggingApplicationBuilderExtensions --> HttpBodyLoggingMiddleware : registers
AetherTracingOptions --> AetherTelemetryOptions : part of
Class diagram for updated background job componentsclassDiagram
class BackgroundJobInfo {
+Guid Id
+string HandlerName
+string JobName
+BackgroundJobStatus Status
+DateTime CreatedAt
+DateTime? ModifiedAt
+DateTime? HandledTime
+ExtraPropertyDictionary ExtraProperties
+byte[] Payload
+BackgroundJobInfo(Guid id, string handlerName, string jobName)
}
class EfCoreJobStore {
-DbContext _dbContext
+Task SaveAsync(BackgroundJobInfo jobInfo, CancellationToken cancellationToken)
+Task UpdateStatusAsync(Guid id, BackgroundJobStatus status, DateTime? handledTime, string result, CancellationToken cancellationToken)
}
class JobDispatcher {
-ILogger_JobDispatcher_ logger
+Task DispatchAsync(Guid jobId, string handlerName, CancellationToken cancellationToken)
-bool IsJobAlreadyProcessed(BackgroundJobInfo jobInfo, Guid jobId, string handlerName)
-Task TryDeleteFromSchedulerAsync(IJobScheduler jobScheduler, string handlerName, string jobName, CancellationToken cancellationToken)
-Task MarkJobStatusAsync(IUnitOfWorkManager uowManager, IJobStore jobStore, Guid jobId, BackgroundJobStatus status, string reason, CancellationToken cancellationToken)
}
class IJobStore {
<<interface>>
+Task<BackgroundJobInfo> GetAsync(Guid id, CancellationToken cancellationToken)
+Task SaveAsync(BackgroundJobInfo jobInfo, CancellationToken cancellationToken)
+Task UpdateStatusAsync(Guid id, BackgroundJobStatus status, DateTime? handledTime, string result, CancellationToken cancellationToken)
}
class IJobScheduler {
<<interface>>
+Task DeleteAsync(string handlerName, string jobName, CancellationToken cancellationToken)
}
class IUnitOfWorkManager {
<<interface>>
+Task<IUnitOfWork> BeginRequiresNew(CancellationToken cancellationToken)
}
class IUnitOfWork {
<<interface>>
+Task CommitAsync(CancellationToken cancellationToken)
+ValueTask DisposeAsync()
}
EfCoreJobStore ..|> IJobStore
JobDispatcher --> IJobStore : uses
JobDispatcher --> IJobScheduler : uses
JobDispatcher --> IUnitOfWorkManager : uses
EfCoreJobStore --> BackgroundJobInfo : persists
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Caution Review failedPull request was closed or merged during review Note
|
| Cohort / File(s) | Summary |
|---|---|
Documentation framework/docs/telemetry/README.md |
Updated docs to reflect new Telemetry top-level config, tracing headers, logging enrichers (CustomAttributes, Headers), HTTP body logging options, middleware integration, redaction and excluded paths. |
Log Enrichment framework/src/BBT.Aether.AspNetCore/.../Telemetry/AetherLogEnricherProcessor.cs, framework/src/BBT.Aether.AspNetCore/.../Telemetry/EnricherLogProcessor.cs |
Removed legacy AetherLogEnricherProcessor; added EnricherLogProcessor that aggregates custom attributes and request/response headers, normalizes keys, redacts sensitive headers, skips if record already has body/header enrichment, and safely merges attributes. |
HTTP Body Logging Middleware framework/src/BBT.Aether.AspNetCore/.../Telemetry/HttpBodyLoggingMiddleware.cs, framework/src/BBT.Aether.AspNetCore/Microsoft/AspNetCore/Builder/HttpBodyLoggingApplicationBuilderExtensions.cs |
New middleware to capture/log request and response bodies and headers with content-type allow-listing, truncation, JSON-field/header redaction, path exclusions, and extension method to register middleware. |
Telemetry Configuration framework/src/BBT.Aether.AspNetCore/.../Telemetry/TelemetryOptions.cs |
Added HttpBodyLoggingOptions and defaults, added Logging.Enrichers.CustomAttributes, moved/added Tracing.Headers, and new body-related configuration properties (EnableRequestBody/ResponseBody, MaxBodyLengthToCapture, AllowedContentTypes, additional sensitive fields/headers). |
Service Registration framework/src/BBT.Aether.AspNetCore/.../DependencyInjection/AetherTelemetryServiceCollectionExtensions.cs |
Switched tracing header source to Tracing.Headers; replaced old enricher registration with EnricherLogProcessor; adjusted exclusion pattern usage for logging/tracing setup. |
Background Job Lifecycle framework/src/BBT.Aether.Domain/.../BackgroundJobInfo.cs, framework/src/BBT.Aether.Infrastructure/.../BackgroundJob/EfCoreJobStore.cs, framework/src/BBT.Aether.Infrastructure/.../BackgroundJob/JobDispatcher.cs |
Initialize CreatedAt on job creation; set ModifiedAt on updates; JobDispatcher now uses IJobScheduler, removes jobs from scheduler after various outcomes, changes idempotency checks to use prefetched BackgroundJobInfo and performs scheduler cleanup on duplicate/failure paths. |
Sequence Diagram(s)
sequenceDiagram
participant Client as Client
participant App as App Pipeline
participant BodyMW as HttpBodyLoggingMiddleware
participant Handler as Request Handler
participant Logger as ILogger/OpenTelemetry
participant Enricher as EnricherLogProcessor
Client->>App: HTTP Request
App->>BodyMW: invoke
BodyMW->>BodyMW: check excluded paths
alt included
BodyMW->>BodyMW: capture request headers/body
BodyMW->>Handler: invoke handler
Handler-->>BodyMW: response
BodyMW->>BodyMW: capture response headers/body, redact/truncate
BodyMW->>Logger: emit log record (with body/headers)
Logger->>Enricher: OnEnd(LogRecord)
Enricher->>Enricher: detect existing enrichment
Enricher->>Enricher: aggregate custom attrs + headers, redact sensitive
Enricher->>Logger: merge attributes back
else excluded
BodyMW->>Handler: pass through
end
BodyMW-->>Client: HTTP Response
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related issues
- Telemetry: HTTP status code, wildcard headers, and request/response logging configuration #48: Overlaps changes to header enrichment and request/response logging with redaction; this PR implements header-based enrichers and body logging that address the issue’s objectives.
Possibly related PRs
- Improve event routing and transaction handling in UoW and background … #28: Modifies background-job transaction/scheduling and touches JobDispatcher areas also changed here.
- Release v1.0 #29: Also adjusts JobDispatcher flow and unit-of-work handling similar to this PR.
- Refactor job dispatching and envelope handling logic #30: Related JobDispatcher and scheduler integration changes overlapping this PR.
Suggested labels
enhancement
Suggested reviewers
- darcoakk
- ukaratas
Poem
🐇 I hop through headers, bodies, and logs,
Capturing whispers and redacting the clogs,
Enrichers hum, jobs tidy their tracks,
Middleware listens and stitches it back,
A rabbit-approved telemetry bounce!
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title accurately describes the three main changes: HTTP body logging middleware addition, log enricher refactoring, and background job lifecycle improvements, which align directly with the changeset. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
- 📝 Generate docstrings (stacked PR)
- 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
48-telemetry-http-status-code-wildcard-headers-and-requestresponse-logging-configuration
📝 Coding Plan
- Generate coding plan for human review comments
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.
Comment @coderabbitai help to get the list of available commands and usage tips.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the application's observability and background job management. It introduces a new, configurable HTTP body logging middleware for detailed request/response inspection, refactors the log enrichment mechanism for broader and more consistent application across all logs, and improves background job reliability by adding audit timestamps and ensuring jobs are automatically removed from schedulers upon completion or failure. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In JobDispatcher.TryDeleteFromSchedulerAsync you reference
logger, but that symbol only exists as a local variable in DispatchAsync; consider using a class-level logger (e.g._logger) or passing the logger into the helper to avoid a compile-time error. - HttpBodyLoggingMiddleware.IsAllowedContentType allocates a new HashSet for allowed content types on every call; you can precompute this set once in the constructor to avoid per-request allocations.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In JobDispatcher.TryDeleteFromSchedulerAsync you reference `logger`, but that symbol only exists as a local variable in DispatchAsync; consider using a class-level logger (e.g. `_logger`) or passing the logger into the helper to avoid a compile-time error.
- HttpBodyLoggingMiddleware.IsAllowedContentType allocates a new HashSet for allowed content types on every call; you can precompute this set once in the constructor to avoid per-request allocations.
## Individual Comments
### Comment 1
<location path="framework/src/BBT.Aether.AspNetCore/BBT/Aether/AspNetCore/Telemetry/HttpBodyLoggingMiddleware.cs" line_range="199-175" />
<code_context>
+ return IsAllowedContentType(response.ContentType);
+ }
+
+ private bool IsAllowedContentType(string contentType)
+ {
+ var normalized = contentType.Split(';', 2)[0].Trim();
+ var allowed = new HashSet<string>(_bodyOptions.AllowedContentTypes ?? [], StringComparer.OrdinalIgnoreCase);
+
+ if (allowed.Contains(normalized))
+ return true;
+
+ if (normalized.EndsWith("+json", StringComparison.OrdinalIgnoreCase))
+ return true;
+
+ if (normalized.StartsWith("text/", StringComparison.OrdinalIgnoreCase))
+ return true;
+
+ return false;
+ }
+
</code_context>
<issue_to_address>
**suggestion (performance):** Avoid allocating a new HashSet on every IsAllowedContentType call.
`IsAllowedContentType` allocates a new `HashSet<string>` on every call from `_bodyOptions.AllowedContentTypes`. Since this may run per request (for both request/response), consider building the `HashSet<string>` once (e.g., in the constructor or lazily) and reusing it, so this method only normalizes the content type and checks membership plus the `+json`/`text/` heuristics, reducing GC pressure.
Suggested implementation:
```csharp
public sealed class HttpBodyLoggingMiddleware
{
private readonly RequestDelegate _next;
private HashSet<string>? _allowedContentTypes;
```
```csharp
private bool IsAllowedContentType(string contentType)
{
var normalized = contentType.Split(';', 2)[0].Trim();
var allowed = _allowedContentTypes ??= new HashSet<string>(
_bodyOptions.AllowedContentTypes ?? [],
StringComparer.OrdinalIgnoreCase);
if (allowed.Contains(normalized))
return true;
if (normalized.EndsWith("+json", StringComparison.OrdinalIgnoreCase))
return true;
if (normalized.StartsWith("text/", StringComparison.OrdinalIgnoreCase))
return true;
return false;
}
```
</issue_to_address>
### Comment 2
<location path="framework/docs/telemetry/README.md" line_range="272" />
<code_context>
+### Enrichers on All Logs
+
+`Telemetry:Logging:Enrichers` (CustomAttributes and Headers) are added as attributes to **every** log record in the application via `EnricherLogProcessor`. This allows querying all logs with the same enrich fields (e.g. `env`, `app`, `RequestHeader.x_correlation_id`) in a single observability query.
+
+- **CustomAttributes**: Key-value pairs added to every log.
</code_context>
<issue_to_address>
**issue (typo):** Consider rephrasing “same enrich fields” to “same enriched fields” or “same enrichment fields” for correct grammar.
The rest of the text is quite polished, so updating this phrase to “same enriched fields” or “same enrichment fields” (e.g., “This allows querying all logs with the same enriched fields …”) would improve clarity and grammatical correctness.
```suggestion
`Telemetry:Logging:Enrichers` (CustomAttributes and Headers) are added as attributes to **every** log record in the application via `EnricherLogProcessor`. This allows querying all logs with the same enriched fields (e.g. `env`, `app`, `RequestHeader.x_correlation_id`) in a single observability query.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| ### Enrichers on All Logs | ||
|
|
||
| `Telemetry:Logging:Enrichers` (CustomAttributes and Headers) are added as attributes to **every** log record in the application via `EnricherLogProcessor`. This allows querying all logs with the same enrich fields (e.g. `env`, `app`, `RequestHeader.x_correlation_id`) in a single observability query. |
There was a problem hiding this comment.
issue (typo): Consider rephrasing “same enrich fields” to “same enriched fields” or “same enrichment fields” for correct grammar.
The rest of the text is quite polished, so updating this phrase to “same enriched fields” or “same enrichment fields” (e.g., “This allows querying all logs with the same enriched fields …”) would improve clarity and grammatical correctness.
Suggested change
| `Telemetry:Logging:Enrichers` (CustomAttributes and Headers) are added as attributes to **every** log record in the application via `EnricherLogProcessor`. This allows querying all logs with the same enrich fields (e.g. `env`, `app`, `RequestHeader.x_correlation_id`) in a single observability query. | |
| `Telemetry:Logging:Enrichers` (CustomAttributes and Headers) are added as attributes to **every** log record in the application via `EnricherLogProcessor`. This allows querying all logs with the same enriched fields (e.g. `env`, `app`, `RequestHeader.x_correlation_id`) in a single observability query. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant improvements to telemetry with configurable HTTP body logging, a refactored log enricher, and enhanced background job handling. The changes are well-structured and add valuable functionality. My review includes suggestions to improve documentation consistency, optimize performance in the new middleware, enhance the robustness of string truncation, and resolve an issue with duplicate attribute enrichment in the logging setup.
| "EnableRequestBody": false, | ||
| "EnableResponseBody": false, | ||
| "MaxBodyLengthToCapture": 16384, | ||
| "AllowedContentTypes": ["application/json", "application/problem+json", "text/plain"], |
There was a problem hiding this comment.
The example for AllowedContentTypes is missing application/xml and text/xml, which are present in the default configuration in the code. To ensure the documentation is accurate and consistent with the default behavior, please update the example to include these missing content types.
Suggested change
| "AllowedContentTypes": ["application/json", "application/problem+json", "text/plain"], | |
| "AllowedContentTypes": ["application/json", "application/problem+json", "application/xml", "text/plain", "text/xml"], |
Comment on lines
240
to
241
| .AddAttributes(opts.Logging.Enrichers.CustomAttributes | ||
| .ToDictionary(x => x.Key, x => (object)x.Value))); |
There was a problem hiding this comment.
The CustomAttributes from Logging.Enrichers are being added here to the logging ResourceBuilder. However, the EnricherLogProcessor (added on line 248) also adds these same attributes to each log record. This will result in duplicate attributes on exported logs. To avoid this, you should remove the .AddAttributes(...) call for CustomAttributes from this ResourceBuilder configuration, as the EnricherLogProcessor is the more appropriate place for log-specific enrichment.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
framework/src/BBT.Aether.AspNetCore/BBT/Aether/AspNetCore/Telemetry/HttpBodyLoggingMiddleware.cs (2)
199-214: Consider caching theHashSetinIsAllowedContentType.A new
HashSetis allocated on every call toIsAllowedContentType, which is invoked for both request and response content-type checks on every processed request. Since_bodyOptions.AllowedContentTypesdoesn't change after construction, consider building this set once in the constructor.♻️ Suggested optimization
private readonly HashSet<string> _sensitiveJsonFields; private readonly HashSet<string> _sensitiveHeaderNames; +private readonly HashSet<string> _allowedContentTypes; public HttpBodyLoggingMiddleware(...) { // ... existing initialization ... _sensitiveHeaderNames = new HashSet<string>( HttpBodyLoggingOptions.DefaultSensitiveHeaderNames.Concat(_bodyOptions.AdditionalSensitiveHeaderNames ?? []), StringComparer.OrdinalIgnoreCase); + _allowedContentTypes = new HashSet<string>( + _bodyOptions.AllowedContentTypes ?? [], + StringComparer.OrdinalIgnoreCase); } private bool IsAllowedContentType(string contentType) { var normalized = contentType.Split(';', 2)[0].Trim(); - var allowed = new HashSet<string>(_bodyOptions.AllowedContentTypes ?? [], StringComparer.OrdinalIgnoreCase); - if (allowed.Contains(normalized)) + if (_allowedContentTypes.Contains(normalized)) return true; // ... rest unchanged ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@framework/src/BBT.Aether.AspNetCore/BBT/Aether/AspNetCore/Telemetry/HttpBodyLoggingMiddleware.cs` around lines 199 - 214, IsAllowedContentType currently allocates a new HashSet from _bodyOptions.AllowedContentTypes on every call; instead create a readonly cached HashSet (e.g., _allowedContentTypesSet) initialized once in the class constructor from _bodyOptions.AllowedContentTypes (handle null/empty safely and use StringComparer.OrdinalIgnoreCase), then update IsAllowedContentType to consult that cached set and keep the same +json and text/ checks.
385-393: UTF-8 truncation may produce malformed sequences.
Encoding.UTF8.GetString(bytes, 0, maxBytes)can produce malformed output ifmaxBytesfalls in the middle of a multi-byte character sequence. The decoder will insert replacement characters (�) for incomplete sequences.Consider using a character-based approach or finding a valid UTF-8 boundary:
♻️ Safe UTF-8 truncation
private static string TruncateUtf8(string value, int maxBytes) { var bytes = Encoding.UTF8.GetBytes(value); if (bytes.Length <= maxBytes) return value; - var truncated = Encoding.UTF8.GetString(bytes, 0, maxBytes); + // Find valid UTF-8 boundary by backing off from incomplete sequences + var truncateAt = maxBytes; + while (truncateAt > 0 && (bytes[truncateAt] & 0xC0) == 0x80) + { + truncateAt--; + } + var truncated = Encoding.UTF8.GetString(bytes, 0, truncateAt); return truncated + "…[TRUNCATED]"; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@framework/src/BBT.Aether.AspNetCore/BBT/Aether/AspNetCore/Telemetry/HttpBodyLoggingMiddleware.cs` around lines 385 - 393, The TruncateUtf8 method currently slices raw UTF-8 bytes which can split multi-byte characters and yield replacement chars; replace that logic by iterating the input string (method TruncateUtf8) and accumulating bytes per character using Encoding.UTF8.GetByteCount(char[]) (or GetByteCount of each char) until adding the next character would exceed maxBytes, then return the substring up to that char plus the "…[TRUNCATED]" marker. This preserves valid UTF-8 boundaries and avoids malformed sequences while keeping the same signature.framework/src/BBT.Aether.Domain/BBT/Aether/Domain/Entities/BackgroundJobInfo.cs (1)
29-29: UseIClockabstraction instead ofDateTime.UtcNowfor testability.Direct use of
DateTime.UtcNowbypasses theIClockabstraction used elsewhere in the codebase (e.g.,JobDispatcherusesclock.UtcNow). This breaks testability and consistency.Since entity constructors don't have access to DI, consider either:
- Removing this line and relying on the
AuditInterceptorto setCreatedAt(it checks for null and sets viaclock.UtcNow)- Passing a timestamp parameter from the caller that has access to
IClockAlso, minor formatting: there are two spaces before
DateTime.UtcNow.Option 1: Remove and rely on AuditInterceptor
public BackgroundJobInfo(Guid id, string handlerName, string jobName) : base(id) { HandlerName = handlerName; JobName = jobName; ExtraProperties = new ExtraPropertyDictionary(); - CreatedAt = DateTime.UtcNow; }Option 2: Accept timestamp from caller
- public BackgroundJobInfo(Guid id, string handlerName, string jobName) : base(id) + public BackgroundJobInfo(Guid id, string handlerName, string jobName, DateTime createdAt) : base(id) { HandlerName = handlerName; JobName = jobName; ExtraProperties = new ExtraPropertyDictionary(); - CreatedAt = DateTime.UtcNow; + CreatedAt = createdAt; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@framework/src/BBT.Aether.Domain/BBT/Aether/Domain/Entities/BackgroundJobInfo.cs` at line 29, BackgroundJobInfo currently sets CreatedAt using DateTime.UtcNow which bypasses the IClock abstraction and hurts testability; remove the "CreatedAt = DateTime.UtcNow" assignment in the BackgroundJobInfo constructor and rely on AuditInterceptor (which sets CreatedAt via IClock.UtcNow when null), or alternatively add an optional DateTimeOffset createdAt parameter to the BackgroundJobInfo constructor and have callers (e.g., callers with access to IClock/JobDispatcher) pass clock.UtcNow into that parameter; also fix the double-space before DateTime.UtcNow if keeping the line.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@framework/src/BBT.Aether.Infrastructure/BBT/Aether/BackgroundJob/EfCoreJobStore.cs`:
- Line 45: Replace the direct DateTime.UtcNow assignment to jobInfo.ModifiedAt
in EfCoreJobStore by injecting and using the IClock abstraction (add an IClock
_clock field and constructor parameter) and remove the manual assignment
entirely because BackgroundJobInfo is audited and AuditInterceptor sets
ModifiedAt; also remove the stray double space before the '=' in the existing
assignments (the occurrences around jobInfo.ModifiedAt and the other similar
assignment) so spacing matches project style; ensure JobDispatcher usage of
IClock remains consistent with this change.
---
Nitpick comments:
In
`@framework/src/BBT.Aether.AspNetCore/BBT/Aether/AspNetCore/Telemetry/HttpBodyLoggingMiddleware.cs`:
- Around line 199-214: IsAllowedContentType currently allocates a new HashSet
from _bodyOptions.AllowedContentTypes on every call; instead create a readonly
cached HashSet (e.g., _allowedContentTypesSet) initialized once in the class
constructor from _bodyOptions.AllowedContentTypes (handle null/empty safely and
use StringComparer.OrdinalIgnoreCase), then update IsAllowedContentType to
consult that cached set and keep the same +json and text/ checks.
- Around line 385-393: The TruncateUtf8 method currently slices raw UTF-8 bytes
which can split multi-byte characters and yield replacement chars; replace that
logic by iterating the input string (method TruncateUtf8) and accumulating bytes
per character using Encoding.UTF8.GetByteCount(char[]) (or GetByteCount of each
char) until adding the next character would exceed maxBytes, then return the
substring up to that char plus the "…[TRUNCATED]" marker. This preserves valid
UTF-8 boundaries and avoids malformed sequences while keeping the same
signature.
In
`@framework/src/BBT.Aether.Domain/BBT/Aether/Domain/Entities/BackgroundJobInfo.cs`:
- Line 29: BackgroundJobInfo currently sets CreatedAt using DateTime.UtcNow
which bypasses the IClock abstraction and hurts testability; remove the
"CreatedAt = DateTime.UtcNow" assignment in the BackgroundJobInfo constructor
and rely on AuditInterceptor (which sets CreatedAt via IClock.UtcNow when null),
or alternatively add an optional DateTimeOffset createdAt parameter to the
BackgroundJobInfo constructor and have callers (e.g., callers with access to
IClock/JobDispatcher) pass clock.UtcNow into that parameter; also fix the
double-space before DateTime.UtcNow if keeping the line.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cacd0b2e-6925-4fac-aa4a-5026976be6da
📒 Files selected for processing (10)
framework/docs/telemetry/README.mdframework/src/BBT.Aether.AspNetCore/BBT/Aether/AspNetCore/Telemetry/AetherLogEnricherProcessor.csframework/src/BBT.Aether.AspNetCore/BBT/Aether/AspNetCore/Telemetry/EnricherLogProcessor.csframework/src/BBT.Aether.AspNetCore/BBT/Aether/AspNetCore/Telemetry/HttpBodyLoggingMiddleware.csframework/src/BBT.Aether.AspNetCore/BBT/Aether/AspNetCore/Telemetry/TelemetryOptions.csframework/src/BBT.Aether.AspNetCore/Microsoft/AspNetCore/Builder/HttpBodyLoggingApplicationBuilderExtensions.csframework/src/BBT.Aether.AspNetCore/Microsoft/Extensions/DependencyInjection/AetherTelemetryServiceCollectionExtensions.csframework/src/BBT.Aether.Domain/BBT/Aether/Domain/Entities/BackgroundJobInfo.csframework/src/BBT.Aether.Infrastructure/BBT/Aether/BackgroundJob/EfCoreJobStore.csframework/src/BBT.Aether.Infrastructure/BBT/Aether/BackgroundJob/JobDispatcher.cs
💤 Files with no reviewable changes (1)
- framework/src/BBT.Aether.AspNetCore/BBT/Aether/AspNetCore/Telemetry/AetherLogEnricherProcessor.cs
| if (existingJob != null) | ||
| { | ||
| // Update existing job | ||
| jobInfo.ModifiedAt = DateTime.UtcNow; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if AuditInterceptor sets ModifiedAt for modified entities
ast-grep --pattern $'ApplyConceptsForModifiedEntity($$$) {
$$$
}'
# Also search for ModifiedAt handling in the interceptor
rg -n "ModifiedAt" --type cs -C 3 framework/src/BBT.Aether.Infrastructure/BBT/Aether/Domain/EntityFrameworkCore/Interceptors/Repository: burgan-tech/aether
Length of output: 1128
🏁 Script executed:
# First, let's see the full EfCoreJobStore.cs file (or at least the relevant sections)
wc -l "framework/src/BBT.Aether.Infrastructure/BBT/Aether/BackgroundJob/EfCoreJobStore.cs"Repository: burgan-tech/aether
Length of output: 148
🏁 Script executed:
# Read the EfCoreJobStore.cs file with context around lines 45 and 116
cat -n "framework/src/BBT.Aether.Infrastructure/BBT/Aether/BackgroundJob/EfCoreJobStore.cs" | head -150 | tail -110Repository: burgan-tech/aether
Length of output: 4680
🏁 Script executed:
# Find JobInfo class definition to check if it implements IHasModifyTime
rg -n "class JobInfo" -A 3 --type cs framework/src/BBT.Aether.Infrastructure/Repository: burgan-tech/aether
Length of output: 44
🏁 Script executed:
# Check if JobInfo or related background job entities implement IHasModifyTime
rg -n "IHasModifyTime" --type cs framework/src/BBT.Aether.Infrastructure/BBT/Aether/BackgroundJob/Repository: burgan-tech/aether
Length of output: 44
🏁 Script executed:
# Find BackgroundJobInfo class definition
fd -e cs | xargs rg -l "class BackgroundJobInfo" framework/src/BBT.Aether.Infrastructure/Repository: burgan-tech/aether
Length of output: 141
🏁 Script executed:
# Search for BackgroundJobInfo in the entire codebase
rg "BackgroundJobInfo" --type cs framework/src/BBT.Aether.Infrastructure/ | head -20Repository: burgan-tech/aether
Length of output: 2383
🏁 Script executed:
# Check if BackgroundJobInfo implements IHasModifyTime
rg -n "class BackgroundJobInfo" -A 5 --type csRepository: burgan-tech/aether
Length of output: 692
🏁 Script executed:
# Verify IClock exists and how it's used elsewhere
rg -n "interface IClock" --type cs framework/src/Repository: burgan-tech/aether
Length of output: 144
🏁 Script executed:
# Find FullAuditedEntity definition
rg -n "class FullAuditedEntity" -A 3 --type csRepository: burgan-tech/aether
Length of output: 2210
🏁 Script executed:
# Check the inheritance chain - look for IHasModifyTime
rg -n "IHasModifyTime" --type cs framework/src/Repository: burgan-tech/aether
Length of output: 1199
🏁 Script executed:
# See what FullAuditedEntity implements
rg -n "FullAuditedEntity" -B 2 -A 3 --type cs framework/src/BBT.Aether.Domain/ | head -30Repository: burgan-tech/aether
Length of output: 3356
🏁 Script executed:
# Check what IFullAuditedObject implements/extends
rg -n "interface IFullAuditedObject" -A 3 --type csRepository: burgan-tech/aether
Length of output: 447
🏁 Script executed:
# Check AuditedEntity to see if it implements IHasModifyTime
rg -n "class AuditedEntity" -A 3 --type cs framework/src/BBT.Aether.Domain/Repository: burgan-tech/aether
Length of output: 1607
🏁 Script executed:
# Check IModifyAuditedObject to understand the hierarchy
rg -n "interface IModifyAuditedObject" -A 3 --type csRepository: burgan-tech/aether
Length of output: 488
🏁 Script executed:
# Check IAuditedObject hierarchy
rg -n "interface IAuditedObject" -A 3 --type cs framework/src/BBT.Aether.Domain/Repository: burgan-tech/aether
Length of output: 437
🏁 Script executed:
# Check CreationAuditedEntity to see the full chain
rg -n "class CreationAuditedEntity" -A 3 --type cs framework/src/BBT.Aether.Domain/Repository: burgan-tech/aether
Length of output: 1132
Use IClock abstraction for consistency and testability.
Direct use of DateTime.UtcNow breaks consistency with the rest of the codebase (e.g., JobDispatcher uses injected IClock). Additionally, since BackgroundJobInfo implements IHasModifyTime through its inheritance chain (FullAuditedEntity → AuditedEntity → IAuditedObject → IModifyAuditedObject), the AuditInterceptor already handles setting ModifiedAt when the entity is modified—this manual assignment is redundant.
Also fix the double space before = on lines 45 and 116.
Inject IClock and use it for timestamps
public class EfCoreJobStore<TDbContext> : IJobStore
where TDbContext : DbContext, IHasEfCoreBackgroundJobs
{
private readonly TDbContext _dbContext;
+ private readonly IClock _clock;
- public EfCoreJobStore(TDbContext dbContext)
+ public EfCoreJobStore(TDbContext dbContext, IClock clock)
{
_dbContext = dbContext ?? throw new ArgumentNullException(nameof(dbContext));
+ _clock = clock ?? throw new ArgumentNullException(nameof(clock));
}Then replace both occurrences:
- jobInfo.ModifiedAt = DateTime.UtcNow;
+ jobInfo.ModifiedAt = _clock.UtcNow;- job.ModifiedAt = DateTime.UtcNow;
+ job.ModifiedAt = _clock.UtcNow;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@framework/src/BBT.Aether.Infrastructure/BBT/Aether/BackgroundJob/EfCoreJobStore.cs`
at line 45, Replace the direct DateTime.UtcNow assignment to jobInfo.ModifiedAt
in EfCoreJobStore by injecting and using the IClock abstraction (add an IClock
_clock field and constructor parameter) and remove the manual assignment
entirely because BackgroundJobInfo is audited and AuditInterceptor sets
ModifiedAt; also remove the stray double space before the '=' in the existing
assignments (the occurrences around jobInfo.ModifiedAt and the other similar
assignment) so spacing matches project style; ensure JobDispatcher usage of
IClock remains consistent with this change.
Initialize a cached, case-insensitive _allowedContentTypes HashSet from options in the constructor and use it in IsAllowedContentType to avoid allocating a new set on each call (retains the +json suffix handling). Rewrite TruncateUtf8 to check UTF-8 byte count before truncating and use an Encoding.UTF8 Decoder to avoid splitting multi-byte characters, appending "…[TRUNCATED]" when truncated.
|
❌ The last analysis has failed. |
Merged
9 tasks
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
1 participant
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.
Summary
This PR adds configurable HTTP request/response body logging to the telemetry stack, refactors the log enricher, and improves background job handling (audit timestamps and scheduler cleanup).
Telemetry
UseHttpBodyLogging()to log request/response bodies and headers per request. Configuration viaTelemetry:Logging:Body(EnableRequestBody/EnableResponseBody, MaxBodyLengthToCapture, AllowedContentTypes, path exclusion usesLogging.ExcludedPaths). Sensitive JSON fields and headers are redacted (built-in lists +AdditionalSensitiveJsonFields/AdditionalSensitiveHeaderNames).HttpBodyLoggingOptionsandLoggingEnricherOptionsadded toTelemetryOptions;Tracing.Headersdocumented as explicit list (no wildcard).AetherLogEnricherProcessorreplaced byEnricherLogProcessor; enrichers (CustomAttributes + Headers) apply to all log records and to the HTTP body log event scope, with duplicate enrichment avoided when the middleware already added headers.Logging.Body,Logging.Enrichers, and configuration examples.Background jobs
BackgroundJobInfosetsCreatedAtin the constructor;EfCoreJobStoresetsModifiedAton update and on status change.JobDispatchercallsIJobScheduler.DeleteAsyncwhen a job completes, fails, or is cancelled so the scheduler (e.g. Dapr) stops triggering it. Failures to delete are logged and do not affect DB state. Idempotency check refactored to use a single job fetch.Testing
Logging.BodyandUseHttpBodyLogging(); verify logs and redaction.CreatedAt/ModifiedAtand scheduler deletion where applicable.Made with Cursor
Summary by Sourcery
Add configurable HTTP request/response body logging to the telemetry stack, enhance logging/tracing enrichment options, and improve background job lifecycle handling and scheduler cleanup.
New Features:
Enhancements:
Documentation:
Summary by CodeRabbit
New Features
Improvements
Documentation