Skip to content

Commit

Permalink
refactor: Generalize request tags in OTP, remove dependency to Microm…
Browse files Browse the repository at this point in the history
…eter API
  • Loading branch information
t2gran committed Mar 30, 2022
1 parent 4f5bfb4 commit 667876c
Show file tree
Hide file tree
Showing 13 changed files with 124 additions and 70 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package org.opentripplanner.routing.algorithm;

import io.micrometer.core.instrument.Tag;
import java.time.Duration;
import java.time.Instant;
import java.time.ZoneId;
Expand Down Expand Up @@ -68,7 +67,7 @@ public RoutingWorker(Router router, RoutingRequest request, ZoneId zoneId) {
request.applyPageCursor();
this.request = request;
this.router = router;
this.debugTimingAggregator = new DebugTimingAggregator(router.meterRegistry, request.timingTags);
this.debugTimingAggregator = new DebugTimingAggregator(router.meterRegistry, request.tags.getTimingTags());
this.transitSearchTimeZero = DateMapper.asStartOfService(request.getDateTime(), zoneId);
this.pagingSearchWindowAdjuster = createPagingSearchWindowAdjuster(router.routerConfig);
this.additionalSearchDays = createAdditionalSearchDays(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ private RaptorRequest<TripSchedule> doMap(
.logger(debugLogger);
}

builder.debug().timingTags(request.timingTags);
builder.addTimingTags(request.tags.getTimingTags());

if(!request.timetableView && request.arriveBy) {
builder.searchParams().preferLateArrival(true);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
package org.opentripplanner.routing.api.request;

import io.micrometer.core.instrument.Tag;
import org.geotools.geojson.geom.GeometryJSON;
import org.locationtech.jts.geom.Coordinate;
import org.locationtech.jts.geom.Envelope;
import org.locationtech.jts.geom.GeometryFactory;
import org.opentripplanner.api.common.LocationStringParser;
import org.opentripplanner.common.geometry.GeometryUtils;
import org.opentripplanner.common.geometry.SphericalDistanceLibrary;
Expand Down Expand Up @@ -790,10 +788,10 @@ public class RoutingRequest implements AutoCloseable, Cloneable, Serializable {
public RaptorOptions raptorOptions = new RaptorOptions();

/**
* List of micrometer tags to be added to all the timer instances for this request.
* List of OTP request tags, these are used to cross-cutting concerns like logging and
* micrometer tags. Currently, all tags are added to all the timer instances for this request.
*/
public List<Tag> timingTags = List.of();

public Tags tags = Tags.of();


/* CONSTRUCTORS */
Expand Down
43 changes: 43 additions & 0 deletions src/main/java/org/opentripplanner/routing/api/request/Tags.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package org.opentripplanner.routing.api.request;

import java.io.Serial;
import java.io.Serializable;
import java.util.Collection;
import java.util.Set;


/**
* A collection of request tags. The intended use of tags are for cross-cutting
* concerns like performance timing, debugging and logging. Currently, we only use tags for
* performance timing; Hence only one getter method {@code #getTimingTags}.
*/
public class Tags implements Serializable {
@Serial
private static final long serialVersionUID = 1L;

/** We only need one set of tags since we support timingTags only */
private final Set<String> tags;

private Tags(Collection<String> tags) {
this.tags = tags == null ? Set.of() :Set.copyOf(tags);
}

public static Tags of() {
return new Tags(null);
}

public static Tags of(Collection<String> tags) {
return new Tags(tags);
}

/** Currently all tags are used as Micrometer timing tags */
public Set<String> getTimingTags() {
return tags;
}

@Override
public String toString() {
return String.join(", ", tags);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
import io.micrometer.core.instrument.DistributionSummary;
import io.micrometer.core.instrument.MeterRegistry;
import io.micrometer.core.instrument.Metrics;
import io.micrometer.core.instrument.Tag;
import io.micrometer.core.instrument.Timer;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import org.opentripplanner.api.resource.DebugOutput;
import org.opentripplanner.api.resource.TransitTimingOutput;
Expand Down Expand Up @@ -84,28 +84,29 @@ public class DebugTimingAggregator {
* Record the time when we first began calculating a path for this request. Note that timings will not
* include network and server request queue overhead, which is what we want.
*/
public DebugTimingAggregator(MeterRegistry registry, List<Tag> timingTags) {
public DebugTimingAggregator(MeterRegistry registry, Collection<String> timingTags) {
var tags = MicrometerUtils.mapTimingTags(timingTags);
clock = registry.config().clock();
startedCalculating = Timer.start(this.clock);

requestTotalTimer = Timer.builder(ROUTING_TOTAL).tags(timingTags).register(registry);
routingTotalTimer = Timer.builder("routing.router").tags(timingTags).register(registry);
renderingTimer = Timer.builder("routing.rendering").tags(timingTags).register(registry);
filteringTimer = Timer.builder("routing.filtering").tags(timingTags).register(registry);
transitRouterTimer = Timer.builder("routing.transit").tags(timingTags).register(registry);
itineraryCreationTimer = Timer.builder("routing.itineraryCreation").tags(timingTags).register(registry);
raptorSearchTimer = Timer.builder(ROUTING_RAPTOR).tags(timingTags).register(registry);
accessEgressTimer = Timer.builder("routing.accessEgress").tags(timingTags).register(registry);
tripPatternFilterTimer = Timer.builder("routing.tripPatternFiltering").tags(timingTags).register(registry);
preCalculationTimer = Timer.builder("routing.preCalculation").tags(timingTags).register(registry);

numEgressesDistribution = DistributionSummary.builder("routing.numEgress").tags(timingTags).register(registry);
numAccessesDistribution = DistributionSummary.builder("routing.numAccess").tags(timingTags).register(registry);

egressTimer = Timer.builder("routing.egress").tags(timingTags).register(registry);
accessTimer = Timer.builder("routing.access").tags(timingTags).register(registry);
directFlexRouterTimer = Timer.builder("routing.directFlex").tags(timingTags).register(registry);
directStreetRouterTimer = Timer.builder("routing.directStreet").tags(timingTags).register(registry);
requestTotalTimer = Timer.builder(ROUTING_TOTAL).tags(tags).register(registry);
routingTotalTimer = Timer.builder("routing.router").tags(tags).register(registry);
renderingTimer = Timer.builder("routing.rendering").tags(tags).register(registry);
filteringTimer = Timer.builder("routing.filtering").tags(tags).register(registry);
transitRouterTimer = Timer.builder("routing.transit").tags(tags).register(registry);
itineraryCreationTimer = Timer.builder("routing.itineraryCreation").tags(tags).register(registry);
raptorSearchTimer = Timer.builder(ROUTING_RAPTOR).tags(tags).register(registry);
accessEgressTimer = Timer.builder("routing.accessEgress").tags(tags).register(registry);
tripPatternFilterTimer = Timer.builder("routing.tripPatternFiltering").tags(tags).register(registry);
preCalculationTimer = Timer.builder("routing.preCalculation").tags(tags).register(registry);

numEgressesDistribution = DistributionSummary.builder("routing.numEgress").tags(tags).register(registry);
numAccessesDistribution = DistributionSummary.builder("routing.numAccess").tags(tags).register(registry);

egressTimer = Timer.builder("routing.egress").tags(tags).register(registry);
accessTimer = Timer.builder("routing.access").tags(tags).register(registry);
directFlexRouterTimer = Timer.builder("routing.directFlex").tags(tags).register(registry);
directStreetRouterTimer = Timer.builder("routing.directStreet").tags(tags).register(registry);
}

public DebugTimingAggregator() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package org.opentripplanner.routing.framework;

import io.micrometer.core.instrument.Tag;
import java.util.Collection;
import java.util.List;

public class MicrometerUtils {
public static List<Tag> mapTimingTags(Collection<String> timingTags) {
return List.of(Tag.of("tags", String.join(" ", timingTags)));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,10 @@ public record DebugRequest(
Consumer<DebugEvent<ArrivalView<?>>> stopArrivalListener,
Consumer<DebugEvent<PatternRide<?>>> patternRideDebugListener,
Consumer<DebugEvent<Path<?>>> pathFilteringListener,
DebugLogger logger,
List<Tag> timingTags
DebugLogger logger
) {
private static final DebugRequest DEFAULT_DEBUG_REQUEST = new DebugRequest(
List.of(), List.of(), 0, null, null, null, DebugLogger.noop(), List.of()
List.of(), List.of(), 0, null, null, null, DebugLogger.noop()
);

/**
Expand All @@ -78,7 +77,6 @@ public String toString() {
.addBoolIfTrue("stopArrivalListener", stopArrivalListener != null)
.addBoolIfTrue("pathFilteringListener", pathFilteringListener != null)
.addBoolIfTrue("logger", logger != DEFAULT_DEBUG_REQUEST.logger())
.addCol("timingTags", timingTags)
.toString();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ public class DebugRequestBuilder {
private Consumer<DebugEvent<PatternRide<?>>> patternRideDebugListener;
private Consumer<DebugEvent<Path<?>>> pathFilteringListener;
private DebugLogger logger;
private List<Tag> timingTags;


DebugRequestBuilder(DebugRequest debug) {
Expand All @@ -38,7 +37,6 @@ public class DebugRequestBuilder {
this.patternRideDebugListener = debug.patternRideDebugListener();
this.pathFilteringListener = debug.pathFilteringListener();
this.logger = debug.logger();
this.timingTags = debug.timingTags();
}

/** Read-only view to stops added sorted in ascending order. */
Expand Down Expand Up @@ -122,16 +120,6 @@ public DebugRequestBuilder logger(DebugLogger logger) {
return this;
}

public List<Tag> timingTags() {
return timingTags;
}

public DebugRequestBuilder timingTags(List<Tag> timingTags) {
this.timingTags = timingTags;
return this;
}


public DebugRequestBuilder reverseDebugRequest() {
Collections.reverse(this.path);
return this;
Expand All @@ -145,8 +133,7 @@ public DebugRequest build() {
stopArrivalListener,
patternRideDebugListener,
pathFilteringListener,
logger,
timingTags
logger
);
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package org.opentripplanner.transit.raptor.api.request;

import io.micrometer.core.instrument.Tag;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import org.opentripplanner.model.base.ToStringBuilder;
Expand All @@ -27,6 +29,8 @@ public class RaptorRequest<T extends RaptorTripSchedule> {
private final Set<Optimization> optimizations;
private final DebugRequest debug;
private final RaptorSlackProvider slackProvider;
private final Set<String> timingTags;



static <T extends RaptorTripSchedule> RaptorRequest<T> defaults() {
Expand All @@ -40,6 +44,7 @@ private RaptorRequest() {
optimizations = Collections.emptySet();
// Slack defaults: 1 minute for transfer-slack, 0 minutes for board- and alight-slack.
slackProvider = RaptorSlackProvider.defaultSlackProvider(60, 0, 0);
timingTags = Set.of();
debug = DebugRequest.defaults();
}

Expand All @@ -49,6 +54,7 @@ private RaptorRequest() {
this.searchDirection = builder.searchDirection();
this.optimizations = Set.copyOf(builder.optimizations());
this.slackProvider = builder.slackProvider();
this.timingTags = builder.timingTags();
this.debug = builder.debug().build();
verify();
}
Expand Down Expand Up @@ -125,6 +131,10 @@ public boolean runInParallel() {
return optimizationEnabled(Optimization.PARALLEL);
}

public Set<String> timingTags() {
return timingTags;
}

/**
* Specify what to debug in the debug request.
* <p/>
Expand All @@ -137,11 +147,12 @@ public DebugRequest debug() {
@Override
public String toString() {
return ToStringBuilder.of(RaptorRequest.class)
.addEnum("profile", profile)
.addBoolIfTrue("reverse", searchDirection.isInReverse())
.addCol("optimizations", optimizations)
.addObj("debug", debug, DebugRequest.defaults())
.addObj("searchParams", searchParams)
.addEnum("profile", profile)
.addBoolIfTrue("reverse", searchDirection.isInReverse())
.addCol("optimizations", optimizations)
.addObj("debug", debug, DebugRequest.defaults())
.addObj("searchParams", searchParams)
.addCol("timingTags", timingTags)
.toString();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.util.Collection;
import java.util.EnumSet;
import java.util.HashSet;
import java.util.Set;
import javax.validation.constraints.NotNull;
import org.opentripplanner.transit.raptor.api.transit.RaptorSlackProvider;
Expand All @@ -28,6 +29,8 @@ public class RaptorRequestBuilder<T extends RaptorTripSchedule> {
private RaptorProfile profile;
private final Set<Optimization> optimizations = EnumSet.noneOf(Optimization.class);

// Timer
private final Set<String> timingTags;

// Debug
private final DebugRequestBuilder debug;
Expand All @@ -45,6 +48,9 @@ public RaptorRequestBuilder() {
this.profile = defaults.profile();
this.optimizations.addAll(defaults.optimizations());

// Timer
timingTags = new HashSet<>(defaults.timingTags());

// Debug
this.debug = new DebugRequestBuilder(defaults.debug());
}
Expand Down Expand Up @@ -98,6 +104,15 @@ public RaptorRequestBuilder<T> disableOptimization(Optimization optimization) {
return this;
}

public RaptorRequestBuilder<T> addTimingTags(Collection<String> tags) {
this.timingTags.addAll(tags);
return this;
}

public Set<String> timingTags() {
return timingTags;
}

public DebugRequestBuilder debug() {
return this.debug;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public SearchContext<T> context(RaptorTransitDataProvider<T> transit, RaptorRequ
transit,
new WorkerPerformanceTimers(
RequestAlias.alias(request, isMultiThreaded()),
request.debug().timingTags(),
request.timingTags(),
registry
)
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package org.opentripplanner.transit.raptor.rangeraptor.debug;

import io.micrometer.core.instrument.MeterRegistry;
import io.micrometer.core.instrument.Tag;
import io.micrometer.core.instrument.Timer;
import java.util.List;
import java.util.Collection;
import org.opentripplanner.routing.framework.MicrometerUtils;

public class WorkerPerformanceTimers {
// Variables to track time spent
Expand All @@ -13,20 +13,21 @@ public class WorkerPerformanceTimers {

public WorkerPerformanceTimers(
String namePrefix,
List<Tag> timingTags,
Collection<String> timingTags,
MeterRegistry registry
) {
var tags = MicrometerUtils.mapTimingTags(timingTags);
timerRoute = Timer
.builder("raptor." + namePrefix + ".route")
.tags(timingTags)
.tags(tags)
.register(registry);
timerByMinuteScheduleSearch = Timer
.builder("raptor." + namePrefix + ".minute.transit")
.tags(timingTags)
.tags(tags)
.register(registry);
timerByMinuteTransfers = Timer
.builder("raptor." + namePrefix + ".minute.transfers")
.tags(timingTags)
.tags(tags)
.register(registry);
}

Expand Down
Loading

0 comments on commit 667876c

Please sign in to comment.