refactor: Improve logging in VehicleRentalUpdater

opentripplanner · Aug 30, 2023 · 6dbd916 · 6dbd916
1 parent e01906b
commit 6dbd916
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 12 deletions.
diff --git a/src/main/java/org/opentripplanner/updater/vehicle_rental/VehicleRentalUpdater.java b/src/main/java/org/opentripplanner/updater/vehicle_rental/VehicleRentalUpdater.java
@@ -10,6 +10,9 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
+import org.opentripplanner.framework.lang.ObjectUtils;
+import org.opentripplanner.framework.logging.ThrottleLogger;
+import org.opentripplanner.framework.time.DurationUtils;
 import org.opentripplanner.framework.time.TimeUtils;
 import org.opentripplanner.framework.tostring.ToStringBuilder;
 import org.opentripplanner.routing.graph.Graph;
@@ -45,7 +48,12 @@
 public class VehicleRentalUpdater extends PollingGraphUpdater {
 
   private static final Logger LOG = LoggerFactory.getLogger(VehicleRentalUpdater.class);
+
+  private final Logger unlinkedPlaceLogger;
+
   private final VehicleRentalDatasource source;
+  private final String nameForLogging;
+
   private WriteToGraphCallback saveResultOnGraph;
 
   private Map<StreetEdge, RentalRestrictionExtension> latestModifiedEdges = Map.of();
@@ -64,9 +72,15 @@ public VehicleRentalUpdater(
   ) throws IllegalArgumentException {
     super(parameters);
     // Configure updater
-    LOG.info("Setting up vehicle rental updater.");
+    LOG.info("Setting up vehicle rental updater for {}.", source);
 
     this.source = source;
+    this.nameForLogging =
+      ObjectUtils.ifNotNull(
+        parameters.sourceParameters().network(),
+        parameters.sourceParameters().url()
+      );
+    this.unlinkedPlaceLogger = ThrottleLogger.throttle(LOG);
 
     // Creation of network linker library will not modify the graph
     this.linker = vertexLinker;
@@ -78,16 +92,19 @@ public VehicleRentalUpdater(
       // Do any setup if needed
       source.setup();
     } catch (UpdaterConstructionException e) {
-      LOG.warn("Unable to setup updater: {}", this, e);
+      LOG.warn("Unable to setup updater: {}", nameForLogging, e);
     }
 
     if (runOnlyOnce()) {
-      LOG.info("Creating vehicle-rental updater running once only (non-polling): {}", source);
+      LOG.info(
+        "Creating vehicle-rental updater running once only (non-polling): {}",
+        nameForLogging
+      );
     } else {
       LOG.info(
-        "Creating vehicle-rental updater running every {} seconds: {}",
-        pollingPeriod(),
-        source
+        "Creating vehicle-rental updater running every {}: {}",
+        DurationUtils.durationToStr(pollingPeriod()),
+        nameForLogging
       );
     }
   }
@@ -109,9 +126,9 @@ public String getConfigRef() {
 
   @Override
   protected void runPolling() {
-    LOG.debug("Updating vehicle rental stations from {}", source);
+    LOG.debug("Updating vehicle rental stations from {}", nameForLogging);
     if (!source.update()) {
-      LOG.debug("No updates");
+      LOG.debug("No updates from {}", nameForLogging);
       return;
     }
     List<VehicleRentalPlace> stations = source.getUpdates();
@@ -149,6 +166,7 @@ public void run(Graph graph, TransitModel transitModel) {
         service.addVehicleRentalStation(station);
         stationSet.add(station.getId());
         VehicleRentalPlaceVertex vehicleRentalVertex = verticesByStation.get(station.getId());
+
         if (vehicleRentalVertex == null) {
           vehicleRentalVertex = vertexFactory.vehicleRentalPlace(station);
           DisposableEdgeCollection tempEdges = linker.linkVertexForRealTime(
@@ -169,7 +187,11 @@ public void run(Graph graph, TransitModel transitModel) {
           );
           if (vehicleRentalVertex.getOutgoing().isEmpty()) {
             // the toString includes the text "Bike rental station"
-            LOG.info("VehicleRentalPlace {} is unlinked", vehicleRentalVertex);
+            unlinkedPlaceLogger.info(
+              "VehicleRentalPlace is unlinked for {}: {}",
+              nameForLogging,
+              vehicleRentalVertex
+            );
           }
           Set<RentalFormFactor> formFactors = Stream
             .concat(
@@ -188,6 +210,7 @@ public void run(Graph graph, TransitModel transitModel) {
           vehicleRentalVertex.setStation(station);
         }
       }
+
       /* remove existing stations that were not present in the update */
       List<FeedScopedId> toRemove = new ArrayList<>();
       for (Entry<FeedScopedId, VehicleRentalPlaceVertex> entry : verticesByStation.entrySet()) {
@@ -206,7 +229,7 @@ public void run(Graph graph, TransitModel transitModel) {
       // this check relies on the generated equals for the record which also recursively checks that
       // the JTS geometries are equal
       if (!geofencingZones.isEmpty() && !geofencingZones.equals(latestAppliedGeofencingZones)) {
-        LOG.info("Computing geofencing zones");
+        LOG.info("Computing geofencing zones for {}", nameForLogging);
         var start = System.currentTimeMillis();
 
         latestModifiedEdges.forEach(StreetEdge::removeRentalExtension);
@@ -218,9 +241,10 @@ public void run(Graph graph, TransitModel transitModel) {
         var end = System.currentTimeMillis();
         var millis = Duration.ofMillis(end - start);
         LOG.info(
-          "Geofencing zones computation took {}. Added extension to {} edges.",
+          "Geofencing zones computation took {}. Added extension to {} edges. For {}",
           TimeUtils.durationToStrCompact(millis),
-          latestModifiedEdges.size()
+          latestModifiedEdges.size(),
+          nameForLogging
         );
       }
     }

diff --git a/...pplanner/updater/vehicle_rental/datasources/params/VehicleRentalDataSourceParameters.java b/...pplanner/updater/vehicle_rental/datasources/params/VehicleRentalDataSourceParameters.java
@@ -1,13 +1,17 @@
 package org.opentripplanner.updater.vehicle_rental.datasources.params;
 
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import org.opentripplanner.updater.spi.HttpHeaders;
 import org.opentripplanner.updater.vehicle_rental.VehicleRentalSourceType;
 
 public interface VehicleRentalDataSourceParameters {
   @Nonnull
   String url();
 
+  @Nullable
+  String network();
+
   @Nonnull
   VehicleRentalSourceType sourceType();