This is an automated email from the ASF dual-hosted git repository. penghui pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/pulsar.git
The following commit(s) were added to refs/heads/master by this push: new 7ab29d8 [Issue 7933][pulsar-broker-common] Add some logging to improve Authentication debugging and Fix typos in code "occured" -> "occurred" (#7934) 7ab29d8 is described below commit 7ab29d85bbfc1004003c91a554d0dd92efec8092 Author: Frank J Kelly <kell...@gmail.com> AuthorDate: Wed Sep 2 09:13:54 2020 -0400 [Issue 7933][pulsar-broker-common] Add some logging to improve Authentication debugging and Fix typos in code "occured" -> "occurred" (#7934) Fixes #7933 ### Motivation Newbie trying to make his first contribution to this projection :-) ### Modifications Added some logging to Authentication Service to help debugging when there are more than one AuthN provider. When I did that I noticed a typo ('occured' should have two r's) so I decided to try to fix all of them --- .../pulsar/broker/authentication/AuthenticationService.java | 3 +++ .../pulsar/broker/authorization/AuthorizationService.java | 4 ++-- .../broker/authorization/PulsarAuthorizationProvider.java | 4 ++-- .../org/apache/pulsar/broker/service/AbstractReplicator.java | 2 +- .../java/org/apache/pulsar/broker/service/ServerCnx.java | 4 ++-- .../java/org/apache/pulsar/broker/SLAMonitoringTest.java | 2 +- .../org/apache/pulsar/client/admin/internal/TopicsImpl.java | 2 +- .../functions/worker/FunctionAssignmentTailerTest.java | 2 +- .../pulsar/functions/worker/FunctionRuntimeManagerTest.java | 2 +- .../pulsar/proxy/server/ProxyRolesEnforcementTest.java | 12 ++++++------ 10 files changed, 20 insertions(+), 17 deletions(-) diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java index 2db2222..2cefad9 100644 --- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java +++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java @@ -90,6 +90,9 @@ public class AuthenticationService implements Closeable { try { return provider.authenticate(authData); } catch (AuthenticationException e) { + if (LOG.isDebugEnabled()) { + LOG.debug("Authentication failed for provider " + provider.getAuthMethodName() + ": " + e.getMessage(), e); + } // Store the exception so we can throw it later instead of a generic one authenticationException = e; } diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/AuthorizationService.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/AuthorizationService.java index afa85ee..75c759f 100644 --- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/AuthorizationService.java +++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/AuthorizationService.java @@ -300,7 +300,7 @@ public class AuthorizationService { } else { if (log.isDebugEnabled()) { log.debug( - "Topic [{}] Role [{}] exception occured while trying to check Produce permissions. {}", + "Topic [{}] Role [{}] exception occurred while trying to check Produce permissions. {}", topicName.toString(), role, ex.getMessage()); } } @@ -313,7 +313,7 @@ public class AuthorizationService { } else { if (log.isDebugEnabled()) { log.debug( - "Topic [{}] Role [{}] exception occured while trying to check Consume permissions. {}", + "Topic [{}] Role [{}] exception occurred while trying to check Consume permissions. {}", topicName.toString(), role, e.getMessage()); } diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java index 66d0c2e..ca7ca9e 100644 --- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java +++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java @@ -191,7 +191,7 @@ public class PulsarAuthorizationProvider implements AuthorizationProvider { } else { if (log.isDebugEnabled()) { log.debug( - "Topic [{}] Role [{}] exception occured while trying to check Produce permissions. {}", + "Topic [{}] Role [{}] exception occurred while trying to check Produce permissions. {}", topicName.toString(), role, ex.getMessage()); } } @@ -204,7 +204,7 @@ public class PulsarAuthorizationProvider implements AuthorizationProvider { } else { if (log.isDebugEnabled()) { log.debug( - "Topic [{}] Role [{}] exception occured while trying to check Consume permissions. {}", + "Topic [{}] Role [{}] exception occurred while trying to check Consume permissions. {}", topicName.toString(), role, e.getMessage()); } diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractReplicator.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractReplicator.java index 3026a5d..a55d21a 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractReplicator.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractReplicator.java @@ -168,7 +168,7 @@ public abstract class AbstractReplicator { }).exceptionally(ex -> { long waitTimeMs = backOff.next(); log.warn( - "[{}][{} -> {}] Exception: '{}' occured while trying to close the producer. retrying again in {} s", + "[{}][{} -> {}] Exception: '{}' occurred while trying to close the producer. retrying again in {} s", topicName, localCluster, remoteCluster, ex.getMessage(), waitTimeMs / 1000.0); // BackOff before retrying brokerService.executor().schedule(this::closeProducerAsync, waitTimeMs, TimeUnit.MILLISECONDS); diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java index 8949db4..8d076ce 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java @@ -383,7 +383,7 @@ public class ServerCnx extends PulsarHandler { } return null; }).exceptionally(ex -> { - final String msg = "Exception occured while trying to authorize lookup"; + final String msg = "Exception occurred while trying to authorize lookup"; log.warn("[{}] {} with role {} on topic {}", remoteAddress, msg, getPrincipal(), topicName, ex); ctx.writeAndFlush(newLookupErrorResponse(ServerError.AuthorizationError, msg, requestId)); lookupSemaphore.release(); @@ -458,7 +458,7 @@ public class ServerCnx extends PulsarHandler { } return null; }).exceptionally(ex -> { - final String msg = "Exception occured while trying to authorize get Partition Metadata"; + final String msg = "Exception occurred while trying to authorize get Partition Metadata"; log.warn("[{}] {} with role {} on topic {}", remoteAddress, msg, getPrincipal(), topicName); ctx.writeAndFlush(Commands.newPartitionMetadataResponse(ServerError.AuthorizationError, msg, requestId)); lookupSemaphore.release(); diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/SLAMonitoringTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/SLAMonitoringTest.java index eb7a555..a5059a3 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/SLAMonitoringTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/SLAMonitoringTest.java @@ -138,7 +138,7 @@ public class SLAMonitoringTest { assertTrue(pulsarServices[0].getNamespaceService().registerSLANamespace()); } catch (PulsarServerException e) { e.printStackTrace(); - log.error("Exception occured", e); + log.error("Exception occurred", e); fail("SLA Namespace should have been owned by the broker, Exception.", e); } } diff --git a/pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/TopicsImpl.java b/pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/TopicsImpl.java index 2711b2f..6a9ae11 100644 --- a/pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/TopicsImpl.java +++ b/pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/TopicsImpl.java @@ -1348,7 +1348,7 @@ public class TopicsImpl extends BaseResource implements Topics { ret.add(new MessageImpl<>(topic, batchMsgId, properties, singleMessagePayload, Schema.BYTES, msgMetadataBuilder)); } catch (Exception ex) { - log.error("Exception occured while trying to get BatchMsgId: {}", batchMsgId, ex); + log.error("Exception occurred while trying to get BatchMsgId: {}", batchMsgId, ex); } singleMessageMetadataBuilder.recycle(); } diff --git a/pulsar-functions/worker/src/test/java/org/apache/pulsar/functions/worker/FunctionAssignmentTailerTest.java b/pulsar-functions/worker/src/test/java/org/apache/pulsar/functions/worker/FunctionAssignmentTailerTest.java index 28eb06a..e0850d4 100644 --- a/pulsar-functions/worker/src/test/java/org/apache/pulsar/functions/worker/FunctionAssignmentTailerTest.java +++ b/pulsar-functions/worker/src/test/java/org/apache/pulsar/functions/worker/FunctionAssignmentTailerTest.java @@ -151,7 +151,7 @@ public class FunctionAssignmentTailerTest { functionAssignmentTailer.start(); - // verify no errors occured + // verify no errors occurred verify(errorNotifier, times(0)).triggerError(any()); messageList.add(message1); diff --git a/pulsar-functions/worker/src/test/java/org/apache/pulsar/functions/worker/FunctionRuntimeManagerTest.java b/pulsar-functions/worker/src/test/java/org/apache/pulsar/functions/worker/FunctionRuntimeManagerTest.java index a3b39ed..be2cfa7 100644 --- a/pulsar-functions/worker/src/test/java/org/apache/pulsar/functions/worker/FunctionRuntimeManagerTest.java +++ b/pulsar-functions/worker/src/test/java/org/apache/pulsar/functions/worker/FunctionRuntimeManagerTest.java @@ -622,7 +622,7 @@ public class FunctionRuntimeManagerTest { Function.Instance.newBuilder().setFunctionMetaData(function1).setInstanceId(0) .build())); - // verify no errors occured + // verify no errors occurred verify(errorNotifier, times(0)).triggerError(any()); } diff --git a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyRolesEnforcementTest.java b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyRolesEnforcementTest.java index c13bb7a..57018ea 100644 --- a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyRolesEnforcementTest.java +++ b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyRolesEnforcementTest.java @@ -196,13 +196,13 @@ public class ProxyRolesEnforcementTest extends ProducerConsumerBase { // Step 2: Try to use proxy Client as a normal Client - expect exception PulsarClient proxyClient = createPulsarClient(pulsar.getBrokerServiceUrl(), proxyAuthParams); - boolean exceptionOccured = false; + boolean exceptionOccurred = false; try { proxyClient.newConsumer().topic(topicName).subscriptionName(subscriptionName).subscribe(); } catch (Exception ex) { - exceptionOccured = true; + exceptionOccurred = true; } - Assert.assertTrue(exceptionOccured); + Assert.assertTrue(exceptionOccurred); // Step 3: Run Pulsar Proxy and pass proxy params as client params - expect exception ProxyConfiguration proxyConfig = new ProxyConfiguration(); @@ -224,14 +224,14 @@ public class ProxyRolesEnforcementTest extends ProducerConsumerBase { proxyService.start(); proxyClient = createPulsarClient(proxyService.getServiceUrl(), proxyAuthParams); - exceptionOccured = false; + exceptionOccurred = false; try { proxyClient.newConsumer().topic(topicName).subscriptionName(subscriptionName).subscribe(); } catch (Exception ex) { - exceptionOccured = true; + exceptionOccurred = true; } - Assert.assertTrue(exceptionOccured); + Assert.assertTrue(exceptionOccurred); // Step 4: Pass correct client params proxyClient = createPulsarClient(proxyService.getServiceUrl(), clientAuthParams);