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

Asadmin should use STDOUT for communication with the user, not STDERR and the logging system #25169

Open
dmatej opened this issue Oct 3, 2024 · 0 comments
Assignees
Labels
breaking change Changes something users / app devs New feature A major new user functionality was added techdebt
Milestone

Comments

@dmatej
Copy link
Contributor

dmatej commented Oct 3, 2024

I have to analyze how many changes this means, but basically when somebody changes the formatter or level, user cannot use commands in script which parse the output.

Expectations:

  • --terse should produce minimalized parseable output, ie. osgi --session new should produce just the session id, no "command successful", no timestamp, no log level, just one new line after the id, done, regardless log format.
  • Without --terse, the command can also produce some logs using defined level and formatter. By default, should print INFO+ level messages to STDOUT, but without timestamps, levels, etc., like now; Btw, now prints FINE too, that sometimes creates a mess.
  • When using --terse and configured logfile, it should use that for all asadmin commands. The current state causes Launcher should not touch server.log #24065 .
  • When instance launcher fails, user should see somewhere why that happened.

Consider also some flexibility:

  • Using logging.properties (higher priority, should be possible to combine, partially), might be specified in asenv.conf as ASADMIN_LOGGING_CFG_FILE
  • Using just env variables and no logging.properties:
    • ASADMIN_LOG_FILE=${HOME}/asadmin.log,
    • ASADMIN_LOG_FILE_FORMATTER=org.glassfish.main.jul.formatter.ODLLogFormatter ,
    • ASADMIN_LOG_FILE_FORMAT=,
    • ASADMIN_LOG_FILE_ROLL_POLICY=hour/day/week/month/year/never,
    • ASADMIN_LOG_FILE_MAX_FILES=10,
    • ASADMIN_STDERR_FORMATTER=%m,
    • ASADMIN_STDERR_FORMAT=%m,
    • ASADMIN_STDERR_ENABLED - last two could be overridden by using --terse explicitly

Notes:

  • linux can separate STDOUT (for user) and STDERR (for developer), but most people use merged output. That is why we have --terse.
  • asadmin can work with multiple domains, instances, so the logging directory should not be the domain log dir (also because it can be deleted with the domain). We can use the home dir as a default, because it is on every operating system, however user should specify some well chosen directory.
  • About the breaking change label - it can affect some scripts, however probably just to simplify them, mostly they will work still the same way. We have to check the CI, TCK, and all we have if we can find some issues with these changes.
  • I already started touching that because of JDK loggers can initialize the JDK LogManager before GlassFishLogManager is set #25133 , but I want to separate this change from that one. They are related and they overlap (when we configure the log manager matters, now asadmin start-domain writes to server.log once, three phases each uses different logging config), but still can be split to standalone PRs.

Key classes to touch:

  • GFLauncherLogger
  • AdminMain.configureLogging
  • GlassFishMain - should log relevant parameters used to start its JVM (system options, env options, classpath used on command line). Then the launcher is not needed to do that.
@dmatej dmatej added New feature A major new user functionality was added techdebt breaking change Changes something users / app devs labels Oct 3, 2024
@dmatej dmatej added this to the 7.0.19 milestone Oct 3, 2024
@dmatej dmatej self-assigned this Oct 3, 2024
@dmatej dmatej modified the milestones: 7.0.19, 7.1.0 Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Changes something users / app devs New feature A major new user functionality was added techdebt
Projects
None yet
Development

No branches or pull requests

1 participant