From ed2eb7493f111eb3f82d42bf2c08e83af2b5a3b4 Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Tue, 4 May 2021 23:26:47 +0100 Subject: Introduce LDAP metrics LDAP performance can have a massive impact on the overall responsiveness and latency of Gerrit API. Expose the LDAP metrics in terms of latency and call rates so that any problem can be highlighted early on and potentially alerted to the Gerrit admin. Bug: Issue 14490 Change-Id: I18e5d5b797b272ca11a6745bc39dcd73cab68c34 --- Documentation/metrics.txt | 7 +++ .../com/google/gerrit/server/auth/ldap/Helper.java | 52 +++++++++++++++++++--- .../gerrit/server/auth/ldap/LdapGroupBackend.java | 3 +- .../google/gerrit/server/auth/ldap/LdapQuery.java | 8 +++- 4 files changed, 61 insertions(+), 9 deletions(-) diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt index 713fbff78b..e64d1de673 100644 --- a/Documentation/metrics.txt +++ b/Documentation/metrics.txt @@ -96,6 +96,13 @@ setting. * `http/server/jetty/threadpool/pool_size`: Current thread pool size * `http/server/jetty/threadpool/queue_size`: Queued requests waiting for a thread +==== LDAP + +* `ldap/login_latency`: Latency of logins. +* `ldap/user_search_latency`: Latency for searching the user account. +* `ldap/group_search_latency`: Latency for querying the group memberships of an account. +* `ldap/group_expansion_latency`: Latency for expanding nested groups. + ==== REST API * `http/server/error_count`: Rate of REST API error responses. diff --git a/java/com/google/gerrit/server/auth/ldap/Helper.java b/java/com/google/gerrit/server/auth/ldap/Helper.java index 5c6b391369..b0f011a3a6 100644 --- a/java/com/google/gerrit/server/auth/ldap/Helper.java +++ b/java/com/google/gerrit/server/auth/ldap/Helper.java @@ -20,6 +20,10 @@ import com.google.common.collect.ImmutableSet; import com.google.common.flogger.FluentLogger; import com.google.gerrit.common.data.ParameterizedString; import com.google.gerrit.entities.AccountGroup; +import com.google.gerrit.metrics.Description; +import com.google.gerrit.metrics.Description.Units; +import com.google.gerrit.metrics.MetricMaker; +import com.google.gerrit.metrics.Timer0; import com.google.gerrit.server.account.AccountException; import com.google.gerrit.server.account.AuthenticationFailedException; import com.google.gerrit.server.auth.NoSuchUserException; @@ -81,11 +85,16 @@ class Helper { private final String connectTimeoutMillis; private final boolean useConnectionPooling; private final boolean groupsVisibleToAll; + private final Timer0 loginLatencyTimer; + private final Timer0 userSearchLatencyTimer; + private final Timer0 groupSearchLatencyTimer; + private final Timer0 groupExpansionLatencyTimer; @Inject Helper( @GerritServerConfig Config config, - @Named(LdapModule.PARENT_GROUPS_CACHE) Cache> parentGroups) { + @Named(LdapModule.PARENT_GROUPS_CACHE) Cache> parentGroups, + MetricMaker metricMaker) { this.config = config; this.server = LdapRealm.optional(config, "server"); this.username = LdapRealm.optional(config, "username"); @@ -112,6 +121,33 @@ class Helper { } this.parentGroups = parentGroups; this.useConnectionPooling = LdapRealm.optional(config, "useConnectionPooling", false); + + this.loginLatencyTimer = + metricMaker.newTimer( + "ldap/login_latency", + new Description("Latency of logins").setCumulative().setUnit(Units.NANOSECONDS)); + this.userSearchLatencyTimer = + metricMaker.newTimer( + "ldap/user_search_latency", + new Description("Latency for searching the user account") + .setCumulative() + .setUnit(Units.NANOSECONDS)); + this.groupSearchLatencyTimer = + metricMaker.newTimer( + "ldap/group_search_latency", + new Description("Latency for querying the groups membership of an account") + .setCumulative() + .setUnit(Units.NANOSECONDS)); + this.groupExpansionLatencyTimer = + metricMaker.newTimer( + "ldap/group_expansion_latency", + new Description("Latency for expanding nested groups") + .setCumulative() + .setUnit(Units.NANOSECONDS)); + } + + Timer0 getGroupSearchLatencyTimer() { + return groupSearchLatencyTimer; } private Properties createContextProperties() { @@ -191,7 +227,9 @@ class Helper { private DirContext kerberosOpen(Properties env) throws IOException, LoginException, NamingException { LoginContext ctx = new LoginContext("KerberosLogin"); - ctx.login(); + try (Timer0.Context ignored = loginLatencyTimer.start()) { + ctx.login(); + } Subject subject = ctx.getSubject(); try { return Subject.doAs( @@ -209,7 +247,7 @@ class Helper { DirContext authenticate(String dn, String password) throws AccountException { final Properties env = createContextProperties(); - try { + try (Timer0.Context ignored = loginLatencyTimer.start()) { env.put(Context.REFERRAL, referral); if (!supportAnonymous) { @@ -258,7 +296,7 @@ class Helper { } for (LdapQuery accountQuery : accountQueryList) { - List res = accountQuery.query(ctx, params); + List res = accountQuery.query(ctx, params, userSearchLatencyTimer); if (res.size() == 1) { return res.get(0); } else if (res.size() > 1) { @@ -290,8 +328,10 @@ class Helper { params.put(LdapRealm.USERNAME, username); for (LdapQuery groupMemberQuery : schema.groupMemberQueryList) { - for (LdapQuery.Result r : groupMemberQuery.query(ctx, params)) { - recursivelyExpandGroups(groupDNs, schema, ctx, r.getDN()); + for (LdapQuery.Result r : groupMemberQuery.query(ctx, params, groupSearchLatencyTimer)) { + try (Timer0.Context ignored = groupExpansionLatencyTimer.start()) { + recursivelyExpandGroups(groupDNs, schema, ctx, r.getDN()); + } } } } diff --git a/java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java b/java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java index 1d85a5eb7a..0d8f3f8a95 100644 --- a/java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java +++ b/java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java @@ -213,7 +213,8 @@ public class LdapGroupBackend implements GroupBackend { Map params = Collections.emptyMap(); for (String groupBase : schema.groupBases) { LdapQuery query = new LdapQuery(groupBase, schema.groupScope, filter, returnAttrs); - for (LdapQuery.Result res : query.query(ctx, params)) { + for (LdapQuery.Result res : + query.query(ctx, params, helper.getGroupSearchLatencyTimer())) { out.add(groupReference(schema.groupName, res)); } } diff --git a/java/com/google/gerrit/server/auth/ldap/LdapQuery.java b/java/com/google/gerrit/server/auth/ldap/LdapQuery.java index 3d25e864b5..3e549f6fdd 100644 --- a/java/com/google/gerrit/server/auth/ldap/LdapQuery.java +++ b/java/com/google/gerrit/server/auth/ldap/LdapQuery.java @@ -15,6 +15,7 @@ package com.google.gerrit.server.auth.ldap; import com.google.gerrit.common.data.ParameterizedString; +import com.google.gerrit.metrics.Timer0; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -61,13 +62,16 @@ class LdapQuery { return pattern.getParameterNames(); } - List query(DirContext ctx, Map params) throws NamingException { + List query(DirContext ctx, Map params, Timer0 queryTimer) + throws NamingException { final SearchControls sc = new SearchControls(); final NamingEnumeration res; sc.setSearchScope(searchScope.scope()); sc.setReturningAttributes(returnAttributes); - res = ctx.search(base, pattern.getRawPattern(), pattern.bind(params), sc); + try (Timer0.Context ignored = queryTimer.start()) { + res = ctx.search(base, pattern.getRawPattern(), pattern.bind(params), sc); + } try { final List r = new ArrayList<>(); try { -- cgit v1.2.3