[ 
https://issues.apache.org/jira/browse/HADOOP-18167?focusedWorklogId=746914&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-746914
 ]

ASF GitHub Bot logged work on HADOOP-18167:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 24/Mar/22 00:56
            Start Date: 24/Mar/22 00:56
    Worklog Time Spent: 10m 
      Work Description: hchaverri commented on a change in pull request #4092:
URL: https://github.com/apache/hadoop/pull/4092#discussion_r833832354



##########
File path: 
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java
##########
@@ -429,11 +445,16 @@ private synchronized void removeExpiredKeys() {
     byte[] password = createPassword(identifier.getBytes(), 
currentKey.getKey());
     DelegationTokenInformation tokenInfo = new DelegationTokenInformation(now
         + tokenRenewInterval, password, getTrackingIdIfEnabled(identifier));
+    long start = Time.monotonicNow();
     try {
       storeToken(identifier, tokenInfo);
     } catch (IOException ioe) {
       LOG.error("Could not store token " + formatTokenId(identifier) + "!!",
           ioe);
+    } finally {
+      if (metrics != null) {
+        metrics.addStoreToken(Time.monotonicNow() - start);
+      }

Review comment:
       This can help us correlate how changes in latency affect the whole 
operation. For example, if calling storeToken times out after x seconds, we 
should be able to recognize it by looking at this metric. That's also the 
pattern that I see with RpcMetrics

##########
File path: 
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java
##########
@@ -825,4 +860,55 @@ protected void syncTokenOwnerStats() {
       addTokenForOwnerStats(id);
     }
   }
+
+  /**
+   * DelegationTokenSecretManagerMetrics tracks token management operations
+   * and publishes them through the metrics interfaces.
+   */
+  @Metrics(about="Delegation token secret manager metrics", context="token")
+  static class DelegationTokenSecretManagerMetrics implements 
IOStatisticsSource {
+    final static String STORE_TOKEN_STAT = "storeToken";
+    final static String UPDATE_TOKEN_STAT = "updateToken";
+    final static String REMOVE_TOKEN_STAT = "removeToken";
+
+    final MetricsRegistry registry = new 
MetricsRegistry("DelegationTokenSecretManagerMetrics");

Review comment:
       I'm mainly adding it now to reduce the risk of someone changing the 
registry name. If we don't add this, metrics will be registered with the class 
name ("DelegationTokenSecretManagerMetrics"). However, if someone decides to 
add the registry in the future, they could choose a different name and break 
something. I prefer to just have it available if someone ever needs it. I've 
seen the same pattern in other metrics classes (ClientSCMMetrics, 
SharedCacheUploaderMetrics,etc), but I'm open to change this if you see a 
bigger problem.
   
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Issue Time Tracking
-------------------

    Worklog Id:     (was: 746914)
    Time Spent: 1h  (was: 50m)

> Add metrics to track delegation token secret manager operations
> ---------------------------------------------------------------
>
>                 Key: HADOOP-18167
>                 URL: https://issues.apache.org/jira/browse/HADOOP-18167
>             Project: Hadoop Common
>          Issue Type: Improvement
>            Reporter: Hector Sandoval Chaverri
>            Priority: Major
>              Labels: pull-request-available
>         Attachments: HADOOP-18167-branch-2.10.patch
>
>          Time Spent: 1h
>  Remaining Estimate: 0h
>
> New metrics to track operations that store, update and remove delegation 
> tokens in implementations of AbstractDelegationTokenSecretManager. This will 
> help evaluate the impact of using different secret managers and add 
> optimizations.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to