Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WFLY-18581] Enable access logs in default configs #671

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ropalka
Copy link
Contributor

@ropalka ropalka commented Dec 3, 2024

https://issues.redhat.com/browse/WFLY-18581

Add analysis document

Resolves #670

@ropalka ropalka added the stability-level/default "Default" stability-level label Dec 3, 2024
@github-actions github-actions bot added stability-level/default "Default" stability-level and removed stability-level/default "Default" stability-level labels Dec 3, 2024
@github-actions github-actions bot added stability-level/default "Default" stability-level and removed stability-level/default "Default" stability-level labels Dec 4, 2024
@github-actions github-actions bot added stability-level/preview "Preview" stability level and removed stability-level/default "Default" stability-level labels Dec 4, 2024
@github-actions github-actions bot added stability-level/default "Default" stability-level and removed stability-level/preview "Preview" stability level labels Dec 4, 2024
@ropalka ropalka self-assigned this Dec 9, 2024
@ropalka ropalka changed the title [WFLY-18581] Enable access logs in default configs Enable access logs in default configs Dec 9, 2024
@ropalka ropalka changed the title Enable access logs in default configs [WFLY-18581] Enable access logs in default configs Dec 9, 2024
@jamezp
Copy link
Member

jamezp commented Feb 6, 2025

What is the performance impact of this?

@ropalka
Copy link
Contributor Author

ropalka commented Feb 7, 2025

The performance impact should be really minimal. For this particular proposal 'use-server-log' attribute defaults to 'false' [1] and that means AccessLogService [2] will use DefaultAccessLogReceiver [3] . DefaultAccessLogReceiver uses Executor framework for writing logs so the penalty is just queuing the log message in logging queue (plus later in different thread async write of the log message to the file system). The message queuing is the most important question here because it has direct impact on the HTTP request-response latency. Considering networking latency (HTTP message exchanges) vs. queuing log message to the logging queue latency, the impact should be IMHO unmeasurable.

[1] AccessLogService
[2] wildfly-undertow_14_0.xsd
[3] DefaultAccessLogReceiver
[4] JBossLoggingAccessLogReceiver

@TomasHofman
Copy link
Contributor

I have two concerns. I think none of them is critical, so just for your consideration...

Documentation

Looking through the docs living directly in the Wildfly repo, I didn't find anything that would specifically describe this functionality (I found a section on "Console Access Logging", which is related, but not exactly what is being enabled by this PR.

Googling, the most relevant page that describes this is the MasterTheBoss website (https://www.mastertheboss.com/web/jboss-web-server/how-do-you-configure-jboss-to-enable-http-logging/), and the WildScribe docs on wildfly.org (https://docs.wildfly.org/35/wildscribe/subsystem/undertow/server/host/setting/access-log/index.html).

So, considering this is quite basic and useful functionality, and as well that we are changing the default behavior, would it be worth it to add docs section specifically on "Access Logging", where we could at least mention that the access log is enabled by default and describe how to disable it and cover the basics?

Log rotation / retention time

Two questions in my head, not covered by the proposal doc, are:

  • "how large can the log file get?"
  • and "how long do the log data survive?"

That boils down to whether there is any log rotation strategy by default. Looking at Wildscribe it looks the logs are rotated daily (that's nice IMO), and are never removed (that may be less OK).

Could this cause a problem for people running busy Wildfly instances (running out of space?). Would it be reasonable to limit the amount of data logged in the default configuration?

The access-log resource doesn't seem to have capabilities to do any more complex rotation strategies, the mastertheboss page suggests to delegate access logging to the logging subsystem if this is needed.

If we don't change this aspect maybe just clarify the behavior in the proposal..?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stability-level/default "Default" stability-level
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Add access logs to default configurations
3 participants