-
Notifications
You must be signed in to change notification settings - Fork 68
feat: add client side logging with slf4j #3403
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
Changes from 10 commits
59b5d73
d34ec88
b6417ae
0c169ad
9ced175
681f7d5
5c01ef2
6605ef3
ebbae15
543298d
aac66f2
487614c
cc3465b
211a3d1
2970210
32f8b7c
245c14d
8358497
97087b2
727a3e9
23bc111
bbe0700
3d7c7f9
b870d81
56a2870
83eedf0
d85c848
1919809
613b6c8
1169eaa
a6b5433
fb0966e
77939fe
04ef774
439e071
673c9fe
c2b607b
3df39fa
4190dc7
fefb436
7901d8a
20f9f5a
172701d
d25e742
a8d2f20
df97834
32dbd0e
37eb391
f95423b
8e01aa0
73210fa
b1386cb
ef26d30
0c466eb
7a23e2d
3821ec3
23f00e8
9f7d77c
86cc4d2
446910a
de01f37
e3862a8
427e820
6ff5479
82ead2b
b5b6e94
5db8884
56316d3
b7234e3
e7acafc
801cef6
cf8ecbc
19faf95
47f1212
420de1e
c222198
227857a
008048a
07888cb
20017cc
2d3ba3e
7c1ea76
7aa8333
d5c759d
1641f55
d86b7a0
f611c74
539031b
091ab38
b3b328e
6aece18
c3db3e7
a975fd5
b1af879
157921b
89da1b4
fad1865
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| package com.google.api.gax.grpc; | ||
|
|
||
| import com.google.api.gax.logging.LoggingUtils; | ||
| import com.google.gson.Gson; | ||
| import com.google.gson.JsonArray; | ||
| import com.google.gson.JsonObject; | ||
| import io.grpc.*; | ||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
| import java.util.UUID; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.event.Level; | ||
|
|
||
| public class GrpcLoggingInterceptor implements ClientInterceptor { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be package private. Same thing for
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With the current test setup, these needs to be public for testing purposes. I'll look into the test setup again
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bumping this now again. Does this still need to be public with InternalApi?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately, as conversations diverged to other topics since then, I did not got the chance to revamp the testing. As of now, these still need to be public with InternalApi.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this class has to be public due to testing purposes, would it make sense to add a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. unfortunately, adding |
||
|
|
||
| private static final Logger logger = LoggingUtils.getLogger(GrpcLoggingInterceptor.class); | ||
| private static final Gson gson = new Gson(); | ||
|
|
||
| @Override | ||
| public <ReqT, RespT> ClientCall<ReqT, RespT> interceptCall( | ||
| MethodDescriptor<ReqT, RespT> method, CallOptions callOptions, Channel next) { | ||
|
|
||
| Map<String, String> serviceAndRpc = new HashMap<>(); | ||
| Map<String, String> requestLogData = new HashMap<>(); | ||
| Map<String, String> responseLogData = new HashMap<>(); | ||
|
|
||
| // Initialize a JsonArray to hold all responses | ||
| JsonArray responsePayloads = new JsonArray(); | ||
| return new ForwardingClientCall.SimpleForwardingClientCall<ReqT, RespT>( | ||
| next.newCall(method, callOptions)) { | ||
|
|
||
| @Override | ||
| public void start(Listener<RespT> responseListener, Metadata headers) { | ||
| if (logger.isInfoEnabled()) { | ||
| String requestId = UUID.randomUUID().toString(); | ||
zhumin8 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| String serviceName = method.getServiceName(); | ||
| String methodName = method.getFullMethodName(); | ||
|
|
||
| serviceAndRpc.put("serviceName", serviceName); | ||
| serviceAndRpc.put("rpcName", methodName); | ||
| serviceAndRpc.put("requestId", requestId); | ||
| } | ||
| if (logger.isInfoEnabled() && !logger.isDebugEnabled()) { | ||
| LoggingUtils.logWithMDC(logger, Level.INFO, serviceAndRpc, "Sending gRPC request"); | ||
| } | ||
|
|
||
| if (logger.isDebugEnabled()) { | ||
| requestLogData.putAll(serviceAndRpc); | ||
|
|
||
| JsonObject requestHeaders = mapHeadersToJsonObject(headers); | ||
| requestLogData.put("request.headers", gson.toJson(requestHeaders)); | ||
| } | ||
|
|
||
| super.start( | ||
| new ForwardingClientCallListener.SimpleForwardingClientCallListener<RespT>( | ||
| responseListener) { | ||
| @Override | ||
| public void onMessage(RespT message) { | ||
| if (logger.isDebugEnabled()) { | ||
| // Add each message to the array | ||
| responsePayloads.add(gson.toJsonTree(message)); | ||
| } | ||
| super.onMessage(message); | ||
| } | ||
|
|
||
| @Override | ||
| public void onClose(Status status, Metadata trailers) { | ||
| if (logger.isInfoEnabled()) { | ||
| serviceAndRpc.put("response.status", status.getCode().name()); | ||
| responseLogData.putAll(serviceAndRpc); | ||
| } | ||
| if (logger.isInfoEnabled() && !logger.isDebugEnabled()) { | ||
| LoggingUtils.logWithMDC(logger, Level.INFO, serviceAndRpc, "Received response."); | ||
| } | ||
| if (logger.isDebugEnabled()) { | ||
| // Access and add response headers | ||
| JsonObject responseHeaders = mapHeadersToJsonObject(trailers); | ||
| responseLogData.put("response.headers", gson.toJson(responseHeaders)); | ||
| // Add the array of payloads to the responseLogData | ||
| responseLogData.put("response.payload", gson.toJson(responsePayloads)); | ||
|
|
||
| LoggingUtils.logWithMDC( | ||
| logger, Level.DEBUG, responseLogData, "Received response."); | ||
| } | ||
|
|
||
| super.onClose(status, trailers); | ||
| } | ||
| }, | ||
| headers); | ||
| } | ||
|
|
||
| @Override | ||
| public void sendMessage(ReqT message) { | ||
|
|
||
| if (logger.isDebugEnabled()) { | ||
| requestLogData.put("request.payload", gson.toJson(message)); | ||
| LoggingUtils.logWithMDC(logger, Level.DEBUG, requestLogData, "Sending gRPC request."); | ||
| } | ||
|
|
||
| super.sendMessage(message); | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| private static JsonObject mapHeadersToJsonObject(Metadata headers) { | ||
| JsonObject jsonHeaders = new JsonObject(); | ||
| headers | ||
| .keys() | ||
| .forEach( | ||
| key -> { | ||
| Metadata.Key<String> metadataKey = | ||
zhumin8 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| Metadata.Key.of(key, Metadata.ASCII_STRING_MARSHALLER); | ||
| String headerValue = headers.get(metadataKey); | ||
| jsonHeaders.addProperty(key, headerValue); | ||
| }); | ||
| return jsonHeaders; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -467,6 +467,7 @@ private ManagedChannel createSingleChannel() throws IOException { | |
| builder = | ||
| builder | ||
| .intercept(new GrpcChannelUUIDInterceptor()) | ||
| .intercept(new GrpcLoggingInterceptor()) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just a thought (I don't know if there are any downsides or if this would work): Could we gate adding the logging interceptor here? i.e. Check if the logging env var exists + is configured and only add an interceptor if so. I think there was some mention about not using interceptors (potentially) so it may not work in the future.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess there is no harm in adding a gate here for now.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was just thinking that having it gate there might be a way to remove all the |
||
| .intercept(headerInterceptor) | ||
| .intercept(metadataHandlerInterceptor) | ||
| .userAgent(headerInterceptor.getUserAgentHeader()) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,116 @@ | ||
| package com.google.api.gax.httpjson; | ||
|
|
||
| import com.google.api.gax.httpjson.ForwardingHttpJsonClientCall.SimpleForwardingHttpJsonClientCall; | ||
| import com.google.api.gax.logging.LoggingUtils; | ||
| import com.google.gson.Gson; | ||
| import com.google.gson.JsonArray; | ||
| import com.google.gson.JsonObject; | ||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
| import java.util.UUID; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.event.Level; | ||
|
|
||
| public class HttpJsonLoggingInterceptor implements HttpJsonClientInterceptor { | ||
|
|
||
| private static final Logger logger = LoggingUtils.getLogger(HttpJsonLoggingInterceptor.class); | ||
| private static final Gson gson = new Gson(); | ||
|
|
||
| @Override | ||
| public <ReqT, RespT> HttpJsonClientCall<ReqT, RespT> interceptCall( | ||
| ApiMethodDescriptor<ReqT, RespT> method, | ||
| HttpJsonCallOptions callOptions, | ||
| HttpJsonChannel next) { | ||
| Map<String, String> requestLogData = new HashMap<>(); | ||
|
|
||
| // Initialize a JsonArray to hold all responses | ||
| JsonArray responsePayloads = new JsonArray(); | ||
| String endpoint = ((ManagedHttpJsonChannel) next).getEndpoint(); | ||
| return new SimpleForwardingHttpJsonClientCall<ReqT, RespT>(next.newCall(method, callOptions)) { | ||
| @Override | ||
| public void start( | ||
| HttpJsonClientCall.Listener<RespT> responseListener, HttpJsonMetadata headers) { | ||
|
|
||
| Map<String, String> serviceAndRpc = new HashMap<>(); | ||
| if (logger.isInfoEnabled()) { | ||
| String requestId = UUID.randomUUID().toString(); | ||
| // Capture request details | ||
| String methodName = method.getFullMethodName(); | ||
| String httpMethod = method.getHttpMethod(); | ||
| serviceAndRpc.put("rpcName", methodName); | ||
| serviceAndRpc.put("requestId", requestId); | ||
| requestLogData.putAll(serviceAndRpc); | ||
| requestLogData.put("request.url", endpoint); | ||
| requestLogData.put("request.method", httpMethod); | ||
| } | ||
| if (logger.isDebugEnabled()) { | ||
| // Capture and log headers | ||
| JsonObject jsonHeaders = new JsonObject(); | ||
| headers | ||
| .getHeaders() | ||
| .forEach((key, value) -> jsonHeaders.addProperty(key, value.toString())); | ||
| requestLogData.put("request.headers", gson.toJson(jsonHeaders)); | ||
| } | ||
|
|
||
| if (logger.isInfoEnabled() && !logger.isDebugEnabled()) { | ||
| LoggingUtils.logWithMDC(logger, Level.INFO, serviceAndRpc, "Sending HTTP request"); | ||
| } | ||
| Map<String, String> responseLogData = new HashMap<>(); | ||
| super.start( | ||
| new HttpJsonClientCall.Listener<RespT>() { | ||
| @Override | ||
| public void onMessage(RespT message) { | ||
| if (logger.isDebugEnabled()) { | ||
| // Add each message to the array | ||
| responsePayloads.add(gson.toJsonTree(message)); | ||
| } | ||
| responseListener.onMessage(message); | ||
| } | ||
|
|
||
| @Override | ||
| public void onClose(int status, HttpJsonMetadata trailers) { | ||
| if (logger.isInfoEnabled()) { | ||
|
|
||
| serviceAndRpc.put("response.status", String.valueOf(status)); | ||
| responseLogData.putAll(serviceAndRpc); | ||
| } | ||
| if (logger.isInfoEnabled() && !logger.isDebugEnabled()) { | ||
| LoggingUtils.logWithMDC( | ||
| logger, Level.INFO, serviceAndRpc, "HTTP request finished."); | ||
| } | ||
| if (logger.isDebugEnabled()) { | ||
|
|
||
| JsonObject jsonHeaders = new JsonObject(); | ||
| headers | ||
| .getHeaders() | ||
| .forEach((key, value) -> jsonHeaders.addProperty(key, value.toString())); | ||
| responseLogData.put("response.headers", gson.toJson(jsonHeaders)); | ||
|
|
||
| // Add the array of payloads to the responseLogData | ||
| responseLogData.put("response.payload", gson.toJson(responsePayloads)); | ||
| LoggingUtils.logWithMDC( | ||
| logger, | ||
| Level.DEBUG, | ||
| responseLogData, | ||
| "Received response header and payload."); | ||
| } | ||
|
|
||
| responseListener.onClose(status, trailers); | ||
| } | ||
| }, | ||
| headers); | ||
| } | ||
|
|
||
| @Override | ||
| public void sendMessage(ReqT message) { | ||
| if (logger.isDebugEnabled()) { | ||
| requestLogData.put("request.payload", gson.toJson(message)); | ||
| LoggingUtils.logWithMDC( | ||
| logger, Level.DEBUG, requestLogData, "HTTP request header and payload."); | ||
| } | ||
|
|
||
| super.sendMessage(message); | ||
| } | ||
| }; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -196,6 +196,7 @@ private HttpJsonTransportChannel createChannel() throws IOException, GeneralSecu | |
| HttpJsonClientInterceptor headerInterceptor = | ||
| new HttpJsonHeaderInterceptor(headerProvider.getHeaders()); | ||
|
|
||
| channel = new ManagedHttpJsonInterceptorChannel(channel, new HttpJsonLoggingInterceptor()); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Can this work similar to grpcChannel where this either takes in a chain of I see this as package-private so we can do whatever is easier and modify it later as well.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current ManagedHttpJsonChannel.builder does not take |
||
| channel = new ManagedHttpJsonInterceptorChannel(channel, headerInterceptor); | ||
| if (interceptorProvider != null && interceptorProvider.getInterceptors() != null) { | ||
| for (HttpJsonClientInterceptor interceptor : interceptorProvider.getInterceptors()) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| /* | ||
| * Copyright 2024 Google LLC | ||
| * | ||
| * Redistribution and use in source and binary forms, with or without | ||
| * modification, are permitted provided that the following conditions are | ||
| * met: | ||
| * | ||
| * * Redistributions of source code must retain the above copyright | ||
| * notice, this list of conditions and the following disclaimer. | ||
| * * Redistributions in binary form must reproduce the above | ||
| * copyright notice, this list of conditions and the following disclaimer | ||
| * in the documentation and/or other materials provided with the | ||
| * distribution. | ||
| * * Neither the name of Google LLC nor the names of its | ||
| * contributors may be used to endorse or promote products derived from | ||
| * this software without specific prior written permission. | ||
| * | ||
| * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS | ||
| * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT | ||
| * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR | ||
| * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT | ||
| * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, | ||
| * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT | ||
| * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, | ||
| * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY | ||
| * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT | ||
| * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE | ||
| * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. | ||
| */ | ||
|
|
||
| package com.google.api.gax.logging; | ||
|
|
||
| import com.google.gson.Gson; | ||
| import com.google.gson.GsonBuilder; | ||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
| import java.util.logging.ConsoleHandler; | ||
| import java.util.logging.Handler; | ||
| import java.util.logging.LogRecord; | ||
|
|
||
| public class JsonContextMapHandler extends Handler { | ||
zhumin8 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| private final Gson gson = new GsonBuilder().setPrettyPrinting().create(); | ||
|
|
||
| @Override | ||
| public void publish(LogRecord record) { | ||
| Object[] params = record.getParameters(); | ||
|
|
||
| ConsoleHandler consoleHandler = new ConsoleHandler(); | ||
| if (params != null && params.length > 0 && params[0] instanceof Map) { | ||
| @SuppressWarnings("unchecked") | ||
| Map<String, String> contextMap = (Map<String, String>) params[0]; | ||
|
|
||
| // Create a map to hold all log data | ||
| Map<String, Object> logData = new HashMap<>(); | ||
| logData.put("message", record.getMessage()); | ||
| logData.put("severity", record.getLevel().getName()); | ||
| logData.put("timestamp", record.getMillis()); | ||
| logData.put("logger", record.getLoggerName()); | ||
|
|
||
| // Add all context data to the top level | ||
| logData.putAll(contextMap); | ||
|
|
||
| // Convert to JSON | ||
| String jsonLog = gson.toJson(logData); | ||
|
|
||
| LogRecord jsonRecord = new LogRecord(record.getLevel(), jsonLog); | ||
| consoleHandler.publish(jsonRecord); | ||
| } else { | ||
| // Handle cases where the context map is not present | ||
| LogRecord messageRecord = | ||
| new LogRecord(record.getLevel(), "Log message without context: " + record.getMessage()); | ||
| consoleHandler.publish(messageRecord); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void flush() {} | ||
|
|
||
| @Override | ||
| public void close() throws SecurityException {} | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.