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

PAYARA-4037 Add access log setting options for Payara Micro #4344

Merged
merged 9 commits into from Nov 22, 2019

Conversation

@AlanRoth
Copy link
Contributor

AlanRoth commented Nov 22, 2019

Description

This is a feature for Payara Micro.

Introduces 3 new Micro Commands,
accessloginterval Sets the access log write-interval attribute
accesslogsuffix Sets the access log rotation-suffix attribute
accesslogprefix Sets a new virtual server property that will be used as the prefix

Testing

Testing Performed

Build tests

java -jar payara-micro --accesslog ~/temp/accesslog --accessloginterval 0 --unpackdir ~/temp
Check in domain.xml in ~/temp/config that write-interval-seconds has a value of 0

java -jar payara-micro --accesslog ~/temp/accesslog --accesslogsuffix ss --unpackdir ~/temp
Check in domain.xml in ~/temp/config that rotation-suffix is set to ss (seconds in simple date format)
Check in ~/temp/accesslog that accesslogs are named simmilar to; server_access_log_32

java -jar payara-micro --accesslog ~/temp/accesslog --accesslogprefix testy --unpackdir ~/temp
Check in ~/temp/accesslog that the file is named testy2019-11-22 or simmilar exists

java -jar payara-micro --accesslog ~/temp/accesslog --accesslogprefix testy --accesslogsuffix ss --unpackdir ~/temp
Check in ~/temp/accesslog that a file named testy32 or simmilar exists

Testing Environment

Manjaro 18.0.4
Maven 3.6.1
Zulu JDK 1.8.0_222

Documentation

Soon to follow

Notes for Reviewers

Feel free to mix and match the commands to make sure they are compatible, from testing the commands work with all existing commands

AlanRoth added 5 commits Nov 18, 2019
sync
@AlanRoth AlanRoth requested review from Pandrex247 and cubastanley Nov 22, 2019
@AlanRoth

This comment has been minimized.

Copy link
Contributor Author

AlanRoth commented Nov 22, 2019

jenkins test please

}

if (accessLogPrefix != null) {
props.setProperty("payaramicro.customlog", accessLogPrefix);

This comment has been minimized.

Copy link
@AlanRoth

AlanRoth Nov 22, 2019

Author Contributor
Suggested change
props.setProperty("payaramicro.customlog", accessLogPrefix);
props.setProperty("payaramicro.accessLogPrefix", accessLogPrefix);


String acFilePrefix = vsBean.getPropertyValue(Constants.ACCESS_LOG_PREFIX);
if(acFilePrefix != null && !acFilePrefix.trim().isEmpty() && !"null".equals(acFilePrefix) && !"null".equals(accessLogPrefix)) {

This comment has been minimized.

Copy link
@AlanRoth

AlanRoth Nov 22, 2019

Author Contributor

Hacky but does the job, caused some grief and NPE

@@ -72,7 +72,7 @@ public void multipleRecordTest() {
}

@Test
public void sinlgeRecordTest() {
public void singleRecordTest() {

This comment has been minimized.

Copy link
@AlanRoth

AlanRoth Nov 22, 2019

Author Contributor

Unrelated, just a typo fix

This comment has been minimized.

Copy link
@cubastanley

cubastanley Nov 22, 2019

Contributor

Was this change made as a codebase wide refactor and not just a renaming of the method in the class? Assuming yes but always worth asking ;)

This comment has been minimized.

Copy link
@AlanRoth

AlanRoth Nov 22, 2019

Author Contributor

No, but yes. I didn't check but luckily thats the only occurrence of the method

Copy link
Member

Pandrex247 left a comment

The suffix and prefix don't actually seem to be suffixes or prefixes - they overwrite stuff.

For example, original log file: server_access_log.2019-11-22.txt
With your options: testy07.txt

Surely it should be something more like testy.server_access_log.07.txt?

@AlanRoth

This comment has been minimized.

Copy link
Contributor Author

AlanRoth commented Nov 22, 2019

The suffix and prefix don't actually seem to be suffixes or prefixes - they overwrite stuff.

For example, original log file: server_access_log.2019-11-22.txt
With your options: testy07.txt

Surely it should be something more like testy.server_access_log.07.txt?

technically server_access_log is the default prefix or more specifically {virtualServer id} _access_log and 2019-11-22 being the suffix, I'm happy to change it though. The suffix is limited to simple date format from the get go by how access logging is implemented, I could edit it but changing the suffix wasn't requested but I added due to being easy enough to do

Copy link
Contributor

cubastanley left a comment

Also second Andrew's comment on the prefix matter - if it's meant to be a prefix the functionality should result in something along the lines of test_prefix-server_access_log.07.txt

EDIT: Given the comment you've just left above, I'd say retain the dot between the points i.e. testy.07.txt

@@ -72,7 +72,7 @@ public void multipleRecordTest() {
}

@Test
public void sinlgeRecordTest() {
public void singleRecordTest() {

This comment has been minimized.

Copy link
@cubastanley

cubastanley Nov 22, 2019

Contributor

Was this change made as a codebase wide refactor and not just a renaming of the method in the class? Assuming yes but always worth asking ;)

AlanRoth and others added 4 commits Nov 22, 2019
…fish/payara/micro/impl/PayaraMicroImpl.java

Co-Authored-By: Andrew Pielage <pandrex247@hotmail.com>
@AlanRoth

This comment has been minimized.

Copy link
Contributor Author

AlanRoth commented Nov 22, 2019

jenkins test please

@AlanRoth AlanRoth requested review from Pandrex247 and cubastanley Nov 22, 2019
@AlanRoth

This comment has been minimized.

Copy link
Contributor Author

AlanRoth commented Nov 22, 2019

jenkins test please

@Pandrex247 Pandrex247 merged commit ef0d18f into payara:master Nov 22, 2019
58 checks passed
58 checks passed
Payara Quick Build and Test Quick build and test passed!
Details
security/snyk - api/payara-api/pom.xml (payara-ci) No new issues
Details
security/snyk - api/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admin/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admingui/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ant-tasks/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/appclient/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/batch/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/common/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/concurrent/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/connectors/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/core/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ejb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/extras/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/featuresets/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ha/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/installer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/jdbc/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/jms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/load-balancer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/orb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/payara-appserver-modules/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/persistence/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/registration/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/security/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/tests/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/transaction/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/web/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/webservices/pom.xml (payara-ci) No new issues
Details
security/snyk - copyright/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/admin/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/cluster/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/common/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/core/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/diagnostics/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/hk2/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/payara-modules/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources-l10n/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/security/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/test-utils/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/tests/pom.xml (payara-ci) No new issues
Details
security/snyk - pom.xml (payara-ci) No new issues
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.