Skip to content

Commit bf02544

Browse files
authored
Record http request parameters in log metadata (#2642)
This allows us to search for logs for a given path using a filter like this: jsonPayload.httpRequest.requestUrl="/_dr/blah" TESTED=tested on crash
1 parent 9f22f2e commit bf02544

File tree

4 files changed

+64
-11
lines changed

4 files changed

+64
-11
lines changed

core/src/main/java/google/registry/module/RegistryServlet.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@
1414

1515
package google.registry.module;
1616

17+
import static google.registry.util.GcpJsonFormatter.setCurrentRequest;
1718
import static google.registry.util.GcpJsonFormatter.setCurrentTraceId;
19+
import static google.registry.util.GcpJsonFormatter.unsetCurrentRequest;
1820
import static google.registry.util.RandomStringGenerator.insecureRandomStringGenerator;
1921
import static google.registry.util.StringGenerator.Alphabets.HEX_DIGITS_ONLY;
2022

@@ -67,10 +69,16 @@ public RegistryServlet() {
6769
@Override
6870
public void service(HttpServletRequest req, HttpServletResponse rsp) throws IOException {
6971
setCurrentTraceId(traceId());
72+
String requestMethod = req.getMethod();
73+
String requestUrl = req.getRequestURI();
74+
String userAgent = String.valueOf(req.getHeader("User-Agent"));
75+
String protocol = req.getProtocol();
76+
setCurrentRequest(requestMethod, requestUrl, userAgent, protocol);
7077
try {
7178
super.service(req, rsp);
7279
} finally {
7380
setCurrentTraceId(null);
81+
unsetCurrentRequest();
7482
}
7583
}
7684

core/src/main/java/google/registry/tools/RegistryCli.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@
4343
final class RegistryCli implements CommandRunner {
4444

4545
private static final ImmutableSet<RegistryToolEnvironment> DEFAULT_GKE_ENVIRONMENTS =
46-
ImmutableSet.of(RegistryToolEnvironment.ALPHA, RegistryToolEnvironment.QA);
46+
ImmutableSet.of(
47+
RegistryToolEnvironment.ALPHA, RegistryToolEnvironment.CRASH, RegistryToolEnvironment.QA);
4748

4849
// The environment parameter is parsed twice: once here, and once with {@link
4950
// RegistryToolEnvironment#parseFromArgs} in the {@link RegistryTool#main} function.

util/src/main/java/google/registry/util/GcpJsonFormatter.java

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.util.logging.Formatter;
2727
import java.util.logging.Level;
2828
import java.util.logging.LogRecord;
29+
import javax.annotation.Nullable;
2930

3031
/**
3132
* JUL formatter that formats log messages in a single-line JSON that Stackdriver logging can parse.
@@ -54,6 +55,9 @@ public class GcpJsonFormatter extends Formatter {
5455
/** JSON field that stores the trace associated with the log entry, if any. */
5556
private static final String TRACE = "logging.googleapis.com/trace";
5657

58+
/** JSON field that stores the parameters of the current request, if any. */
59+
private static final String HTTP_REQUEST = "httRequest";
60+
5761
/** JSON field that contains the content, this will show up as the main entry in a log. */
5862
private static final String MESSAGE = "message";
5963

@@ -77,19 +81,36 @@ public class GcpJsonFormatter extends Formatter {
7781

7882
private static final ThreadLocal<String> traceId = new ThreadLocal<>();
7983

84+
private static final ThreadLocal<HttpRequest> request = new ThreadLocal<>();
85+
8086
/**
8187
* Set the Trace ID associated with any logging done by the current thread.
8288
*
8389
* @param id The traceID, in the form projects/[PROJECT_ID]/traces/[TRACE_ID]
8490
*/
85-
public static void setCurrentTraceId(String id) {
91+
public static void setCurrentTraceId(@Nullable String id) {
8692
if (id == null) {
8793
traceId.remove();
8894
} else {
8995
traceId.set(id);
9096
}
9197
}
9298

99+
/**
100+
* Record the parameters of the current request.
101+
*
102+
* @see <a
103+
* href="https://cloud.google.com/logging/docs/reference/v2/rest/v2/LogEntry#httprequest">HttpRequest</a>
104+
*/
105+
public static void setCurrentRequest(
106+
String requestMethod, String requestUrl, String userAgent, String protocol) {
107+
request.set(new HttpRequest(requestMethod, requestUrl, userAgent, protocol));
108+
}
109+
110+
public static void unsetCurrentRequest() {
111+
request.remove();
112+
}
113+
93114
/**
94115
* Get the Trace ID associated with any logging done by the current thread.
95116
*
@@ -156,6 +177,9 @@ public String format(LogRecord record) {
156177
if (traceId.get() != null) {
157178
json.put(TRACE, traceId.get());
158179
}
180+
if (request.get() != null) {
181+
json.put(HTTP_REQUEST, request.get());
182+
}
159183
// This trailing newline is required for the proxy because otherwise multiple logs might be
160184
// sent to Stackdriver together (due to the async nature of the proxy), and not parsed
161185
// correctly.
@@ -181,4 +205,7 @@ private static String severityFor(Level level) {
181205
default -> "DEFAULT";
182206
};
183207
}
208+
209+
private record HttpRequest(
210+
String requestMethod, String requestUrl, String userAgent, String protocol) {}
184211
}

util/src/test/java/google/registry/util/GcpJsonFormatterTest.java

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ class GcpJsonFormatterTest {
4545

4646
private static final String LOG_TEMPLATE =
4747
"""
48-
{"severity":"@@SEVERITY@@","logging.googleapis.com/sourceLocation":{"file":"GcpJsonFormatterTest.java","line":"@@LINE@@","function":"google.registry.util.GcpJsonFormatterTest.@@FUNCTION@@"},"message":"@@MESSAGE@@"}
49-
""";
48+
{"severity":"@@SEVERITY@@","logging.googleapis.com/sourceLocation":{"file":"GcpJsonFormatterTest.java","line":"@@LINE@@","function":"google.registry.util.GcpJsonFormatterTest.@@FUNCTION@@"},"message":"@@MESSAGE@@"}
49+
""";
5050

5151
private static String makeJson(String severity, int line, String function, String message) {
5252
return LOG_TEMPLATE
@@ -71,14 +71,15 @@ void beforeEach() {
7171
void afterEach() {
7272
jdkLogger.removeHandler(handler);
7373
GcpJsonFormatter.setCurrentTraceId(null);
74+
GcpJsonFormatter.unsetCurrentRequest();
7475
}
7576

7677
@Test
7778
void testSuccess() {
7879
logger.atInfo().log("Something I have to say");
7980
handler.close();
8081
String output = ostream.toString(StandardCharsets.US_ASCII);
81-
assertThat(output).isEqualTo(makeJson("INFO", 78, "testSuccess", "Something I have to say"));
82+
assertThat(output).isEqualTo(makeJson("INFO", 79, "testSuccess", "Something I have to say"));
8283
}
8384

8485
@Test
@@ -87,21 +88,37 @@ void testSuccess_traceId() {
8788
logger.atInfo().log("Something I have to say");
8889
handler.close();
8990
String output = ostream.toString(StandardCharsets.US_ASCII);
90-
String expected = makeJson("INFO", 87, "testSuccess_traceId", "Something I have to say");
91+
String expected = makeJson("INFO", 88, "testSuccess_traceId", "Something I have to say");
9192
// Remove the last two characters (}, \n) from the template and add the trace ID.
9293
expected =
9394
expected.substring(0, expected.length() - 2)
9495
+ ",\"logging.googleapis.com/trace\":\"trace_id\"}\n";
9596
assertThat(output).isEqualTo(expected);
9697
}
9798

99+
@Test
100+
void testSuccess_currentRequest() {
101+
GcpJsonFormatter.setCurrentRequest("GET", "/path", "My-Agent", "HTTP/1.1");
102+
logger.atInfo().log("Something I have to say");
103+
handler.close();
104+
String output = ostream.toString(StandardCharsets.US_ASCII);
105+
String expected =
106+
makeJson("INFO", 102, "testSuccess_currentRequest", "Something I have to say");
107+
// Remove the last two characters (}, \n) from the template and add the request.
108+
expected =
109+
expected.substring(0, expected.length() - 2)
110+
+ ",\"httRequest\":{\"requestMethod\":\"GET\",\"requestUrl\":\"/path\""
111+
+ ",\"userAgent\":\"My-Agent\",\"protocol\":\"HTTP/1.1\"}}\n";
112+
assertThat(output).isEqualTo(expected);
113+
}
114+
98115
@Test
99116
void testSuccess_logLevel() {
100117
logger.atSevere().log("Something went terribly wrong");
101118
handler.close();
102119
String output = ostream.toString(StandardCharsets.US_ASCII);
103120
assertThat(output)
104-
.isEqualTo(makeJson("ERROR", 100, "testSuccess_logLevel", "Something went terribly wrong"));
121+
.isEqualTo(makeJson("ERROR", 117, "testSuccess_logLevel", "Something went terribly wrong"));
105122
}
106123

107124
@Test
@@ -110,7 +127,7 @@ void testSuccess_withCause() {
110127
handler.close();
111128
String output = ostream.toString(StandardCharsets.US_ASCII);
112129
String prefix =
113-
makeJson("ERROR", 109, "testSuccess_withCause", "Something went terribly wrong");
130+
makeJson("ERROR", 126, "testSuccess_withCause", "Something went terribly wrong");
114131
// Remove the last two characters (}, \n) from the template as the actual output contains
115132
// the full stack trace.
116133
prefix = prefix.substring(0, prefix.length() - 2);
@@ -124,7 +141,7 @@ void testSuccess_withStackTrace() {
124141
handler.close();
125142
String output = ostream.toString(StandardCharsets.US_ASCII);
126143
String prefix =
127-
makeJson("ERROR", 123, "testSuccess_withStackTrace", "Something is worth checking");
144+
makeJson("ERROR", 140, "testSuccess_withStackTrace", "Something is worth checking");
128145
// Remove the last three characters (}, \n) from the template as the actual output contains
129146
// the full stack trace.
130147
prefix = prefix.substring(0, prefix.length() - 2);
@@ -150,7 +167,7 @@ void testSuccess_timeLimiter() throws Exception {
150167
TimeLimiter.create().callWithTimeout(this::logSomething, 1, TimeUnit.SECONDS);
151168
handler.close();
152169
String output = ostream.toString(StandardCharsets.US_ASCII);
153-
assertThat(output).isEqualTo(makeJson("INFO", 171, "logSomething", "Something I have to say"));
170+
assertThat(output).isEqualTo(makeJson("INFO", 188, "logSomething", "Something I have to say"));
154171
}
155172

156173
@Test
@@ -159,7 +176,7 @@ void testSuccess_timeLimiter_traceId() throws Exception {
159176
TimeLimiter.create().callWithTimeout(this::logSomething, 1, TimeUnit.SECONDS);
160177
handler.close();
161178
String output = ostream.toString(StandardCharsets.US_ASCII);
162-
String expected = makeJson("INFO", 171, "logSomething", "Something I have to say");
179+
String expected = makeJson("INFO", 188, "logSomething", "Something I have to say");
163180
// Remove the last two characters (}, \n) from the template and add the trace ID.
164181
expected =
165182
expected.substring(0, expected.length() - 2)

0 commit comments

Comments
 (0)