v1.0.19#51
Merged
Merged
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
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.
…-wildcard-headers-and-requestresponse-logging-configuration feat(telemetry): HTTP body logging, enricher refactor, and background job improvements
Reviewer's GuideAdds configurable HTTP body logging and header-based enrichment to telemetry, exposes tracing/logging header configuration in options and docs, and improves background job handling by syncing scheduler state and tracking Created/Modified timestamps. Sequence diagram for HTTP request logging with body middleware and enricher processorsequenceDiagram
actor User
participant Browser
participant AspNetCoreApp as AspNetCore_App
participant HttpBodyMiddleware as HttpBodyLoggingMiddleware
participant NextMiddleware
participant Controller
participant Logger as ILogger
participant EnricherProcessor as EnricherLogProcessor
participant OtelExporter as OpenTelemetry_Exporter
User->>Browser: Make HTTP request
Browser->>AspNetCoreApp: HTTP request
AspNetCoreApp->>HttpBodyMiddleware: Invoke(HttpContext)
HttpBodyMiddleware->>HttpBodyMiddleware: ShouldProcess(path) and compile regex
alt Request body capture enabled
HttpBodyMiddleware->>HttpBodyMiddleware: CaptureRequestBodyAsync
end
HttpBodyMiddleware->>NextMiddleware: Invoke(HttpContext)
NextMiddleware->>Controller: Execute action
Controller-->>NextMiddleware: Response
NextMiddleware-->>HttpBodyMiddleware: Response
alt Response body capture enabled
HttpBodyMiddleware->>HttpBodyMiddleware: CaptureResponseBodyAsync
end
HttpBodyMiddleware->>Logger: LogInformation("HTTP request/response logged", scope with Method, RequestPath, StatusCode, RequestHeaders, ResponseHeaders, RequestBody*, ResponseBody*, Enrichers CustomAttributes + Headers)
Logger-->>AspNetCoreApp: LogRecord created
note over Logger,EnricherProcessor: OpenTelemetry logging pipeline processes LogRecord
Logger->>EnricherProcessor: OnEnd(LogRecord)
EnricherProcessor->>EnricherProcessor: Add CustomAttributes + header properties when not already enriched by middleware log
EnricherProcessor-->>Logger: Enriched LogRecord
Logger->>OtelExporter: Export enriched log to OTLP backend
OtelExporter-->>Logger: Ack
HttpBodyMiddleware-->>AspNetCoreApp: Response body restored
AspNetCoreApp-->>Browser: HTTP response
Browser-->>User: Show result
Sequence diagram for background job dispatch with scheduler deletionsequenceDiagram
participant Scheduler as ExternalScheduler
participant Dispatcher as JobDispatcher
participant Scope as DI_Scope
participant UowMgr as IUnitOfWorkManager
participant JobStore as IJobStore
participant SchedulerSvc as IJobScheduler
participant Handler as JobHandler
participant Db as Database
Scheduler->>Dispatcher: DispatchAsync(jobId, handlerName)
Dispatcher->>Scope: CreateScope()
Scope->>UowMgr: BeginRequiresNew()
Scope->>JobStore: GetAsync(jobId)
JobStore-->>Scope: BackgroundJobInfo jobInfo
alt jobInfo is null
Scope-->>Dispatcher: return
Dispatcher-->>Scheduler: exit
else jobInfo exists
Dispatcher->>Dispatcher: IsJobAlreadyProcessed(jobInfo)
alt already Completed or Cancelled
Scope->>SchedulerSvc: TryDeleteFromSchedulerAsync(handlerName, jobInfo.JobName)
SchedulerSvc->>Db: Delete scheduled job trigger
Db-->>SchedulerSvc: ok or error (logged, not thrown)
Scope-->>Dispatcher: Commit and return
Dispatcher-->>Scheduler: exit
else Pending
Scope->>JobStore: UpdateStatusAsync(jobId, Running)
Scope->>Db: Save changes
Scope-->>Dispatcher: Commit
Dispatcher->>Handler: Execute job handler
alt handler succeeds
Dispatcher->>UowMgr: BeginRequiresNew()
UowMgr->>JobStore: UpdateStatusAsync(jobId, Completed, handledTime)
UowMgr->>Db: Save changes
UowMgr-->>Dispatcher: Commit
Dispatcher->>SchedulerSvc: TryDeleteFromSchedulerAsync(handlerName, jobInfo.JobName)
else handler throws or cancelled
Dispatcher->>UowMgr: MarkJobStatusAsync(jobId, Failed or Cancelled)
UowMgr->>Db: Save changes
UowMgr-->>Dispatcher: Commit
Dispatcher->>SchedulerSvc: TryDeleteFromSchedulerAsync(handlerName, jobInfo.JobName)
end
end
end
SchedulerSvc->>Db: DeleteAsync(handlerName, jobName)
Db-->>SchedulerSvc: success or failure
SchedulerSvc-->>Dispatcher: no exception on failure (warning logged)
Class diagram for updated telemetry logging options and middlewareclassDiagram
class AetherTelemetryOptions {
bool TracingEnabled
bool MetricsEnabled
bool LoggingEnabled
AetherLoggingOptions Logging
AetherTracingOptions Tracing
}
class AetherLoggingOptions {
List~string~ ExcludedPaths
LoggingEnricherOptions Enrichers
HttpBodyLoggingOptions Body
bool EnableOtlpExporter
bool EnableConsoleExporter
bool IncludeFormattedMessage
bool IncludeScopes
bool ParseStateValues
}
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~ ExcludedPaths
List~string~ AdditionalSources
List~string~ Headers
}
class HttpBodyLoggingMiddleware {
<<sealed>>
-RequestDelegate _next
-ILogger~HttpBodyLoggingMiddleware~ _logger
-HttpBodyLoggingOptions _bodyOptions
-LoggingEnricherOptions _enricherOptions
-List~Regex~ _excludedPatterns
-HashSet~string~ _sensitiveJsonFields
-HashSet~string~ _sensitiveHeaderNames
-HashSet~string~ _allowedContentTypes
+HttpBodyLoggingMiddleware(RequestDelegate next, ILogger 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)
-static string HeadersToJson(IHeaderDictionary headers, HashSet~string~ sensitiveHeaderNames)
-static List~Regex~ CompileRegex(IEnumerable~string~ patterns)
-static string TruncateUtf8(string value, int maxBytes)
-static string RedactIfPossible(string text, string contentType, HashSet~string~ sensitiveJsonFields)
-static string RedactJson(string json, HashSet~string~ sensitiveJsonFields)
-static object RedactElement(JsonElement element, HashSet~string~ sensitiveJsonFields)
}
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 : uses
HttpBodyLoggingMiddleware --> LoggingEnricherOptions : uses
EnricherLogProcessor --> AetherTelemetryOptions : uses
EnricherLogProcessor --> HttpBodyLoggingOptions : uses defaults
HttpBodyLoggingApplicationBuilderExtensions --> HttpBodyLoggingMiddleware : adds middleware
Class diagram for updated background job processing and scheduler integrationclassDiagram
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 BackgroundJobStatus {
<<enum>>
Pending
Running
Completed
Cancelled
Failed
}
class IJobStore {
<<interface>>
Task SaveAsync(BackgroundJobInfo jobInfo, CancellationToken cancellationToken)
Task<BackgroundJobInfo> GetAsync(Guid id, CancellationToken cancellationToken)
Task UpdateStatusAsync(Guid id, BackgroundJobStatus status, DateTime? handledTime, string error, CancellationToken cancellationToken)
}
class EfCoreJobStore {
+Task SaveAsync(BackgroundJobInfo jobInfo, CancellationToken cancellationToken)
+Task UpdateStatusAsync(Guid id, BackgroundJobStatus status, DateTime? handledTime, string error, CancellationToken cancellationToken)
-DbContext _dbContext
}
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()
}
class JobDispatcher {
+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)
-ILogger logger
}
IJobStore <|.. EfCoreJobStore
EfCoreJobStore --> BackgroundJobInfo : persists
JobDispatcher --> IJobStore : uses
JobDispatcher --> IJobScheduler : uses
JobDispatcher --> IUnitOfWorkManager : uses
BackgroundJobInfo --> BackgroundJobStatus : has
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
⌛ 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
Note
|
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 introduces significant enhancements to the application's telemetry and background job processing capabilities. The telemetry system now offers advanced HTTP request/response body and header logging with sensitive data redaction, alongside a more robust and centralized log enrichment mechanism. Furthermore, improvements to background job management ensure better tracking of job lifecycle events and more reliable cleanup from job schedulers. These changes collectively aim to provide richer diagnostic data and more resilient background task execution. 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
HttpBodyLoggingMiddleware,CompileRegexwill throw during startup if anyLogging.ExcludedPathsentry is an invalid regex; consider handling per-pattern compilation failures (e.g. try/catch with a warning and skip) so a bad config value doesn’t take the whole app down. AddEnricherPropertiesinHttpBodyLoggingMiddlewaretakesrequestHeadersJsonandresponseHeadersJsonbut never uses them; you can drop these unused parameters (and corresponding call-site arguments) to simplify the method signature.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `HttpBodyLoggingMiddleware`, `CompileRegex` will throw during startup if any `Logging.ExcludedPaths` entry is an invalid regex; consider handling per-pattern compilation failures (e.g. try/catch with a warning and skip) so a bad config value doesn’t take the whole app down.
- `AddEnricherProperties` in `HttpBodyLoggingMiddleware` takes `requestHeadersJson` and `responseHeadersJson` but never uses them; you can drop these unused parameters (and corresponding call-site arguments) to simplify the method signature.
## Individual Comments
### Comment 1
<location path="framework/src/BBT.Aether.AspNetCore/BBT/Aether/AspNetCore/Telemetry/HttpBodyLoggingMiddleware.cs" line_range="432-436" />
<code_context>
+ }
+ }
+
+ private static object? RedactElement(JsonElement element, HashSet<string> sensitiveJsonFields)
+ {
+ return element.ValueKind switch
+ {
+ JsonValueKind.Object => element.EnumerateObject()
+ .ToDictionary(
+ p => p.Name,
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid potential exceptions when redacting JSON objects with duplicate property names.
`EnumerateObject().ToDictionary(...)` will throw if the JSON has duplicate property names, which JSON technically allows. That exception would trigger the `RedactJson` catch and skip redaction entirely. To avoid this, iterate and populate the dictionary manually (e.g., assigning keys so later properties overwrite earlier ones) instead of using `ToDictionary`, so duplicates don’t break redaction.
</issue_to_address>
### Comment 2
<location path="framework/src/BBT.Aether.AspNetCore/BBT/Aether/AspNetCore/Telemetry/EnricherLogProcessor.cs" line_range="62-63" />
<code_context>
+ if (enricherAttrs.Count == 0)
+ return;
+
+ var existing = record.Attributes ?? Array.Empty<KeyValuePair<string, object?>>();
+ var merged = new List<KeyValuePair<string, object?>>(existing.Count + enricherAttrs.Count);
+ foreach (var kv in existing)
+ merged.Add(kv);
</code_context>
<issue_to_address>
**suggestion (performance):** Use array Length instead of LINQ Count for existing attributes to avoid unnecessary overhead.
Since `existing` is always an array (`record.Attributes ?? Array.Empty<...>()`), `existing.Count` calls the LINQ extension and walks the array. Use `existing.Length` instead to avoid the extra allocation and O(n) overhead in this per-log-record path.
```suggestion
var existing = record.Attributes ?? Array.Empty<KeyValuePair<string, object?>>();
var merged = new List<KeyValuePair<string, object?>>(existing.Length + enricherAttrs.Count);
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+432
to
+436
| private static object? RedactElement(JsonElement element, HashSet<string> sensitiveJsonFields) | ||
| { | ||
| return element.ValueKind switch | ||
| { | ||
| JsonValueKind.Object => element.EnumerateObject() |
There was a problem hiding this comment.
issue (bug_risk): Avoid potential exceptions when redacting JSON objects with duplicate property names.
EnumerateObject().ToDictionary(...) will throw if the JSON has duplicate property names, which JSON technically allows. That exception would trigger the RedactJson catch and skip redaction entirely. To avoid this, iterate and populate the dictionary manually (e.g., assigning keys so later properties overwrite earlier ones) instead of using ToDictionary, so duplicates don’t break redaction.
Comment on lines
+62
to
+63
| var existing = record.Attributes ?? Array.Empty<KeyValuePair<string, object?>>(); | ||
| var merged = new List<KeyValuePair<string, object?>>(existing.Count + enricherAttrs.Count); |
There was a problem hiding this comment.
suggestion (performance): Use array Length instead of LINQ Count for existing attributes to avoid unnecessary overhead.
Since existing is always an array (record.Attributes ?? Array.Empty<...>()), existing.Count calls the LINQ extension and walks the array. Use existing.Length instead to avoid the extra allocation and O(n) overhead in this per-log-record path.
Suggested change
| var existing = record.Attributes ?? Array.Empty<KeyValuePair<string, object?>>(); | |
| var merged = new List<KeyValuePair<string, object?>>(existing.Count + enricherAttrs.Count); | |
| var existing = record.Attributes ?? Array.Empty<KeyValuePair<string, object?>>(); | |
| var merged = new List<KeyValuePair<string, object?>>(existing.Length + enricherAttrs.Count); |
![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 enhancements to the telemetry and background job handling systems. The new configurable HTTP header enrichment and optional body logging are well-documented and the implementation appears robust, especially with features like content redaction and stream handling. The improvements to the background job dispatcher, which now ensures jobs are removed from the scheduler upon completion or failure, are a great step towards a more resilient system. I've found one area for improvement regarding potential duplication in logging enrichment configuration.
Comment on lines
238
to
241
| logging.SetResourceBuilder(ResourceBuilder.CreateDefault() | ||
| .AddService(opts.ServiceName!, opts.ServiceNamespace ?? "aether", opts.ServiceVersion ?? "1.0.0", false, Environment.MachineName) | ||
| .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 to the logging ResourceBuilder here, while they are also added to each log record by the EnricherLogProcessor (registered on line 247). This results in duplicating these attributes on every log record.
Since EnricherLogProcessor is designed to enrich all log records with these attributes, adding them to the resource builder is redundant. To avoid this duplication, the .AddAttributes(...) call should be removed.
logging.SetResourceBuilder(ResourceBuilder.CreateDefault()
.AddService(opts.ServiceName!, opts.ServiceNamespace ?? "aether", opts.ServiceVersion ?? "1.0.0", false, Environment.MachineName));
|
❌ The last analysis has failed. |
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 by Sourcery
Add configurable HTTP header-based telemetry enrichment and optional HTTP body logging, and improve background job lifecycle handling.
New Features:
Bug Fixes:
Enhancements:
Documentation: