diff options
author | Luca Milanesio <luca.milanesio@gmail.com> | 2021-05-04 23:26:47 +0100 |
---|---|---|
committer | David Ostrovsky <david.ostrovsky@gmail.com> | 2021-05-06 12:34:34 +0000 |
commit | ed2eb7493f111eb3f82d42bf2c08e83af2b5a3b4 (patch) | |
tree | 4466afbd3e1ca67fd528d54bd9b21051cf601240 | |
parent | 85510403edb60ee6146ce0354137f1b8028c8522 (diff) |
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
-rw-r--r-- | Documentation/metrics.txt | 7 | ||||
-rw-r--r-- | java/com/google/gerrit/server/auth/ldap/Helper.java | 52 | ||||
-rw-r--r-- | java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java | 3 | ||||
-rw-r--r-- | java/com/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<String, ImmutableSet<String>> parentGroups) { + @Named(LdapModule.PARENT_GROUPS_CACHE) Cache<String, ImmutableSet<String>> 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<LdapQuery.Result> res = accountQuery.query(ctx, params); + List<LdapQuery.Result> 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<String, String> 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<Result> query(DirContext ctx, Map<String, String> params) throws NamingException { + List<Result> query(DirContext ctx, Map<String, String> params, Timer0 queryTimer) + throws NamingException { final SearchControls sc = new SearchControls(); final NamingEnumeration<SearchResult> 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<Result> r = new ArrayList<>(); try { |