search

Terracotta-OSS / terracotta-platform

http://terracotta.org
Apache License 2.0
32 stars 48 forks source link

Shortening the extension output which also appears in the help output #1019

Closed mathieucarbou closed 3 years ago

mathieucarbou commented 3 years ago

@myronkscott : I don't really like the fact that -help is polluted by the logging in INFO.

@tmesic99: as a workaround right now I have reduced the extension description (1 line instead of 4)

I have also refactored the implementations to support exposing more information, which I will use for next PR for TDB-5308

New help output

❯  ./kit/target/platform-kit-5.8-SNAPSHOT/platform-kit-5.8-SNAPSHOT/server/bin/start-tc-server.sh -help
2021-06-23 22:32:07,312 INFO - Terracotta 5.8.2-pre5, as of 2021-06-22 at 19:40:16 UTC (Revision f61e7ba47428e3bc5703c6071169755ed83b1ac2 from UNKNOWN)
2021-06-23 22:32:07,398 INFO - Extensions:
2021-06-23 22:32:07,411 INFO -  * Entity : Cluster Topology           5.8.0.SNAPSHOT  (built on 2021-06-24T02:30:30Z with JDK 1.8.0_282)
2021-06-23 22:32:07,412 INFO -  * Config : Dynamic                    5.8.0.SNAPSHOT  (built on 2021-06-24T02:30:30Z with JDK 1.8.0_282)
2021-06-23 22:32:07,412 INFO -  * Service: Client Message Tracker     5.8.0.SNAPSHOT  (built on 2021-06-24T02:30:30Z with JDK 1.8.0_282)
2021-06-23 22:32:07,412 INFO -  * Entity : Monitoring Agent           5.8.0.SNAPSHOT  (built on 2021-06-24T02:30:30Z with JDK 1.8.0_282)
2021-06-23 22:32:07,412 INFO -  * Plugin : Off-Heap                   5.8.0.SNAPSHOT  (built on 2021-06-24T02:30:30Z with JDK 1.8.0_282)
2021-06-23 22:32:07,412 INFO -  * Service: Dynamic Configuration      5.8.0.SNAPSHOT  (built on 2021-06-24T02:30:30Z with JDK 1.8.0_282)
2021-06-23 22:32:07,412 INFO -  * Entity : Monitoring Platform        5.8.0.SNAPSHOT  (built on 2021-06-24T02:30:30Z with JDK 1.8.0_282)
2021-06-23 22:32:07,412 INFO -  * Entity : Nomad                      5.8.0.SNAPSHOT  (built on 2021-06-24T02:30:30Z with JDK 1.8.0_282)
2021-06-23 22:32:07,412 INFO -  * Service: Monitoring                 5.8.0.SNAPSHOT  (built on 2021-06-24T02:30:30Z with JDK 1.8.0_282)
2021-06-23 22:32:07,412 INFO -  * Service: Configuration Monitoring   5.8.0.SNAPSHOT  (built on 2021-06-24T02:30:30Z with JDK 1.8.0_282)
2021-06-23 22:32:07,412 INFO -  * Service: Diagnostic Communication   5.8.0.SNAPSHOT  (built on 2021-06-24T02:30:30Z with JDK 1.8.0_282)
2021-06-23 22:32:07,412 INFO -  * Service: Server Information         5.8.0.SNAPSHOT  (built on 2021-06-24T02:30:30Z with JDK 1.8.0_282)
2021-06-23 22:32:07,412 INFO -  * Service: Lease                      5.8.0.SNAPSHOT  (built on 2021-06-24T02:30:30Z with JDK 1.8.0_282)
2021-06-23 22:32:07,422 INFO - PID is 28363
2021-06-23 22:32:07,813 INFO - 
    -audit-log-dir              security audit log directory
    -authc                      security authentication setting (file|ldap|certificate)
    -auto-activate              automatically activate the node so that it becomes active or joins a stripe (true|false)
    -backup-dir                 node backup directory
    -bind-address               node bind address for port
    -client-lease-duration      client lease duration
    -client-reconnect-window    client reconnect window
    -cluster-name               cluster name
    -config-dir                 node configuration directory
    -config-file                configuration properties file
    -data-dirs                  data directory
    -failover-priority          failover priority setting (availability|consistency)
    -group-bind-address         node bind address for group port
    -group-port                 node port used for intra-stripe communication
    -help                       provide usage information
    -hostname                   node host name
    -log-dir                    node log directory
    -metadata-dir               node metadata directory
    -name                       node name
    -offheap-resources          offheap resources
    -port                       node port
    -public-hostname            public node host name
    -public-port                public node port
    -repair-mode                node repair mode (true|false)
    -security-dir               security root directory
    -ssl-tls                    ssl-tls setting (true|false)
    -stripe-name                stripe name
    -tc-properties              tc-properties
    -whitelist                  security whitelist (true|false)

Allowed substitution parameters:
    %v    version of the operating system. Same as java 'os.version' property
    %h    host name of the machine
    %H    home directory of the user. Same as java 'user.home' property
    %i    IP address of the machine corresponding to localhost
    %n    username of the user. Same as java 'user.name' property
    %o    name of the operating system. Same as java 'os.name' property
    %a    architecture of the machine. Same as java 'os.arch' property
    %c    canonical host name of the machine
    %t    temporary directory of the machine. Same as java 'java.io.tmpdir' property
    %d    unique temporary directory
    %D    date stamp corresponding to current time

2021-06-23 22:32:07,814 INFO - providing usage information
2021-06-23 22:32:07,814 INFO - ExitState : CallbackOnExitState[Throwable: class org.terracotta.configuration.ConfigurationException; RestartNeeded: false]; AutoRestart: true
2021-06-23 22:32:07,815 INFO - Stopping server
myronkscott commented 3 years ago

@mathieucarbou One option might be to throw an IllegalStateException or some other RuntimeException from ServerConfiguration.getLogsLocation(). This will suppress all logging and write help to stdout directly. Let me know if you want this implemented this way.

mathieucarbou commented 3 years ago

@mathieucarbou One option might be to throw an IllegalStateException or some other RuntimeException from ServerConfiguration.getLogsLocation(). This will suppress all logging and write help to stdout directly. Let me know if you want this implemented this way.

IMO there shouldn't be any processing done at all when -help is on the CLI.

When this flag is there, we should exit immediately after displaying the user help and not do any processing at all especially not the logging since logging is supposed to be based on a potential user configuration (directory).

This is how all the other CLI work.

mathieucarbou commented 3 years ago

@mathieucarbou One option might be to throw an IllegalStateException or some other RuntimeException from ServerConfiguration.getLogsLocation(). This will suppress all logging and write help to stdout directly. Let me know if you want this implemented this way.

IMO there shouldn't be any processing done at all when -help is on the CLI.

When this flag is there, we should exit immediately after displaying the user help and not do any processing at all especially not the logging since logging is supposed to be based on a potential user configuration (directory).

This is how all the other CLI work.

@myronkscott : looking at the code, right now, -help triggers an early exception thrown from DynamicConfigConfigurationProvider in the initialize method:

https://github.com/Terracotta-OSS/terracotta-platform/blob/61149a1a98900b5de25f3113c26969afa64c9e3f/dynamic-config/server/configuration-provider/src/main/java/org/terracotta/dynamic_config/server/configuration/DynamicConfigConfigurationProvider.java#L106-L111

So we cannot even reach ServerConfiguration.getLogsLocation().

mathieucarbou commented 3 years ago

@mathieucarbou One option might be to throw an IllegalStateException or some other RuntimeException from ServerConfiguration.getLogsLocation(). This will suppress all logging and write help to stdout directly. Let me know if you want this implemented this way.

IMO there shouldn't be any processing done at all when -help is on the CLI. When this flag is there, we should exit immediately after displaying the user help and not do any processing at all especially not the logging since logging is supposed to be based on a potential user configuration (directory). This is how all the other CLI work.

@myronkscott : looking at the code, right now, -help triggers an early exception thrown from DynamicConfigConfigurationProvider in the initialize method:

https://github.com/Terracotta-OSS/terracotta-platform/blob/61149a1a98900b5de25f3113c26969afa64c9e3f/dynamic-config/server/configuration-provider/src/main/java/org/terracotta/dynamic_config/server/configuration/DynamicConfigConfigurationProvider.java#L106-L111

So we cannot even reach ServerConfiguration.getLogsLocation().

To correctly fix the problem, I would fix the ordering in the core Bootstrap class:

https://github.com/Terracotta-OSS/terracotta-core/blob/61dc22baf5677b4a10b6c1e43b81f5aee2e5b577/tc-server/src/main/java/com/tc/server/Bootstrap.java#L67-L91

IMO, we should have in order:

  1. CLI parsing and initialization=> call to setup.initialize()
  2. Information display and env setup => calls to writeVersion(...), writePID(), etc
  3. Startup => call to impl.start()

Moving 

writeVersion(setup.getProductInfo());
writePID();

just before 

writeSystemProperties();

would probably solve the issue ?

Right now, this is a sort of chicken and egg problem because some methods are extracting and display information from an "not-yet-initialized" environment where CLI parameters have not yet been parsed.