From 9ea130679e3e3fcefe39c3950ef69b6794f2b429 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Wed, 20 Nov 2024 14:24:59 +0100 Subject: [PATCH 1/4] feature: Log BUILD STREET GRAPH, BUILD TRANSIT GRAPH, and RUN PLANNER in OTP startup/shutdown messages." --- .../org/opentripplanner/standalone/OTPMain.java | 2 +- .../standalone/OtpStartupInfo.java | 11 ++++++++--- .../standalone/config/CommandLineParameters.java | 15 +++++++++++++++ .../config/CommandLineParametersTest.java | 10 ++++++++++ .../transit/speed_test/SpeedTest.java | 2 +- 5 files changed, 35 insertions(+), 5 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/standalone/OTPMain.java b/application/src/main/java/org/opentripplanner/standalone/OTPMain.java index a8096d806ba..06b0fcc010c 100644 --- a/application/src/main/java/org/opentripplanner/standalone/OTPMain.java +++ b/application/src/main/java/org/opentripplanner/standalone/OTPMain.java @@ -52,7 +52,7 @@ public static void main(String[] args) { try { Thread.currentThread().setName("main"); CommandLineParameters params = parseAndValidateCmdLine(args); - OtpStartupInfo.logInfo(); + OtpStartupInfo.logInfo(params.logInfo()); startOTPServer(params); } catch (OtpAppException ae) { LOG.error(ae.getMessage(), ae); diff --git a/application/src/main/java/org/opentripplanner/standalone/OtpStartupInfo.java b/application/src/main/java/org/opentripplanner/standalone/OtpStartupInfo.java index c601846d889..beae898d678 100644 --- a/application/src/main/java/org/opentripplanner/standalone/OtpStartupInfo.java +++ b/application/src/main/java/org/opentripplanner/standalone/OtpStartupInfo.java @@ -34,13 +34,18 @@ private static String info() { ); } - public static void logInfo() { + public static void logInfo(String runInfo) { // This is good when aggregating logs across multiple load balanced instances of OTP // Hint: a regexp filter like "^OTP (START|SHUTTING)" will list nodes going up/down - LOG.info("OTP STARTING UP ({}) using Java {}", projectInfo().getVersionString(), javaVersion()); + LOG.info( + "OTP STARTING UP - {} - {} - Java {}", + runInfo, + projectInfo().getVersionString(), + javaVersion() + ); ApplicationShutdownSupport.addShutdownHook( "server-shutdown-info", - () -> LOG.info("OTP SHUTTING DOWN ({})", projectInfo().getVersionString()) + () -> LOG.info("OTP SHUTTING DOWN - {} - {}", runInfo, projectInfo().getVersionString()) ); LOG.info(NEW_LINE + "{}", info()); } diff --git a/application/src/main/java/org/opentripplanner/standalone/config/CommandLineParameters.java b/application/src/main/java/org/opentripplanner/standalone/config/CommandLineParameters.java index 3513ec252ea..c089ff96a74 100644 --- a/application/src/main/java/org/opentripplanner/standalone/config/CommandLineParameters.java +++ b/application/src/main/java/org/opentripplanner/standalone/config/CommandLineParameters.java @@ -201,6 +201,21 @@ public boolean doServe() { return load || (serve && doBuildTransit()); } + public String logInfo() { + var mainCommands = new ArrayList(); + if (doBuildStreet() & doBuildTransit()) { + mainCommands.add("BUILD STREET & TRANSIT GRAPH"); + } else if (doBuildStreet()) { + mainCommands.add("BUILD STREET GRAPH"); + } else if (doBuildTransit()) { + mainCommands.add("BUILD TRANSIT GRAPH"); + } + if (doServe()) { + mainCommands.add("RUN PLANNER"); + } + return String.join(", ", mainCommands); + } + /** * @param port a port that we plan to bind to * @throws ParameterException if that port is not available diff --git a/application/src/test/java/org/opentripplanner/standalone/config/CommandLineParametersTest.java b/application/src/test/java/org/opentripplanner/standalone/config/CommandLineParametersTest.java index 82c3589202c..dc30e80e587 100644 --- a/application/src/test/java/org/opentripplanner/standalone/config/CommandLineParametersTest.java +++ b/application/src/test/java/org/opentripplanner/standalone/config/CommandLineParametersTest.java @@ -41,6 +41,8 @@ public void everyThingOffByDefault() { assertFalse(subject.doLoadStreetGraph()); assertFalse(subject.doSaveGraph()); assertFalse(subject.doServe()); + var ex = assertThrows(ParameterException.class, () -> subject.inferAndValidate()); + assertEquals("Nothing to do. Use --help to see available options.", ex.getMessage()); } @Test @@ -50,6 +52,7 @@ public void build() { assertTrue(subject.doBuildTransit()); assertFalse(subject.doSaveGraph()); assertFalse(subject.doServe()); + assertEquals("BUILD STREET & TRANSIT GRAPH", subject.logInfo()); subject.save = true; subject.serve = false; @@ -58,6 +61,7 @@ public void build() { assertTrue(subject.doSaveGraph()); assertFalse(subject.doServe()); subject.inferAndValidate(); + assertEquals("BUILD STREET & TRANSIT GRAPH", subject.logInfo()); subject.save = false; subject.serve = true; @@ -66,6 +70,7 @@ public void build() { assertFalse(subject.doSaveGraph()); assertTrue(subject.doServe()); subject.inferAndValidate(); + assertEquals("BUILD STREET & TRANSIT GRAPH, RUN PLANNER", subject.logInfo()); subject.save = true; subject.serve = true; @@ -74,6 +79,7 @@ public void build() { assertTrue(subject.doSaveGraph()); assertTrue(subject.doServe()); subject.inferAndValidate(); + assertEquals("BUILD STREET & TRANSIT GRAPH, RUN PLANNER", subject.logInfo()); } @Test @@ -83,6 +89,7 @@ public void buildStreet() { assertFalse(subject.doBuildTransit()); assertTrue(subject.doSaveStreetGraph()); assertFalse(subject.doSaveGraph()); + assertEquals("BUILD STREET GRAPH", subject.logInfo()); } @Test @@ -90,6 +97,7 @@ public void doLoadGraph() { subject.load = true; assertTrue(subject.doLoadGraph()); assertTrue(subject.doServe()); + assertEquals("RUN PLANNER", subject.logInfo()); } @Test @@ -99,6 +107,7 @@ public void doLoadStreetGraph() { assertFalse(subject.doBuildStreet()); assertFalse(subject.doSaveStreetGraph()); assertFalse(subject.doSaveGraph()); + assertEquals("BUILD TRANSIT GRAPH", subject.logInfo()); subject.save = true; subject.serve = true; @@ -119,6 +128,7 @@ public void validateLoad() { // Implicit given, but should be ok to set subject.serve = true; + assertEquals("RUN PLANNER", subject.logInfo()); // No exception thrown subject.inferAndValidate(); diff --git a/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java b/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java index f0e87b046c5..e3e77e6d972 100644 --- a/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java +++ b/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java @@ -144,7 +144,7 @@ public TestStatus status() { public static void main(String[] args) { try { - OtpStartupInfo.logInfo(); + OtpStartupInfo.logInfo("Run SpeedTest"); // Given the following setup SpeedTestCmdLineOpts opts = new SpeedTestCmdLineOpts(args); var config = SpeedTestConfig.config(opts.rootDir()); From fb650bac86177dc7032c9a4d39acb14c77b78525 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 21 Nov 2024 16:54:14 +0100 Subject: [PATCH 2/4] review: Change the case for the commands logged --- .../standalone/config/CommandLineParameters.java | 8 ++++---- .../config/CommandLineParametersTest.java | 16 ++++++++-------- .../transit/speed_test/SpeedTest.java | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/standalone/config/CommandLineParameters.java b/application/src/main/java/org/opentripplanner/standalone/config/CommandLineParameters.java index c089ff96a74..00fc7c75250 100644 --- a/application/src/main/java/org/opentripplanner/standalone/config/CommandLineParameters.java +++ b/application/src/main/java/org/opentripplanner/standalone/config/CommandLineParameters.java @@ -204,14 +204,14 @@ public boolean doServe() { public String logInfo() { var mainCommands = new ArrayList(); if (doBuildStreet() & doBuildTransit()) { - mainCommands.add("BUILD STREET & TRANSIT GRAPH"); + mainCommands.add("Build Street & Transit Graph"); } else if (doBuildStreet()) { - mainCommands.add("BUILD STREET GRAPH"); + mainCommands.add("Build Street Graph"); } else if (doBuildTransit()) { - mainCommands.add("BUILD TRANSIT GRAPH"); + mainCommands.add("Build Transit Graph"); } if (doServe()) { - mainCommands.add("RUN PLANNER"); + mainCommands.add("Run Server"); } return String.join(", ", mainCommands); } diff --git a/application/src/test/java/org/opentripplanner/standalone/config/CommandLineParametersTest.java b/application/src/test/java/org/opentripplanner/standalone/config/CommandLineParametersTest.java index dc30e80e587..fd2cb852444 100644 --- a/application/src/test/java/org/opentripplanner/standalone/config/CommandLineParametersTest.java +++ b/application/src/test/java/org/opentripplanner/standalone/config/CommandLineParametersTest.java @@ -52,7 +52,7 @@ public void build() { assertTrue(subject.doBuildTransit()); assertFalse(subject.doSaveGraph()); assertFalse(subject.doServe()); - assertEquals("BUILD STREET & TRANSIT GRAPH", subject.logInfo()); + assertEquals("Build Street & Transit Graph", subject.logInfo()); subject.save = true; subject.serve = false; @@ -61,7 +61,7 @@ public void build() { assertTrue(subject.doSaveGraph()); assertFalse(subject.doServe()); subject.inferAndValidate(); - assertEquals("BUILD STREET & TRANSIT GRAPH", subject.logInfo()); + assertEquals("Build Street & Transit Graph", subject.logInfo()); subject.save = false; subject.serve = true; @@ -70,7 +70,7 @@ public void build() { assertFalse(subject.doSaveGraph()); assertTrue(subject.doServe()); subject.inferAndValidate(); - assertEquals("BUILD STREET & TRANSIT GRAPH, RUN PLANNER", subject.logInfo()); + assertEquals("Build Street & Transit Graph, Run Server", subject.logInfo()); subject.save = true; subject.serve = true; @@ -79,7 +79,7 @@ public void build() { assertTrue(subject.doSaveGraph()); assertTrue(subject.doServe()); subject.inferAndValidate(); - assertEquals("BUILD STREET & TRANSIT GRAPH, RUN PLANNER", subject.logInfo()); + assertEquals("Build Street & Transit Graph, Run Server", subject.logInfo()); } @Test @@ -89,7 +89,7 @@ public void buildStreet() { assertFalse(subject.doBuildTransit()); assertTrue(subject.doSaveStreetGraph()); assertFalse(subject.doSaveGraph()); - assertEquals("BUILD STREET GRAPH", subject.logInfo()); + assertEquals("Build Street Graph", subject.logInfo()); } @Test @@ -97,7 +97,7 @@ public void doLoadGraph() { subject.load = true; assertTrue(subject.doLoadGraph()); assertTrue(subject.doServe()); - assertEquals("RUN PLANNER", subject.logInfo()); + assertEquals("Run Server", subject.logInfo()); } @Test @@ -107,7 +107,7 @@ public void doLoadStreetGraph() { assertFalse(subject.doBuildStreet()); assertFalse(subject.doSaveStreetGraph()); assertFalse(subject.doSaveGraph()); - assertEquals("BUILD TRANSIT GRAPH", subject.logInfo()); + assertEquals("Build Transit Graph", subject.logInfo()); subject.save = true; subject.serve = true; @@ -128,7 +128,7 @@ public void validateLoad() { // Implicit given, but should be ok to set subject.serve = true; - assertEquals("RUN PLANNER", subject.logInfo()); + assertEquals("Run Server", subject.logInfo()); // No exception thrown subject.inferAndValidate(); diff --git a/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java b/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java index e3e77e6d972..e5e305d8081 100644 --- a/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java +++ b/application/src/test/java/org/opentripplanner/transit/speed_test/SpeedTest.java @@ -144,7 +144,7 @@ public TestStatus status() { public static void main(String[] args) { try { - OtpStartupInfo.logInfo("Run SpeedTest"); + OtpStartupInfo.logInfo("Run Speed Test"); // Given the following setup SpeedTestCmdLineOpts opts = new SpeedTestCmdLineOpts(args); var config = SpeedTestConfig.config(opts.rootDir()); From 1e02e113f6ee107fc2c498dff800049d8835d3c8 Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 21 Nov 2024 17:07:16 +0100 Subject: [PATCH 3/4] refactor: Improve doc and logged version text. --- .../model/projectinfo/OtpProjectInfo.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/model/projectinfo/OtpProjectInfo.java b/application/src/main/java/org/opentripplanner/model/projectinfo/OtpProjectInfo.java index 37651d4522f..c49fbc1ae4f 100644 --- a/application/src/main/java/org/opentripplanner/model/projectinfo/OtpProjectInfo.java +++ b/application/src/main/java/org/opentripplanner/model/projectinfo/OtpProjectInfo.java @@ -80,7 +80,7 @@ public String toString() { * dev-2.x} */ public String getVersionString() { - String format = "version: %s, ser.ver.id: %s, commit: %s, branch: %s"; + String format = "Version: %s, ser.ver.id: %s, commit: %s, branch: %s"; return String.format( format, version.version, @@ -91,8 +91,8 @@ public String getVersionString() { } /** - * This method compare the maven project version, an return {@code true} if both are the same. Two - * different SNAPSHOT versions are considered the same - work in progress. + * This method compares the maven project version, and return {@code true} if both are the same. + * Two different SNAPSHOT versions are considered the same version - they are work in progress. */ public boolean sameVersion(OtpProjectInfo other) { return this.version.sameVersion(other.version); @@ -100,8 +100,8 @@ public boolean sameVersion(OtpProjectInfo other) { /** * The OTP Serialization version id is used to determine if OTP and a serialized blob(Graph.obj) - * of the otp internal model are compatible. This filed is writen into the Graph.obj file header - * and checked when loading the graph later. + * of the otp internal model are compatible. This field is written into the Graph.obj + * file header and checked when loading the graph later. */ public String getOtpSerializationVersionId() { return graphFileHeaderInfo.otpSerializationVersionId(); From f7f2b93a54358acdcd711303aac9747611ade2bc Mon Sep 17 00:00:00 2001 From: Thomas Gran Date: Thu, 21 Nov 2024 17:11:40 +0100 Subject: [PATCH 4/4] refactor: Improve cli task info variable name --- .../org/opentripplanner/standalone/OTPMain.java | 2 +- .../standalone/OtpStartupInfo.java | 6 +++--- .../standalone/config/CommandLineParameters.java | 2 +- .../config/CommandLineParametersTest.java | 16 ++++++++-------- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/application/src/main/java/org/opentripplanner/standalone/OTPMain.java b/application/src/main/java/org/opentripplanner/standalone/OTPMain.java index 06b0fcc010c..a10f8f8ef7b 100644 --- a/application/src/main/java/org/opentripplanner/standalone/OTPMain.java +++ b/application/src/main/java/org/opentripplanner/standalone/OTPMain.java @@ -52,7 +52,7 @@ public static void main(String[] args) { try { Thread.currentThread().setName("main"); CommandLineParameters params = parseAndValidateCmdLine(args); - OtpStartupInfo.logInfo(params.logInfo()); + OtpStartupInfo.logInfo(params.logTaskInfo()); startOTPServer(params); } catch (OtpAppException ae) { LOG.error(ae.getMessage(), ae); diff --git a/application/src/main/java/org/opentripplanner/standalone/OtpStartupInfo.java b/application/src/main/java/org/opentripplanner/standalone/OtpStartupInfo.java index beae898d678..ddcc2c60212 100644 --- a/application/src/main/java/org/opentripplanner/standalone/OtpStartupInfo.java +++ b/application/src/main/java/org/opentripplanner/standalone/OtpStartupInfo.java @@ -34,18 +34,18 @@ private static String info() { ); } - public static void logInfo(String runInfo) { + public static void logInfo(String cliTaskInfo) { // This is good when aggregating logs across multiple load balanced instances of OTP // Hint: a regexp filter like "^OTP (START|SHUTTING)" will list nodes going up/down LOG.info( "OTP STARTING UP - {} - {} - Java {}", - runInfo, + cliTaskInfo, projectInfo().getVersionString(), javaVersion() ); ApplicationShutdownSupport.addShutdownHook( "server-shutdown-info", - () -> LOG.info("OTP SHUTTING DOWN - {} - {}", runInfo, projectInfo().getVersionString()) + () -> LOG.info("OTP SHUTTING DOWN - {} - {}", cliTaskInfo, projectInfo().getVersionString()) ); LOG.info(NEW_LINE + "{}", info()); } diff --git a/application/src/main/java/org/opentripplanner/standalone/config/CommandLineParameters.java b/application/src/main/java/org/opentripplanner/standalone/config/CommandLineParameters.java index 00fc7c75250..2324ce325d9 100644 --- a/application/src/main/java/org/opentripplanner/standalone/config/CommandLineParameters.java +++ b/application/src/main/java/org/opentripplanner/standalone/config/CommandLineParameters.java @@ -201,7 +201,7 @@ public boolean doServe() { return load || (serve && doBuildTransit()); } - public String logInfo() { + public String logTaskInfo() { var mainCommands = new ArrayList(); if (doBuildStreet() & doBuildTransit()) { mainCommands.add("Build Street & Transit Graph"); diff --git a/application/src/test/java/org/opentripplanner/standalone/config/CommandLineParametersTest.java b/application/src/test/java/org/opentripplanner/standalone/config/CommandLineParametersTest.java index fd2cb852444..0551d9e7056 100644 --- a/application/src/test/java/org/opentripplanner/standalone/config/CommandLineParametersTest.java +++ b/application/src/test/java/org/opentripplanner/standalone/config/CommandLineParametersTest.java @@ -52,7 +52,7 @@ public void build() { assertTrue(subject.doBuildTransit()); assertFalse(subject.doSaveGraph()); assertFalse(subject.doServe()); - assertEquals("Build Street & Transit Graph", subject.logInfo()); + assertEquals("Build Street & Transit Graph", subject.logTaskInfo()); subject.save = true; subject.serve = false; @@ -61,7 +61,7 @@ public void build() { assertTrue(subject.doSaveGraph()); assertFalse(subject.doServe()); subject.inferAndValidate(); - assertEquals("Build Street & Transit Graph", subject.logInfo()); + assertEquals("Build Street & Transit Graph", subject.logTaskInfo()); subject.save = false; subject.serve = true; @@ -70,7 +70,7 @@ public void build() { assertFalse(subject.doSaveGraph()); assertTrue(subject.doServe()); subject.inferAndValidate(); - assertEquals("Build Street & Transit Graph, Run Server", subject.logInfo()); + assertEquals("Build Street & Transit Graph, Run Server", subject.logTaskInfo()); subject.save = true; subject.serve = true; @@ -79,7 +79,7 @@ public void build() { assertTrue(subject.doSaveGraph()); assertTrue(subject.doServe()); subject.inferAndValidate(); - assertEquals("Build Street & Transit Graph, Run Server", subject.logInfo()); + assertEquals("Build Street & Transit Graph, Run Server", subject.logTaskInfo()); } @Test @@ -89,7 +89,7 @@ public void buildStreet() { assertFalse(subject.doBuildTransit()); assertTrue(subject.doSaveStreetGraph()); assertFalse(subject.doSaveGraph()); - assertEquals("Build Street Graph", subject.logInfo()); + assertEquals("Build Street Graph", subject.logTaskInfo()); } @Test @@ -97,7 +97,7 @@ public void doLoadGraph() { subject.load = true; assertTrue(subject.doLoadGraph()); assertTrue(subject.doServe()); - assertEquals("Run Server", subject.logInfo()); + assertEquals("Run Server", subject.logTaskInfo()); } @Test @@ -107,7 +107,7 @@ public void doLoadStreetGraph() { assertFalse(subject.doBuildStreet()); assertFalse(subject.doSaveStreetGraph()); assertFalse(subject.doSaveGraph()); - assertEquals("Build Transit Graph", subject.logInfo()); + assertEquals("Build Transit Graph", subject.logTaskInfo()); subject.save = true; subject.serve = true; @@ -128,7 +128,7 @@ public void validateLoad() { // Implicit given, but should be ok to set subject.serve = true; - assertEquals("Run Server", subject.logInfo()); + assertEquals("Run Server", subject.logTaskInfo()); // No exception thrown subject.inferAndValidate();