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

DIRTY - Do not merge - Log failed itin checks and bus notifs #263

Open
wants to merge 13 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 0 additions & 32 deletions .github/workflows/maven-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,38 +56,6 @@ jobs:
TWILIO_ACCOUNT_SID: ${{ secrets.TWILIO_ACCOUNT_SID }}
TWILIO_AUTH_TOKEN: ${{ secrets.TWILIO_AUTH_TOKEN }}
TWILIO_VERIFICATION_SERVICE_SID: ${{ secrets.TWILIO_VERIFICATION_SERVICE_SID }}
- name: Run E2E tests
run: mvn --no-transfer-progress test
env:
OTP_ADMIN_DASHBOARD_NAME: OTP Admin Dashboard
OTP_ADMIN_DASHBOARD_FROM_EMAIL: OTP Admin Dashboard <[email protected]>
OTP_UI_NAME: Trip Planner
AUTH0_API_CLIENT: ${{ secrets.AUTH0_API_CLIENT }}
AUTH0_API_SECRET: ${{ secrets.AUTH0_API_SECRET }}
AUTH0_DOMAIN: ${{ secrets.AUTH0_DOMAIN }}
AWS_PROFILE: default
AWS_REGION: us-east-1
BUGSNAG_API_KEY: ${{ secrets.BUGSNAG_API_KEY }}
BUGSNAG_ORGANIZATION: ${{ secrets.BUGSNAG_ORGANIZATION }}
BUGSNAG_PROJECT_NOTIFIER_API_KEY: ${{ secrets.BUGSNAG_PROJECT_NOTIFIER_API_KEY }}
CONNECTED_DATA_PLATFORM_S3_BUCKET_NAME: ${{ secrets.CONNECTED_DATA_PLATFORM_S3_BUCKET_NAME }}
CONNECTED_DATA_PLATFORM_S3_FOLDER_NAME: ${{ secrets.CONNECTED_DATA_PLATFORM_S3_FOLDER_NAME }}
CONNECTED_DATA_PLATFORM_TRIP_HISTORY_UPLOAD_JOB_FREQUENCY_IN_MINUTES: ${{ secrets.CONNECTED_DATA_PLATFORM_TRIP_HISTORY_UPLOAD_JOB_FREQUENCY_IN_MINUTES }}
DEFAULT_USAGE_PLAN_ID: ${{ secrets.DEFAULT_USAGE_PLAN_ID }}
MONGO_DB_NAME: ${{ secrets.MONGO_DB_NAME }}
NOTIFICATION_FROM_EMAIL: ${{ secrets.NOTIFICATION_FROM_EMAIL }}
NOTIFICATION_FROM_PHONE: ${{ secrets.NOTIFICATION_FROM_PHONE }}
OTP_ADMIN_DASHBOARD_URL: ${{ secrets.OTP_ADMIN_DASHBOARD_URL }}
OTP_API_ROOT: ${{ secrets.OTP_API_ROOT }}
OTP2_API_ROOT: ${{ secrets.OTP_API_ROOT }}
OTP_PLAN_ENDPOINT: ${{ secrets.OTP_PLAN_ENDPOINT }}
OTP_TIMEZONE: ${{ secrets.OTP_TIMEZONE }}
OTP_UI_URL: ${{ secrets.OTP_UI_URL }}
SPARKPOST_KEY: ${{ secrets.SPARKPOST_KEY }}
TWILIO_ACCOUNT_SID: ${{ secrets.TWILIO_ACCOUNT_SID }}
TWILIO_AUTH_TOKEN: ${{ secrets.TWILIO_AUTH_TOKEN }}
TWILIO_VERIFICATION_SERVICE_SID: ${{ secrets.TWILIO_VERIFICATION_SERVICE_SID }}
RUN_E2E: true

# if this point it reached, the CI build has succeeded

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
import org.opentripplanner.middleware.otp.response.TripPlan;
import org.opentripplanner.middleware.utils.DateTimeUtils;
import org.opentripplanner.middleware.utils.ItineraryUtils;
import org.opentripplanner.middleware.utils.JsonUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.time.DayOfWeek;
import java.time.ZonedDateTime;
Expand All @@ -29,6 +32,8 @@
* particular day of the week.
*/
public class ItineraryExistence extends Model {
private static final Logger LOG = LoggerFactory.getLogger(ItineraryExistence.class);

/**
* Initial set of requests on which to base the itinerary existence checks. We do not want these persisted.
*/
Expand Down Expand Up @@ -186,7 +191,9 @@ public String getInvalidDaysOfWeekMessage() {
public void checkExistence(MonitoredTrip trip) {
// TODO: Consider multi-threading?
// Check existence of itinerary in the response for each OTP request.
int index = 0;
for (OtpRequest otpRequest : otpRequests) {
index++;
boolean hasMatchingItinerary = false;
DayOfWeek dayOfWeek = otpRequest.dateTime.getDayOfWeek();
// Get existing result for day of week if a date for that day of week has already been processed, or create
Expand All @@ -199,26 +206,38 @@ public void checkExistence(MonitoredTrip trip) {

// Send off each plan query to OTP.
OtpResponse response = this.otpResponseProvider.apply(otpRequest);
TripPlan plan = response.plan;

// Handle response if valid itineraries exist.
if (plan != null && plan.itineraries != null) {
for (Itinerary itineraryCandidate : plan.itineraries) {
// If a matching itinerary on the same service day as the request date is found,
// save the date with the matching itinerary.
// (The matching itinerary will replace the original trip.itinerary.)
if (
ItineraryUtils.occursOnSameServiceDay(itineraryCandidate, otpRequest.dateTime, tripIsArriveBy) &&
ItineraryUtils.itinerariesMatch(referenceItinerary, itineraryCandidate)
) {
result.handleValidDate(otpRequest.dateTime, itineraryCandidate);
hasMatchingItinerary = true;
if (response == null) {
LOG.warn("Itinerary existence check failed on {} for trip {} - OTP response was null.", dayOfWeek , trip.id);
} else {
TripPlan plan = response.plan;

// Handle response if valid itineraries exist.
if (plan != null && plan.itineraries != null) {
for (Itinerary itineraryCandidate : plan.itineraries) {
// If a matching itinerary on the same service day as the request date is found,
// save the date with the matching itinerary.
// (The matching itinerary will replace the original trip.itinerary.)
if (
ItineraryUtils.occursOnSameServiceDay(itineraryCandidate, otpRequest.dateTime, tripIsArriveBy) &&
ItineraryUtils.itinerariesMatch(referenceItinerary, itineraryCandidate)
) {
result.handleValidDate(otpRequest.dateTime, itineraryCandidate);
hasMatchingItinerary = true;
}
}
}

if (!hasMatchingItinerary) {
// If no match was found for the date, mark day of week as non-existent for the itinerary.
result.handleInvalidDate(otpRequest.dateTime);

// Log if the itinerary didn't exist "today"
if (index == 1) {
LOG.warn("Itinerary existence check failed 'today' for trip {} - params: {}", trip.id, JsonUtils.toJson(trip.otp2QueryParams));
LOG.warn("Itinerary existence check failed 'today' for trip {} - saved itinerary: {}", trip.id, JsonUtils.toJson(trip.itinerary));
LOG.warn("Itinerary existence check failed 'today' for trip {} - OTP itineraries: {}", trip.id, JsonUtils.toJson(plan.itineraries));
}
}
}
if (!hasMatchingItinerary) {
// If no match was found for the date, mark day of week as non-existent for the itinerary.
result.handleInvalidDate(otpRequest.dateTime);
}
}
if (!allMonitoredDaysAreValid(trip)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.opentripplanner.middleware.utils.DateTimeUtils;
import org.opentripplanner.middleware.utils.I18nUtils;
import org.opentripplanner.middleware.utils.ItineraryUtils;
import org.opentripplanner.middleware.utils.JsonUtils;
import org.opentripplanner.middleware.utils.NotificationUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -259,7 +260,10 @@ private boolean isTrackingOngoing() {
*/
private boolean makeOTPRequestAndUpdateMatchingItineraryInternal() {
OtpResponse otpResponse = otpResponseProvider.get();
if (otpResponse == null) return false;
if (otpResponse == null) {
LOG.warn("No comparison itinerary found for trip {} - OTP response was null.", trip.id);
return false;
}
for (int i = 0; i < otpResponse.plan.itineraries.size(); i++) {
Itinerary candidateItinerary = otpResponse.plan.itineraries.get(i);
if (ItineraryUtils.itinerariesMatch(trip.itinerary, candidateItinerary)) {
Expand Down Expand Up @@ -301,7 +305,9 @@ private boolean makeOTPRequestAndUpdateMatchingItineraryInternal() {
}

// If this point is reached, a matching itinerary was not found
LOG.warn("No comparison itinerary found in otp response for trip");
LOG.warn("No comparison itinerary found for trip {} - params: {}", trip.id, JsonUtils.toJson(trip.otp2QueryParams));
LOG.warn("No comparison itinerary found for trip {} - saved itinerary: {}", trip.id, JsonUtils.toJson(trip.itinerary));
LOG.warn("No comparison itinerary found for trip {} - OTP itineraries: {}", trip.id, JsonUtils.toJson(otpResponse.plan.itineraries));

if (hasReachedMaxItineraryChecks()) {
// Check whether this trip should no longer ever be checked due to not having matching itineraries on any
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ public void handleSendNotificationAction(TripStatus tripStatus, TravelerPosition
LOG.error("Could not trigger class {} for agency {}", action.trigger, action.agencyId, e);
throw new RuntimeException(e);
}
} else {
LOG.warn("No agency action found for this notification.");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ public void cancelNotification(TravelerPosition travelerPosition) {
* Makes a call to the bus driver notification API, followed by a call to the bus priority API.
*/
private static void makeApiRequests(TravelerPosition travelerPosition, String body, String routeId, String action) {
LOG.info("Sending bus operator/priority requests: {}", body);

var httpStatus = postBusDriverNotification(body);
if (httpStatus == HttpStatus.OK_200) {
travelerPosition.trackedJourney.updateNotificationMessage(routeId, body);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,34 +330,34 @@ public static String removeAgencyPrefix(String idParts) {
* Get the route id from leg.
*/
public static String getRouteIdFromLeg(Leg leg) {
return (leg != null) ? leg.routeId : null;
return (leg != null) ? leg.route.gtfsId : null;
}

/**
* Get the agency id from leg.
*/
public static String getAgencyIdFromLeg(Leg leg) {
return (leg != null) ? leg.agencyId : null;
return (leg != null) ? leg.agency.gtfsId : null;
}

/**
* Get the trip id from leg.
*/
public static String getTripIdFromLeg(Leg leg) {
return (leg != null) ? leg.tripId : null;
return (leg != null) ? leg.trip.gtfsId : null;
}

/**
* Get the stop id from place.
*/
public static String getStopIdFromPlace(Place place) {
return (place != null) ? place.stopId : null;
return (place != null) ? place.stop.gtfsId : null;
}

/**
* Get the route short name from leg.
*/
public static String getRouteShortNameFromLeg(Leg leg) {
return leg.routeShortName;
return leg.route.shortName;
}
}
Loading