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

feat: add client logging with slf4j #1586

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
c38282b
add slf4j and other test dep. add logging utils. add logging to req/r…
zhumin8 Dec 5, 2024
3f30267
remove JUL wrapper as fallback condition. Other test setups.
zhumin8 Dec 5, 2024
35d9a75
remove unused LoggingInterceptor.
zhumin8 Dec 5, 2024
a63d6fe
revert unintential change to test. add comments.
zhumin8 Dec 5, 2024
83329e7
remove public from LoggingUtils.
zhumin8 Dec 5, 2024
6a5d3d3
downgrade logback and remove unused.
zhumin8 Dec 5, 2024
930d0aa
put logging inside try-catch block to avoid crashing for logging errors.
zhumin8 Dec 5, 2024
44f3de8
simplify logic for getLogger. remove unused argLine for maven-surefir…
zhumin8 Dec 5, 2024
598d5b9
logging util class refactor.
zhumin8 Dec 5, 2024
445fe08
fix test setup timing issue.
zhumin8 Dec 5, 2024
a7f5a5e
add tests. feedback. minor cleanups.
zhumin8 Dec 7, 2024
8bc1ae7
TestAppender change to per test. add LoggingTest.
zhumin8 Dec 7, 2024
bde6ede
update messages.
zhumin8 Dec 9, 2024
7c89c3f
test setup - try use env for all test in ci.
zhumin8 Dec 10, 2024
6da450d
add env set to sonar test.
zhumin8 Dec 10, 2024
9e20be9
duplicate fields to log in message. add tests.
zhumin8 Dec 10, 2024
db99e70
add tests.
zhumin8 Dec 10, 2024
68307fc
minor cleanup and comment.
zhumin8 Dec 10, 2024
4202902
fix add env to sonar ci.
zhumin8 Dec 10, 2024
cfbe873
add log env to integration in build.sh
zhumin8 Dec 10, 2024
858b725
deps: add gson explicitly. move gson and slf4j-api version to parent …
zhumin8 Dec 10, 2024
fb8889a
deps: explicitly declare org.hamcrest:hamcrest-core:test
zhumin8 Dec 10, 2024
15d3016
update logging messages to rm 'auth'.
zhumin8 Dec 10, 2024
c2a8d76
make sensitive key comparison case insensitive.
zhumin8 Dec 11, 2024
6485526
extract env var name.
zhumin8 Dec 11, 2024
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
3 changes: 3 additions & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ jobs:
- run: .kokoro/build.sh
env:
JOB_TYPE: test
GOOGLE_SDK_JAVA_LOGGING: true
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for next release: this env var is probably only needed for LoggingTest.java class.

units-java8:
# Building using Java 17 and run the tests with Java 8 runtime
name: "units (8)"
Expand All @@ -58,6 +59,7 @@ jobs:
- run: .kokoro/build.sh
env:
JOB_TYPE: test
GOOGLE_SDK_JAVA_LOGGING: true
windows:
runs-on: windows-latest
steps:
Expand All @@ -72,6 +74,7 @@ jobs:
- run: .kokoro/build.bat
env:
JOB_TYPE: test
GOOGLE_SDK_JAVA_LOGGING: true
dependencies:
runs-on: ubuntu-latest
strategy:
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/sonar.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ jobs:
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Needed to get PR information, if any
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
GOOGLE_SDK_JAVA_LOGGING: true
run: |
mvn -B verify -Dcheckstyle.skip \
-DenableFullTestCoverage \
Expand Down
1 change: 1 addition & 0 deletions .kokoro/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ javadoc)
RETURN_CODE=$?
;;
integration)
export GOOGLE_SDK_JAVA_LOGGING=true
mvn -B ${INTEGRATION_TEST_ARGS} \
-ntp \
-Penable-integration-tests \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ public class ComputeEngineCredentials extends GoogleCredentials
static final Duration COMPUTE_REFRESH_MARGIN = Duration.ofMinutes(3).plusSeconds(45);

private static final Logger LOGGER = Logger.getLogger(ComputeEngineCredentials.class.getName());
private static final org.slf4j.Logger SLF4JLOGGER =
LoggingConfigs.getLogger(ComputeEngineCredentials.class);
lqiu96 marked this conversation as resolved.
Show resolved Hide resolved

static final String DEFAULT_METADATA_SERVER_URL = "http://metadata.google.internal";

Expand Down Expand Up @@ -296,11 +298,14 @@ public AccessToken refreshAccessToken() throws IOException {
throw new IOException("Empty content from metadata token server request.");
}
GenericData responseData = response.parseAs(GenericData.class);
LoggingUtils.logGenericData(
responseData, SLF4JLOGGER, "Auth response from refresh access token payload");
String accessToken =
OAuth2Utils.validateString(responseData, "access_token", PARSE_ERROR_PREFIX);
int expiresInSeconds =
OAuth2Utils.validateInt32(responseData, "expires_in", PARSE_ERROR_PREFIX);
long expiresAtMilliseconds = clock.currentTimeMillis() + expiresInSeconds * 1000;

return new AccessToken(accessToken, new Date(expiresAtMilliseconds));
}

Expand Down Expand Up @@ -361,7 +366,23 @@ private HttpResponse getMetadataResponse(
request.setThrowExceptionOnExecuteError(false);
HttpResponse response;
try {
String requestMessage;
String responseMessage;
if (requestType.equals(RequestType.ID_TOKEN_REQUEST)) {
requestMessage = "Auth get metadata sending request for id token";
responseMessage = "Auth get metadata received response for id token";
} else if (requestType.equals(RequestType.ACCESS_TOKEN_REQUEST)) {
requestMessage = "Auth get metadata sending request for access token";
responseMessage = "Auth get metadata received response for access token";
} else {
// TODO: this includes get universe domain and get default sa.
// refactor for more clear logging message.
requestMessage = "Auth get metadata sending request";
responseMessage = "Auth get metadata received response";
}
lqiu96 marked this conversation as resolved.
Show resolved Hide resolved
LoggingUtils.logRequest(request, SLF4JLOGGER, requestMessage);
response = request.execute();
LoggingUtils.logResponse(response, SLF4JLOGGER, responseMessage);
} catch (UnknownHostException exception) {
throw new IOException(
"ComputeEngineCredentials cannot find the metadata server. This is"
Expand Down Expand Up @@ -461,7 +482,11 @@ private static boolean pingComputeEngineMetadata(
request,
MetricsUtils.getGoogleCredentialsMetricsHeader(
RequestType.METADATA_SERVER_PING, CredentialTypeForMetrics.DO_NOT_SEND));

LoggingUtils.logRequest(request, SLF4JLOGGER, "Pin auth Metadata Server");
lqiu96 marked this conversation as resolved.
Show resolved Hide resolved
HttpResponse response = request.execute();
LoggingUtils.logResponse(
response, SLF4JLOGGER, "Received auth response from Metadata Server");
lqiu96 marked this conversation as resolved.
Show resolved Hide resolved
try {
// Internet providers can return a generic response to all requests, so it is necessary
// to check that metadata header is present also.
Expand Down Expand Up @@ -633,6 +658,8 @@ private String getDefaultServiceAccount() throws IOException {
throw new IOException("Empty content from metadata token server request.");
}
GenericData responseData = response.parseAs(GenericData.class);
LoggingUtils.logGenericData(
responseData, SLF4JLOGGER, "Auth get default service account payload");
Map<String, Object> defaultAccount =
OAuth2Utils.validateMap(responseData, "default", PARSE_ERROR_ACCOUNT);
return OAuth2Utils.validateString(defaultAccount, "email", PARSE_ERROR_ACCOUNT);
Expand Down
12 changes: 12 additions & 0 deletions oauth2_http/java/com/google/auth/oauth2/IamUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import org.slf4j.Logger;

/**
* This internal class provides shared utilities for interacting with the IAM API for common
Expand All @@ -68,6 +69,7 @@ class IamUtils {
"https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/%s:generateIdToken";
private static final String PARSE_ERROR_MESSAGE = "Error parsing error message response. ";
private static final String PARSE_ERROR_SIGNATURE = "Error parsing signature response. ";
private static final Logger LOGGER = LoggingConfigs.getLogger(IamUtils.class);

// Following guidance for IAM retries:
// https://cloud.google.com/iam/docs/retry-strategy#errors-to-retry
Expand Down Expand Up @@ -142,7 +144,11 @@ private static String getSignature(
IamUtils.IAM_RETRYABLE_STATUS_CODES.contains(response.getStatusCode())));
request.setIOExceptionHandler(new HttpBackOffIOExceptionHandler(backoff));

LoggingUtils.logRequest(
request, LOGGER, "Sending auth request to get signature to sign the blob");
HttpResponse response = request.execute();
LoggingUtils.logResponse(
response, LOGGER, "Received auth response for signature to sign the blob");
int statusCode = response.getStatusCode();
if (statusCode >= 400 && statusCode < HttpStatusCodes.STATUS_CODE_SERVER_ERROR) {
GenericData responseError = response.parseAs(GenericData.class);
Expand All @@ -169,6 +175,7 @@ private static String getSignature(
}

GenericData responseData = response.parseAs(GenericData.class);
LoggingUtils.logGenericData(responseData, LOGGER, "Auth response payload for sign blob");
return OAuth2Utils.validateString(responseData, "signedBlob", PARSE_ERROR_SIGNATURE);
}

Expand Down Expand Up @@ -220,7 +227,10 @@ static IdToken getIdToken(
MetricsUtils.getGoogleCredentialsMetricsHeader(
RequestType.ID_TOKEN_REQUEST, credentialTypeForMetrics));

LoggingUtils.logRequest(request, LOGGER, "Sending auth request to get id token");
HttpResponse response = request.execute();

LoggingUtils.logResponse(response, LOGGER, "Received auth response for id token");
int statusCode = response.getStatusCode();
if (statusCode >= 400 && statusCode < HttpStatusCodes.STATUS_CODE_SERVER_ERROR) {
GenericData responseError = response.parseAs(GenericData.class);
Expand All @@ -245,6 +255,8 @@ static IdToken getIdToken(
}

GenericJson responseData = response.parseAs(GenericJson.class);
LoggingUtils.logGenericData(
responseData, LOGGER, "Auth response data payload for id token request");
String rawToken = OAuth2Utils.validateString(responseData, "token", PARSE_ERROR_MESSAGE);
return IdToken.create(rawToken);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
import org.slf4j.Logger;

/**
* ImpersonatedCredentials allowing credentials issued to a user or service account to impersonate
Expand Down Expand Up @@ -110,6 +111,7 @@ public class ImpersonatedCredentials extends GoogleCredentials
private int lifetime;
private String iamEndpointOverride;
private final String transportFactoryClassName;
private static final Logger LOGGER = LoggingConfigs.getLogger(ImpersonatedCredentials.class);

private transient HttpTransportFactory transportFactory;

Expand Down Expand Up @@ -546,12 +548,15 @@ public AccessToken refreshAccessToken() throws IOException {

HttpResponse response = null;
try {
LoggingUtils.logRequest(request, LOGGER, "Sending auth request to refresh access token");
response = request.execute();
LoggingUtils.logResponse(response, LOGGER, "Received auth response for access token");
} catch (IOException e) {
throw new IOException("Error requesting access token", e);
}

GenericData responseData = response.parseAs(GenericData.class);
LoggingUtils.logGenericData(responseData, LOGGER, "Auth response payload for access token");
response.disconnect();

String accessToken =
Expand Down
80 changes: 80 additions & 0 deletions oauth2_http/java/com/google/auth/oauth2/LoggingConfigs.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/*
* 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.auth.oauth2;

import org.slf4j.ILoggerFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

class LoggingConfigs {

private static EnvironmentProvider environmentProvider = SystemEnvironmentProvider.getInstance();
private static final Logger NO_OP_LOGGER = org.slf4j.helpers.NOPLogger.NOP_LOGGER;
private static boolean loggingEnabled = isLoggingEnabled();
// expose this setter only for testing purposes
static void setEnvironmentProvider(EnvironmentProvider provider) {
environmentProvider = provider;
// Recalculate LOGGING_ENABLED after setting the new provider
loggingEnabled = isLoggingEnabled();
}
lqiu96 marked this conversation as resolved.
Show resolved Hide resolved

private LoggingConfigs() {}

static Logger getLogger(Class<?> clazz) {
return getLogger(clazz, new DefaultLoggerFactoryProvider());
}

// constructor with LoggerFactoryProvider to make testing easier
static Logger getLogger(Class<?> clazz, LoggerFactoryProvider factoryProvider) {
if (!loggingEnabled) {
// use SLF4j's NOP logger regardless of bindings
return NO_OP_LOGGER;
}
return factoryProvider.getLoggerFactory().getLogger(clazz.getName());
}

static boolean isLoggingEnabled() {
String enableLogging = environmentProvider.getEnv("GOOGLE_SDK_JAVA_LOGGING");
lqiu96 marked this conversation as resolved.
Show resolved Hide resolved
return "true".equalsIgnoreCase(enableLogging);
}

interface LoggerFactoryProvider {
ILoggerFactory getLoggerFactory();
}

static class DefaultLoggerFactoryProvider implements LoggerFactoryProvider {
@Override
public ILoggerFactory getLoggerFactory() {
return LoggerFactory.getILoggerFactory();
}
}
}
Loading
Loading