Skip to content

Commit 044edcd

Browse files
committed
Fix query ui history redirect
1 parent 69ac14b commit 044edcd

File tree

2 files changed

+64
-32
lines changed

2 files changed

+64
-32
lines changed

gateway-ha/src/main/java/com/lyft/data/gateway/ha/handler/QueryIdCachingProxyHandler.java

Lines changed: 53 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,10 @@ public class QueryIdCachingProxyHandler extends ProxyHandler {
4242
public static final String SOURCE_HEADER = "X-Trino-Source";
4343
public static final String ALTERNATE_SOURCE_HEADER = "X-Presto-Source";
4444
public static final String HOST_HEADER = "Host";
45+
public static final String REFERER_STRING = "Referer";
4546
private static final int QUERY_TEXT_LENGTH_FOR_HISTORY = 200;
46-
private static final Pattern QUERY_ID_PATTERN = Pattern.compile(".*[/=?](\\d+_\\d+_\\d+_\\w+).*");
47+
private static final Pattern QUERY_ID_PATTERN =
48+
Pattern.compile("(.*[/=?])?(\\d+_\\d+_\\d+_\\w+).*");
4749

4850
private static final Pattern EXTRACT_BETWEEN_SINGLE_QUOTES = Pattern.compile("'([^\\s']+)'");
4951

@@ -115,22 +117,23 @@ public String rewriteTarget(HttpServletRequest request) {
115117

116118
// Only load balance presto query APIs.
117119
if (isPathWhiteListed(request.getRequestURI())) {
118-
String queryId = extractQueryIdIfPresent(request);
120+
Optional<String> queryId = extractQueryIdIfPresent(request);
119121

122+
backendAddress = queryId
120123
// Find query id and get url from cache
121-
if (!Strings.isNullOrEmpty(queryId)) {
122-
backendAddress = routingManager.findBackendForQueryId(queryId);
123-
} else {
124+
.map(routingManager::findBackendForQueryId)
125+
.orElseGet(() -> {
124126
String routingGroup = routingGroupSelector.findRoutingGroup(request);
125127
String user = Optional.ofNullable(request.getHeader(USER_HEADER))
126128
.orElse(request.getHeader(ALTERNATE_USER_HEADER));
127129
if (!Strings.isNullOrEmpty(routingGroup)) {
128130
// This falls back on adhoc backend if there are no cluster found for the routing group.
129-
backendAddress = routingManager.provideBackendForRoutingGroup(routingGroup, user);
131+
return routingManager.provideBackendForRoutingGroup(routingGroup, user);
130132
} else {
131-
backendAddress = routingManager.provideAdhocBackend(user);
133+
return routingManager.provideAdhocBackend(user);
132134
}
133-
}
135+
});
136+
134137
// set target backend so that we could save queryId to backend mapping later.
135138
((MultiReadHttpServletRequest) request).addHeader(PROXY_TARGET_HEADER, backendAddress);
136139
}
@@ -159,7 +162,7 @@ public String rewriteTarget(HttpServletRequest request) {
159162
return targetLocation;
160163
}
161164

162-
protected String extractQueryIdIfPresent(HttpServletRequest request) {
165+
protected Optional<String> extractQueryIdIfPresent(HttpServletRequest request) {
163166
String path = request.getRequestURI();
164167
String queryParams = request.getQueryString();
165168
try {
@@ -174,7 +177,8 @@ protected String extractQueryIdIfPresent(HttpServletRequest request) {
174177
if (m.find()) {
175178
String queryQuoted = m.group();
176179
if (!Strings.isNullOrEmpty(queryQuoted) && queryQuoted.length() > 0) {
177-
return queryQuoted.substring(1, queryQuoted.length() - 1);
180+
String res = queryQuoted.substring(1, queryQuoted.length() - 1);
181+
return Optional.of(res).filter(s -> !s.isEmpty());
178182
}
179183
}
180184
}
@@ -183,37 +187,58 @@ protected String extractQueryIdIfPresent(HttpServletRequest request) {
183187
} catch (Exception e) {
184188
log.error("Error extracting query payload from request", e);
185189
}
186-
187-
return extractQueryIdIfPresent(path, queryParams);
190+
191+
return extractQueryIdIfPresent(path, queryParams).or(() -> {
192+
return Optional.ofNullable(request.getHeader(REFERER_STRING))
193+
.flatMap(referer -> {
194+
try {
195+
URI uri = URI.create(referer);
196+
return extractQueryIdIfPresent(uri.getPath(), uri.getQuery());
197+
} catch (Exception e) {
198+
log.error("Error extracting query id from Referer header", e);
199+
}
200+
return Optional.empty();
201+
});
202+
}).filter(s -> !s.isEmpty());
188203
}
189204

190-
protected static String extractQueryIdIfPresent(String path, String queryParams) {
205+
protected static Optional<String> extractQueryIdIfPresent(String path, String queryParams) {
191206
if (path == null) {
192-
return null;
207+
return Optional.empty();
193208
}
194-
String queryId = null;
195209

196210
log.debug("trying to extract query id from path [{}] or queryString [{}]", path, queryParams);
197211
if (path.startsWith(V1_STATEMENT_PATH) || path.startsWith(V1_QUERY_PATH)) {
198212
String[] tokens = path.split("/");
199-
if (tokens.length >= 4) {
200-
if (path.contains("queued")
201-
|| path.contains("scheduled")
202-
|| path.contains("executing")
203-
|| path.contains("partialCancel")) {
204-
queryId = tokens[4];
205-
} else {
206-
queryId = tokens[3];
207-
}
213+
if (tokens.length < 4) {
214+
return Optional.empty();
208215
}
209-
} else if (path.startsWith(PRESTO_UI_PATH)) {
216+
217+
if (path.contains("queued")
218+
|| path.contains("scheduled")
219+
|| path.contains("executing")
220+
|| path.contains("partialCancel")) {
221+
return Optional.of(tokens[4]);
222+
}
223+
224+
return Optional.of(tokens[3]);
225+
}
226+
227+
if (path.startsWith(PRESTO_UI_PATH)) {
210228
Matcher matcher = QUERY_ID_PATTERN.matcher(path);
211229
if (matcher.matches()) {
212-
queryId = matcher.group(1);
230+
return Optional.of(matcher.group(2));
231+
}
232+
}
233+
234+
if (queryParams != null) {
235+
Matcher matcher = QUERY_ID_PATTERN.matcher(queryParams);
236+
if (matcher.matches()) {
237+
return Optional.of(matcher.group(2));
213238
}
214239
}
215-
log.debug("query id in url [{}]", queryId);
216-
return queryId;
240+
241+
return Optional.empty();
217242
}
218243

219244
protected void postConnectionHook(

gateway-ha/src/test/java/com/lyft/data/gateway/ha/handler/TestQueryIdCachingProxyHandler.java

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import static org.testng.Assert.assertNull;
55

66
import java.io.IOException;
7+
import java.util.Optional;
78

89
import javax.servlet.http.HttpServletRequest;
910

@@ -23,15 +24,21 @@ public void testExtractQueryIdFromUrl() throws IOException {
2324
"/ui/api/query?query_id=20200416_160256_03078_6b4yt",
2425
"/ui/api/query.html?20200416_160256_03078_6b4yt"};
2526
for (String path : paths) {
26-
String queryId = QueryIdCachingProxyHandler.extractQueryIdIfPresent(path, null);
27-
assertEquals(queryId, "20200416_160256_03078_6b4yt");
27+
Optional<String> queryId = QueryIdCachingProxyHandler.extractQueryIdIfPresent(path, null);
28+
assertEquals(queryId, Optional.of("20200416_160256_03078_6b4yt"));
2829
}
30+
31+
Optional<String> queryParamId = QueryIdCachingProxyHandler.extractQueryIdIfPresent(
32+
"/ui/query",
33+
"20200416_160256_03078_6b4yt");
34+
assertEquals(queryParamId, Optional.of("20200416_160256_03078_6b4yt"));
35+
2936
String[] nonPaths = {
3037
"/ui/api/query/myOtherThing",
3138
"/ui/api/query/20200416_blah?bogus_fictional_param"};
3239
for (String path : nonPaths) {
33-
String queryId = QueryIdCachingProxyHandler.extractQueryIdIfPresent(path, null);
34-
assertNull(queryId);
40+
Optional<String> queryId = QueryIdCachingProxyHandler.extractQueryIdIfPresent(path, null);
41+
assertEquals(queryId, Optional.empty());
3542
}
3643
}
3744

0 commit comments

Comments
 (0)