D52125 Execution Logger Fix#4321
Conversation
WalkthroughAdds a gateway accessor to GingerPlayEndPointManager and integrates it into HttpLogAppender's ApiUrl composition; refactors HttpLogAppender Append/ProcessQueue/activation behavior to snapshot event properties, handle exception-bearing events, reduce trailing newlines, and downgrade several error logs to debug. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant HttpLogAppender
participant EndPointMgr as GingerPlayEndPointManager
participant APIHandler as AccountReportApiHandler
Note over Caller,HttpLogAppender: ApiUrl setter (compose with gateway)
Caller->>HttpLogAppender: set ApiUrl(value)
HttpLogAppender->>HttpLogAppender: _apiUrl = value (pre-gateway)
HttpLogAppender->>APIHandler: init/ensure handler for pre-gateway _apiUrl
HttpLogAppender->>EndPointMgr: GetAccountReportServiceGateWay()
EndPointMgr-->>HttpLogAppender: ACCOUNT_REPORT_SERVICE_URL
HttpLogAppender->>HttpLogAppender: _apiUrl = pre-gateway + gateway
HttpLogAppender->>APIHandler: ensure handler for composed _apiUrl
Note over Caller,HttpLogAppender: Append flow (snapshot props, exception-aware)
Caller->>HttpLogAppender: Append(LoggingEvent e)
HttpLogAppender->>HttpLogAppender: snapshot Properties from e
alt e has Exception
HttpLogAppender->>HttpLogAppender: create new event with exception-aware ctor
else
HttpLogAppender->>HttpLogAppender: create new event with standard ctor
end
HttpLogAppender->>HttpLogAppender: copy snapshot properties to new event
HttpLogAppender->>APIHandler: enqueue rendered event (POST with single "\n")
APIHandler-->>HttpLogAppender: send result (success / retry / fail logged at DEBUG)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)Ginger/GingerCoreNET/log4netLib/HttpLogAppender.cs (2)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Ginger/GingerCoreNET/log4netLib/HttpLogAppender.cs (1)
41-49: Instantiate handler after appending the gateway URL
In ApiUrl’s setter, build the full URL first, then create the handler. For example, replace the current block with:_apiUrl = $"{value}{GingerPlayEndPointManager.GetAccoutReportServiceGateWay()}"; _accountReportApiHandler = string.IsNullOrWhiteSpace(_apiUrl) ? null : new AccountReportApiHandler(_apiUrl);This ensures the RestClient in AccountReportApiHandler is initialized with the correct full endpoint.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
Ginger/GingerCoreNET/External/GingerPlay/GingerPlayEndPointManager.cs(1 hunks)Ginger/GingerCoreNET/log4netLib/HttpLogAppender.cs(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-30T06:03:09.397Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4314
File: Ginger/GingerCoreNET/RunLib/DynamicExecutionLib/DynamicExecutionManager.cs:960-976
Timestamp: 2025-09-30T06:03:09.397Z
Learning: In GingerPlayDetails configuration mapping (file: Ginger/GingerCoreNET/RunLib/DynamicExecutionLib/DynamicExecutionManager.cs), both EnableAccountReportService and EnableHTMLReportService should be mapped from gingerPlayConfig.GingerPlayReportServiceEnabled as they represent the same underlying service.
Applied to files:
Ginger/GingerCoreNET/External/GingerPlay/GingerPlayEndPointManager.cs
🧬 Code graph analysis (1)
Ginger/GingerCoreNET/log4netLib/HttpLogAppender.cs (1)
Ginger/GingerCoreNET/External/GingerPlay/GingerPlayEndPointManager.cs (2)
GingerPlayEndPointManager(27-276)GetAccoutReportServiceGateWay(87-90)
🔇 Additional comments (1)
Ginger/GingerCoreNET/log4netLib/HttpLogAppender.cs (1)
213-213: Verify the log entry formatting change.The trailing newline has been reduced from two newlines to one, which will reduce spacing between log entries. Ensure this formatting change aligns with the expected log output format and doesn't impact log readability or parsing.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Ginger/GingerCoreNET/log4netLib/HttpLogAppender.cs (1)
43-48: Fix the URL composition order.The
AccountReportApiHandleris being initialized with an incomplete URL. Line 46 creates the handler using_apiUrl(which equalsvalue), but line 47 then appends the gateway segment to_apiUrl. This means the handler is created with the wrong URL.Additionally, there's no slash handling when concatenating
valueand the gateway segment, which could result in malformed URLs likehttps://example.comginger-report.Apply this diff to fix the logic:
if (string.IsNullOrWhiteSpace(_apiUrl) || (!string.IsNullOrWhiteSpace(value) && value != _apiUrl)) { - _apiUrl = value; - _accountReportApiHandler = string.IsNullOrWhiteSpace(_apiUrl) ? null : new AccountReportApiHandler(_apiUrl); - _apiUrl = $"{value}{GingerPlayEndPointManager.GetAccountReportServiceGateWay()}"; //Final Url with gateway + // Compose the final URL with gateway before creating the handler + var gateway = GingerPlayEndPointManager.GetAccountReportServiceGateWay(); + var baseUrl = value?.TrimEnd('/') ?? string.Empty; + var gatewaySegment = gateway?.TrimStart('/') ?? string.Empty; + _apiUrl = string.IsNullOrWhiteSpace(baseUrl) ? string.Empty : $"{baseUrl}/{gatewaySegment}"; + _accountReportApiHandler = string.IsNullOrWhiteSpace(_apiUrl) ? null : new AccountReportApiHandler(_apiUrl); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
Ginger/GingerCoreNET/External/GingerPlay/GingerPlayEndPointManager.cs(1 hunks)Ginger/GingerCoreNET/log4netLib/HttpLogAppender.cs(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-30T06:03:09.397Z
Learnt from: AmanPrasad43
PR: Ginger-Automation/Ginger#4314
File: Ginger/GingerCoreNET/RunLib/DynamicExecutionLib/DynamicExecutionManager.cs:960-976
Timestamp: 2025-09-30T06:03:09.397Z
Learning: In GingerPlayDetails configuration mapping (file: Ginger/GingerCoreNET/RunLib/DynamicExecutionLib/DynamicExecutionManager.cs), both EnableAccountReportService and EnableHTMLReportService should be mapped from gingerPlayConfig.GingerPlayReportServiceEnabled as they represent the same underlying service.
Applied to files:
Ginger/GingerCoreNET/External/GingerPlay/GingerPlayEndPointManager.cs
🧬 Code graph analysis (1)
Ginger/GingerCoreNET/log4netLib/HttpLogAppender.cs (1)
Ginger/GingerCoreNET/External/GingerPlay/GingerPlayEndPointManager.cs (2)
GingerPlayEndPointManager(27-276)GetAccountReportServiceGateWay(87-90)
🔇 Additional comments (1)
Ginger/GingerCoreNET/log4netLib/HttpLogAppender.cs (1)
213-213: LGTM!Reducing the trailing newlines from two to one improves log readability by reducing excessive whitespace between log entries.
Thank you for your contribution.
Before submitting this PR, please make sure:
Summary by CodeRabbit
New Features
Bug Fixes
Reliability