Added docs on routing access logs to log4j2#5953
Added docs on routing access logs to log4j2#5953himeshsiriwardana wants to merge 6 commits intowso2:masterfrom
Conversation
en/identity-server/6.1.0/docs/deploy/monitor/http-access-logging.md
Outdated
Show resolved
Hide resolved
|
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: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughTwo vocabulary entries ("appender" and "appenders") were added to Vale's accepted terms list. Concurrently, comprehensive documentation was added explaining how to configure HTTP access logging through Log4j2 in deployment, including two routing approaches: to rolling log files or console output. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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. 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: 1
🧹 Nitpick comments (1)
en/identity-server/6.1.0/docs/deploy/monitor/http-access-logging.md (1)
86-95: DocumentuseLoggeras a full configuration setting.This section explains what
useLoggerdoes, but it does not explicitly state the default value or allowed values. Please add that next to the example so readers can tell whether they are enabling new behavior or overriding the default.As per coding guidelines, "When documenting configuration, describe what the setting controls, state the default value, state constraints (type, valid range, allowed values), provide a minimal example, and explain when the user should change it."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@en/identity-server/6.1.0/docs/deploy/monitor/http-access-logging.md` around lines 86 - 95, The doc currently shows how to set useLogger under [http_access_log] but doesn't document its semantics; update the text around the example to state that useLogger is a boolean configuration (allowed values: true | false), that its default is false (Tomcat Access Log Valve writing to http_access.log), and that setting useLogger = true routes HTTP access logs via the Log4j2 logger; also mention where to change it (in <IS_HOME>/repository/conf/deployment.toml) and that corresponding log4j2.properties must be updated to add the HTTP access log appender and logger when enabling it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@en/identity-server/6.1.0/docs/deploy/monitor/http-access-logging.md`:
- Around line 143-147: The docs incorrectly instruct to set the top-level Log4j2
"appenders" registry to only CARBON_CONSOLE which breaks other appenders; revert
the "appenders" list to include all required appenders (e.g., CARBON_CONSOLE,
CARBON_LOGFILE, AUDIT_LOGFILE, etc.) so appender references used by loggers
remain available, and instead configure the HTTP_ACCESS logger to reference only
CARBON_CONSOLE (update the HTTP_ACCESS logger definition to use CARBON_CONSOLE
as its sole appender) so HTTP access logs go to console without removing other
appenders from the global registry.
---
Nitpick comments:
In `@en/identity-server/6.1.0/docs/deploy/monitor/http-access-logging.md`:
- Around line 86-95: The doc currently shows how to set useLogger under
[http_access_log] but doesn't document its semantics; update the text around the
example to state that useLogger is a boolean configuration (allowed values: true
| false), that its default is false (Tomcat Access Log Valve writing to
http_access.log), and that setting useLogger = true routes HTTP access logs via
the Log4j2 logger; also mention where to change it (in
<IS_HOME>/repository/conf/deployment.toml) and that corresponding
log4j2.properties must be updated to add the HTTP access log appender and logger
when enabling it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 122e4da6-68a6-4c99-9202-475c2149f18b
📒 Files selected for processing (2)
.vale/styles/config/vocabularies/vocab/accept.txten/identity-server/6.1.0/docs/deploy/monitor/http-access-logging.md
| Set `appenders` to `CARBON_CONSOLE` only: | ||
|
|
||
| ```properties | ||
| appenders = CARBON_CONSOLE | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Apache Log4j2 properties configuration, what does the top-level appenders = ... property control, and what happens if existing appenders referenced by other loggers are omitted from that list?
💡 Result:
In a Log4j2 log4j2.properties configuration, the top-level:
appenders = A, B, Cis a comma-separated list of appender identifiers that tells Log4j which appender.<id>.* blocks belong to the configuration (this listing was required prior to Log4j 2.6, and if the list is present, Log4j will use it). [1]
If an appender is referenced by a logger (via *.appenderRef.*.ref = ...) but its identifier is omitted from appenders = ..., that appender’s properties won’t be assembled/instantiated as part of the configuration, so the reference can’t be resolved—typically producing status errors like “Unable to locate appender … for logger config …”, and the affected logger simply won’t be able to write to that missing destination. [2][3]
Sources:
[1] Apache Log4j 2 manual – Properties configuration and the appenders identifier list behavior (logging.apache.org)
[2] Apache Log4j 2 manual – Appender references are how loggers route events to appenders (logging.apache.org)
[3] Example of the “Unable to locate appender … for logger config …” failure mode (support.stardog.com)
Citations:
- 1: https://logging.apache.org/log4j/2.12.x/manual/configuration.html
- 2: https://logging.apache.org/log4j/2.x/manual/configuration.html
- 3: https://support.stardog.com/support/solutions/articles/151000177175-log4j2-xml-error-unable-to-locate-appender-x-for-logger-config-y
Do not set appenders to only CARBON_CONSOLE.
In Log4j2 properties configuration, appenders is the top-level registry of all available appenders. Omitting CARBON_LOGFILE, AUDIT_LOGFILE, or other appenders from this list breaks the appender references that loggers use, causing those loggers to fail with "Unable to locate appender" errors and preventing file and audit logging.
To send HTTP access logs to the console only, keep the existing appenders list unchanged and instead configure the HTTP_ACCESS logger to use only CARBON_CONSOLE.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@en/identity-server/6.1.0/docs/deploy/monitor/http-access-logging.md` around
lines 143 - 147, The docs incorrectly instruct to set the top-level Log4j2
"appenders" registry to only CARBON_CONSOLE which breaks other appenders; revert
the "appenders" list to include all required appenders (e.g., CARBON_CONSOLE,
CARBON_LOGFILE, AUDIT_LOGFILE, etc.) so appender references used by loggers
remain available, and instead configure the HTTP_ACCESS logger to reference only
CARBON_CONSOLE (update the HTTP_ACCESS logger definition to use CARBON_CONSOLE
as its sole appender) so HTTP access logs go to console without removing other
appenders from the global registry.
|
@himeshsiriwardana Shall we add the doc to all the IS versions after 6.1.0 as well ? |
4 participants
Purpose
$subject
Related issues:
wso2/product-is#26884
Summary by CodeRabbit