Remove CallerFinder from Gerrit
CallerFinder internally throws an exception to traverse the stack trace
and in turn find the caller. This is expensive (anywhere between 50ms
and 100ms) per incovation. If called inside a component with many
invocations it can easily lead to substantial latency regressions.
This is too easy to get wrong (i.e. call without understanding the
latency penalty it carries). Hence it is better to remove it
completely from the code base.
Release-Notes: Solves a latency regression related to CallerFinder
Change-Id: I04acf0b15cd1d18163fe5eb2ec0f1d6f25f169c9
diff --git a/java/com/google/gerrit/index/query/QueryProcessor.java b/java/com/google/gerrit/index/query/QueryProcessor.java
index b8eb8bb..cfb9f33 100644
--- a/java/com/google/gerrit/index/query/QueryProcessor.java
+++ b/java/com/google/gerrit/index/query/QueryProcessor.java
@@ -40,7 +40,6 @@
import com.google.gerrit.metrics.Field;
import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.metrics.Timer1;
-import com.google.gerrit.server.logging.CallerFinder;
import com.google.gerrit.server.logging.Metadata;
import java.util.ArrayList;
import java.util.List;
@@ -83,7 +82,6 @@
private final IndexRewriter<T> rewriter;
private final String limitField;
private final IntSupplier userQueryLimit;
- private final CallerFinder callerFinder;
// This class is not generally thread-safe, but programmer error may result in it being shared
// across threads. At least ensure the bit for checking if it's been used is threadsafe.
@@ -113,13 +111,6 @@
this.limitField = limitField;
this.userQueryLimit = userQueryLimit;
this.used = new AtomicBoolean(false);
- this.callerFinder =
- CallerFinder.builder()
- .addTarget(InternalQuery.class)
- .addTarget(QueryProcessor.class)
- .matchSubClasses(true)
- .skip(1)
- .build();
}
@CanIgnoreReturnValue
@@ -242,9 +233,7 @@
return disabledResults(queryStrings, queries);
}
- logger.atFine().log(
- "Executing %d %s index queries for %s",
- cnt, schemaDef.getName(), callerFinder.findCallerLazy());
+ logger.atFine().log("Executing %d %s index queries", cnt, schemaDef.getName());
List<QueryResult<T>> out;
try {
// Parse and rewrite all queries.
diff --git a/java/com/google/gerrit/server/account/AccountCacheImpl.java b/java/com/google/gerrit/server/account/AccountCacheImpl.java
index 9a535ef..d269b71 100644
--- a/java/com/google/gerrit/server/account/AccountCacheImpl.java
+++ b/java/com/google/gerrit/server/account/AccountCacheImpl.java
@@ -26,7 +26,6 @@
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.exceptions.StorageException;
-import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.ModuleImpl;
import com.google.gerrit.server.account.externalids.ExternalIdKeyFactory;
import com.google.gerrit.server.account.externalids.ExternalIds;
@@ -35,7 +34,6 @@
import com.google.gerrit.server.config.CachedPreferences;
import com.google.gerrit.server.config.DefaultPreferencesCache;
import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.logging.CallerFinder;
import com.google.gerrit.server.logging.Metadata;
import com.google.gerrit.server.logging.TraceContext;
import com.google.gerrit.server.logging.TraceContext.TraceTimer;
@@ -151,8 +149,7 @@
public Map<Account.Id, AccountState> get(Set<Account.Id> accountIds) {
try (TraceTimer ignored =
TraceContext.newTimer(
- "Loading accounts",
- Metadata.builder().caller(findCaller()).resourceCount(accountIds.size()).build())) {
+ "Loading accounts", Metadata.builder().resourceCount(accountIds.size()).build())) {
try (Repository allUsers = repoManager.openRepository(allUsersName)) {
Set<CachedAccountDetails.Key> keys =
Sets.newLinkedHashSetWithExpectedSize(accountIds.size());
@@ -197,17 +194,6 @@
return AccountState.forAccount(account.build());
}
- private String findCaller() {
- return CallerFinder.builder()
- .addTarget(AccountLoader.class)
- .addTarget(InternalAccountDirectory.class)
- .addTarget(AccountResolver.class)
- .addTarget(IdentifiedUser.class)
- .addTarget(AccountCacheImpl.class)
- .build()
- .findCaller();
- }
-
@Singleton
static class Loader extends CacheLoader<CachedAccountDetails.Key, CachedAccountDetails> {
private final GitRepositoryManager repoManager;
diff --git a/java/com/google/gerrit/server/account/externalids/storage/notedb/ExternalIdNotes.java b/java/com/google/gerrit/server/account/externalids/storage/notedb/ExternalIdNotes.java
index f0632e4..6137884 100644
--- a/java/com/google/gerrit/server/account/externalids/storage/notedb/ExternalIdNotes.java
+++ b/java/com/google/gerrit/server/account/externalids/storage/notedb/ExternalIdNotes.java
@@ -41,14 +41,11 @@
import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.account.externalids.ExternalIdCache;
import com.google.gerrit.server.account.externalids.ExternalIdUpsertPreprocessor;
-import com.google.gerrit.server.account.externalids.ExternalIds;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
import com.google.gerrit.server.git.meta.VersionedMetaData;
import com.google.gerrit.server.index.account.AccountIndexer;
-import com.google.gerrit.server.logging.CallerFinder;
-import com.google.gerrit.server.update.RetryHelper;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -361,7 +358,6 @@
private final Counter0 updateCount;
private final Repository repo;
private final DynamicMap<ExternalIdUpsertPreprocessor> upsertPreprocessors;
- private final CallerFinder callerFinder;
private final ExternalIdFactoryNoteDbImpl externalIdFactory;
private NoteMap noteMap;
@@ -415,19 +411,6 @@
this.allUsersName = requireNonNull(allUsersName, "allUsersRepo");
this.repo = requireNonNull(allUsersRepo, "allUsersRepo");
this.upsertPreprocessors = upsertPreprocessors;
- this.callerFinder =
- CallerFinder.builder()
- // 1. callers that come through ExternalIds
- .addTarget(ExternalIds.class)
-
- // 2. callers that come through AccountsUpdate
- .addTarget(AccountsUpdate.class)
- .addIgnoredPackage("com.github.rholder.retry")
- .addIgnoredClass(RetryHelper.class)
-
- // 3. direct callers
- .addTarget(ExternalIdNotes.class)
- .build();
this.externalIdFactory = externalIdFactory;
this.isUserNameCaseInsensitiveMigrationMode = isUserNameCaseInsensitiveMigrationMode;
}
@@ -852,8 +835,6 @@
@Override
protected void onLoad() throws IOException, ConfigInvalidException {
if (revision != null) {
- logger.atFine().log(
- "Reading external ID note map (caller: %s)", callerFinder.findCallerLazy());
noteMap = NoteMap.read(reader, revision);
} else {
noteMap = NoteMap.newEmptyMap();
diff --git a/java/com/google/gerrit/server/change/RebaseChangeOp.java b/java/com/google/gerrit/server/change/RebaseChangeOp.java
index 5daac75..a430034 100644
--- a/java/com/google/gerrit/server/change/RebaseChangeOp.java
+++ b/java/com/google/gerrit/server/change/RebaseChangeOp.java
@@ -45,7 +45,6 @@
import com.google.gerrit.server.git.GroupCollector;
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.git.MergeUtilFactory;
-import com.google.gerrit.server.logging.CallerFinder;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.patch.DiffNotAvailableException;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -99,7 +98,6 @@
private final RebaseUtil rebaseUtil;
private final ChangeResource.Factory changeResourceFactory;
private final ChangeNotes.Factory notesFactory;
- private final CallerFinder callerFinder;
private final ChangeNotes notes;
private final PatchSet originalPatchSet;
@@ -201,7 +199,6 @@
this.notes = notes;
this.projectName = notes.getProjectName();
this.originalPatchSet = originalPatchSet;
- this.callerFinder = CallerFinder.builder().addTarget(RebaseChangeOp.class).build();
}
@CanIgnoreReturnValue
@@ -500,8 +497,8 @@
filesWithGitConflicts = null;
tree = merger.getResultTreeId();
logger.atFine().log(
- "tree of rebased commit: %s (no conflicts, inserter: %s, caller: %s)",
- tree.name(), merger.getObjectInserter(), callerFinder.findCallerLazy());
+ "tree of rebased commit: %s (no conflicts, inserter: %s)",
+ tree.name(), merger.getObjectInserter());
} else {
List<String> conflicts = ImmutableList.of();
Map<String, ResolveMerger.MergeFailureReason> failed = ImmutableMap.of();
@@ -560,8 +557,8 @@
ctx.getRevWalk().parseCommit(base),
mergeResults);
logger.atFine().log(
- "tree of rebased commit: %s (with conflicts, inserter: %s, caller: %s)",
- tree.name(), ctx.getInserter(), callerFinder.findCallerLazy());
+ "tree of rebased commit: %s (with conflicts, inserter: %s)",
+ tree.name(), ctx.getInserter());
}
List<ObjectId> parents = new ArrayList<>();
diff --git a/java/com/google/gerrit/server/logging/CallerFinder.java b/java/com/google/gerrit/server/logging/CallerFinder.java
deleted file mode 100644
index a6c2131..0000000
--- a/java/com/google/gerrit/server/logging/CallerFinder.java
+++ /dev/null
@@ -1,275 +0,0 @@
-// Copyright (C) 2018 The Android Open Source Project
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.gerrit.server.logging;
-
-import static com.google.common.flogger.LazyArgs.lazy;
-
-import com.google.auto.value.AutoValue;
-import com.google.common.collect.ImmutableList;
-import com.google.common.flogger.LazyArg;
-import java.util.Optional;
-
-/**
- * Utility to compute the caller of a method.
- *
- * <p>In the logs we see for each entry from where it was triggered (class/method/line) but in case
- * the logging is done in a utility method or inside of a module this doesn't tell us from where the
- * action was actually triggered. To get this information we could included the stacktrace into the
- * logs (by calling {@link
- * com.google.common.flogger.LoggingApi#withStackTrace(com.google.common.flogger.StackSize)} but
- * sometimes there are too many uninteresting stacks so that this would blow up the logs too much.
- * In this case CallerFinder can be used to find the first interesting caller from the current
- * stacktrace by specifying the class that interesting callers invoke as target.
- *
- * <p>Example:
- *
- * <p>Index queries are executed by the {@code query(List<String>, List<Predicate<T>>)} method in
- * {@link com.google.gerrit.index.query.QueryProcessor}. At this place the index query is logged but
- * from the log we want to see which code triggered this index query.
- *
- * <p>E.g. the stacktrace could look like this:
- *
- * <pre>{@code
- * GroupQueryProcessor(QueryProcessor<T>).query(List<String>, List<Predicate<T>>) line: 216
- * GroupQueryProcessor(QueryProcessor<T>).query(List<Predicate<T>>) line: 188
- * GroupQueryProcessor(QueryProcessor<T>).query(Predicate<T>) line: 171
- * InternalGroupQuery(InternalQuery<T>).query(Predicate<T>) line: 81
- * InternalGroupQuery.getOnlyGroup(Predicate<InternalGroup>, String) line: 67
- * InternalGroupQuery.byName(NameKey) line: 50
- * GroupCacheImpl$ByNameLoader.load(String) line: 166
- * GroupCacheImpl$ByNameLoader.load(Object) line: 1
- * LocalCache$LoadingValueReference<K,V>.loadFuture(K, CacheLoader<? super K,V>) line: 3527
- * ...
- * }</pre>
- *
- * <p>The first interesting caller is {@code GroupCacheImpl$ByNameLoader.load(String) line: 166}. To
- * find this caller from the stacktrace we could specify {@link
- * com.google.gerrit.server.query.group.InternalGroupQuery} as a target since we know that all
- * internal group queries go through this class:
- *
- * <pre>
- * CallerFinder.builder()
- * .addTarget(InternalGroupQuery.class)
- * .build();
- * </pre>
- *
- * <p>Since in some places {@link com.google.gerrit.server.query.group.GroupQueryProcessor} may also
- * be used directly we can add it as a secondary target to catch these callers as well:
- *
- * <pre>
- * CallerFinder.builder()
- * .addTarget(InternalGroupQuery.class)
- * .addTarget(GroupQueryProcessor.class)
- * .build();
- * </pre>
- *
- * <p>However since {@link com.google.gerrit.index.query.QueryProcessor} is also responsible to
- * execute other index queries (for changes, accounts, projects) we would need to add the classes
- * for them as targets too. Since there are common base classes we can simply specify the base
- * classes and request matching of subclasses:
- *
- * <pre>
- * CallerFinder.builder()
- * .addTarget(InternalQuery.class)
- * .addTarget(QueryProcessor.class)
- * .matchSubClasses(true)
- * .build();
- * </pre>
- *
- * <p>Another special case is if the entry point is always an inner class of a known interface. E.g.
- * {@link com.google.gerrit.server.permissions.PermissionBackend} is the entry point for all
- * permission checks but they are done through inner classes, e.g. {@link
- * com.google.gerrit.server.permissions.PermissionBackend.ForProject}. In this case matching of
- * inner classes must be enabled as well:
- *
- * <pre>
- * CallerFinder.builder()
- * .addTarget(PermissionBackend.class)
- * .matchSubClasses(true)
- * .matchInnerClasses(true)
- * .build();
- * </pre>
- *
- * <p>Finding the interesting caller requires specifying the entry point class as target. This may
- * easily break when code is refactored and hence should be used only with care. It's recommended to
- * use this only when the corresponding code is relatively stable and logging the caller information
- * brings some significant benefit.
- *
- * <p>Based on {@link com.google.common.flogger.util.CallerFinder}.
- */
-@AutoValue
-public abstract class CallerFinder {
- public static Builder builder() {
- return new AutoValue_CallerFinder.Builder()
- .matchSubClasses(false)
- .matchInnerClasses(false)
- .skip(0);
- }
-
- /**
- * The target classes for which the caller should be found, in the order in which they should be
- * checked.
- *
- * @return the target classes for which the caller should be found
- */
- public abstract ImmutableList<Class<?>> targets();
-
- /**
- * Whether inner classes should be matched.
- *
- * @return whether inner classes should be matched
- */
- public abstract boolean matchSubClasses();
-
- /**
- * Whether sub classes of the target classes should be matched.
- *
- * @return whether sub classes of the target classes should be matched
- */
- public abstract boolean matchInnerClasses();
-
- /**
- * The minimum number of calls known to have occurred between the first call to the target class
- * and the call of {@link #findCallerLazy()}. If in doubt, specify zero here to avoid accidentally
- * skipping past the caller.
- *
- * @return the number of stack elements to skip when computing the caller
- */
- public abstract int skip();
-
- /**
- * Packages that should be ignored and not be considered as caller once a target has been found.
- *
- * @return the ignored packages
- */
- public abstract ImmutableList<String> ignoredPackages();
-
- /**
- * Classes that should be ignored and not be considered as caller once a target has been found.
- *
- * @return the qualified names of the ignored classes
- */
- public abstract ImmutableList<String> ignoredClasses();
-
- @AutoValue.Builder
- public abstract static class Builder {
- abstract ImmutableList.Builder<Class<?>> targetsBuilder();
-
- public Builder addTarget(Class<?> target) {
- targetsBuilder().add(target);
- return this;
- }
-
- public abstract Builder matchSubClasses(boolean matchSubClasses);
-
- public abstract Builder matchInnerClasses(boolean matchInnerClasses);
-
- public abstract Builder skip(int skip);
-
- abstract ImmutableList.Builder<String> ignoredPackagesBuilder();
-
- public Builder addIgnoredPackage(String ignoredPackage) {
- ignoredPackagesBuilder().add(ignoredPackage);
- return this;
- }
-
- abstract ImmutableList.Builder<String> ignoredClassesBuilder();
-
- public Builder addIgnoredClass(Class<?> ignoredClass) {
- ignoredClassesBuilder().add(ignoredClass.getName());
- return this;
- }
-
- public abstract CallerFinder build();
- }
-
- public String findCaller() {
- return targets().stream()
- .map(t -> findCallerOf(t, skip() + 1))
- .filter(Optional::isPresent)
- .findFirst()
- .map(Optional::get)
- .orElse("unknown");
- }
-
- public LazyArg<String> findCallerLazy() {
- return lazy(() -> findCaller());
- }
-
- private Optional<String> findCallerOf(Class<?> target, int skip) {
- // Skip one additional stack frame because we create the Throwable inside this method, not at
- // the point that this method was invoked.
- skip++;
-
- StackTraceElement[] stack = new Throwable().getStackTrace();
-
- // Note: To avoid having to reflect the getStackTraceDepth() method as well, we assume that we
- // will find the caller on the stack and simply catch an exception if we fail (which should
- // hardly ever happen).
- boolean foundCaller = false;
- try {
- for (int index = skip; ; index++) {
- StackTraceElement element = stack[index];
- if (isCaller(target, element.getClassName(), matchSubClasses())) {
- foundCaller = true;
- } else if (foundCaller
- && !ignoredPackages().contains(getPackageName(element))
- && !ignoredClasses().contains(element.getClassName())) {
- return Optional.of(element.toString());
- }
- }
- } catch (Exception e) {
- // This should only happen if a) the caller was not found on the stack
- // (IndexOutOfBoundsException) b) a class that is mentioned in the stack was not found
- // (ClassNotFoundException), however we don't want anything to be thrown from here.
- return Optional.empty();
- }
- }
-
- private static String getPackageName(StackTraceElement element) {
- String className = element.getClassName();
- return className.substring(0, className.lastIndexOf("."));
- }
-
- private boolean isCaller(Class<?> target, String className, boolean matchSubClasses)
- throws ClassNotFoundException {
- if (matchSubClasses) {
- Class<?> clazz = Class.forName(className);
- while (clazz != null) {
- if (Object.class.getName().equals(clazz.getName())) {
- break;
- }
-
- if (isCaller(target, clazz.getName(), false)) {
- return true;
- }
- clazz = clazz.getSuperclass();
- }
- }
-
- if (matchInnerClasses()) {
- int i = className.indexOf('$');
- if (i > 0) {
- className = className.substring(0, i);
- }
- }
-
- if (target.getName().equals(className)) {
- return true;
- }
-
- return false;
- }
-}
diff --git a/java/com/google/gerrit/server/patch/AutoMerger.java b/java/com/google/gerrit/server/patch/AutoMerger.java
index 1dacde7..74f5886 100644
--- a/java/com/google/gerrit/server/patch/AutoMerger.java
+++ b/java/com/google/gerrit/server/patch/AutoMerger.java
@@ -28,7 +28,6 @@
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.InMemoryInserter;
import com.google.gerrit.server.git.MergeUtil;
-import com.google.gerrit.server.logging.CallerFinder;
import com.google.gerrit.server.logging.Metadata;
import com.google.gerrit.server.update.RepoView;
import com.google.inject.Inject;
@@ -99,7 +98,6 @@
private final boolean save;
private final boolean useDiff3;
private final ThreeWayMergeStrategy configuredMergeStrategy;
- private final CallerFinder callerFinder;
@Inject
AutoMerger(
@@ -126,7 +124,6 @@
this.useDiff3 = diff3ConflictView(cfg);
this.gerritIdentProvider = gerritIdentProvider;
this.configuredMergeStrategy = MergeUtil.getMergeStrategy(cfg);
- this.callerFinder = CallerFinder.builder().addTarget(AutoMerger.class).build();
}
/**
@@ -252,8 +249,7 @@
if (couldMerge) {
treeId = m.getResultTreeId();
logger.atFine().log(
- "AutoMerge treeId=%s (no conflicts, inserter: %s, caller: %s)",
- treeId.name(), m.getObjectInserter(), callerFinder.findCallerLazy());
+ "AutoMerge treeId=%s (no conflicts, inserter: %s)", treeId.name(), m.getObjectInserter());
} else {
if (m.getResultTreeId() != null) {
// Merging with conflicts below uses the same DirCache instance that has been used by the
@@ -289,8 +285,7 @@
m.getMergeResults(),
useDiff3);
logger.atFine().log(
- "AutoMerge treeId=%s (with conflicts, inserter: %s, caller: %s)",
- treeId.name(), nonFlushingInserter, callerFinder.findCallerLazy());
+ "AutoMerge treeId=%s (with conflicts, inserter: %s)", treeId.name(), nonFlushingInserter);
}
rw.parseHeaders(merge);
diff --git a/java/com/google/gerrit/server/patch/DiffOperationsImpl.java b/java/com/google/gerrit/server/patch/DiffOperationsImpl.java
index 27c6ca6..44810e8 100644
--- a/java/com/google/gerrit/server/patch/DiffOperationsImpl.java
+++ b/java/com/google/gerrit/server/patch/DiffOperationsImpl.java
@@ -33,7 +33,6 @@
import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.logging.CallerFinder;
import com.google.gerrit.server.patch.diff.ModifiedFilesCache;
import com.google.gerrit.server.patch.diff.ModifiedFilesCacheImpl;
import com.google.gerrit.server.patch.diff.ModifiedFilesCacheKey;
@@ -84,7 +83,6 @@
private final ModifiedFilesLoader.Factory modifiedFilesLoaderFactory;
private final FileDiffCache fileDiffCache;
private final BaseCommitUtil baseCommitUtil;
- private final CallerFinder callerFinder;
public static Module module() {
return new CacheModule() {
@@ -113,11 +111,6 @@
this.modifiedFilesLoaderFactory = modifiedFilesLoaderFactory;
this.fileDiffCache = fileDiffCache;
this.baseCommitUtil = baseCommit;
- this.callerFinder =
- CallerFinder.builder()
- .addTarget(DiffOperations.class)
- .addTarget(DiffOperationsImpl.class)
- .build();
}
@Override
@@ -129,8 +122,8 @@
ObjectReader reader = ins.newReader();
RevWalk revWalk = new RevWalk(reader)) {
logger.atFine().log(
- "Opened repo %s to list modified files against parent for %s (inserter: %s, caller: %s)",
- project, newCommit.name(), ins, callerFinder.findCallerLazy());
+ "Opened repo %s to list modified files against parent for %s (inserter: %s)",
+ project, newCommit.name(), ins);
DiffParameters diffParams =
computeDiffParameters(project, newCommit, parent, new RepoView(repo, revWalk, ins), ins);
return getModifiedFiles(diffParams, diffOptions);
@@ -210,8 +203,8 @@
ObjectReader reader = ins.newReader();
RevWalk revWalk = new RevWalk(reader)) {
logger.atFine().log(
- "Opened repo %s to get modified file against parent for %s (inserter: %s, caller: %s)",
- project, newCommit.name(), ins, callerFinder.findCallerLazy());
+ "Opened repo %s to get modified file against parent for %s (inserter: %s)",
+ project, newCommit.name(), ins);
DiffParameters diffParams =
computeDiffParameters(project, newCommit, parent, new RepoView(repo, revWalk, ins), ins);
FileDiffCacheKey key =
diff --git a/java/com/google/gerrit/server/permissions/RefControl.java b/java/com/google/gerrit/server/permissions/RefControl.java
index aba9522..0aa68e2 100644
--- a/java/com/google/gerrit/server/permissions/RefControl.java
+++ b/java/com/google/gerrit/server/permissions/RefControl.java
@@ -31,7 +31,6 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.logging.CallerFinder;
import com.google.gerrit.server.logging.LoggingContext;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.PermissionBackend.ForChange;
@@ -61,8 +60,6 @@
/** All permissions that apply to this reference. */
private final PermissionCollection relevant;
- private final CallerFinder callerFinder;
-
// The next 4 members are cached canPerform() permissions.
private Boolean owner;
@@ -83,13 +80,6 @@
this.repositoryManager = repositoryManager;
this.refName = ref;
this.relevant = relevant;
- this.callerFinder =
- CallerFinder.builder()
- .addTarget(PermissionBackend.class)
- .matchSubClasses(true)
- .matchInnerClasses(true)
- .skip(1)
- .build();
}
ProjectControl getProjectControl() {
@@ -435,7 +425,6 @@
projectControl.getProject().getName(),
refName);
LoggingContext.getInstance().addAclLogRecord(logMessage);
- logger.atFine().log("%s (caller: %s)", logMessage, callerFinder.findCallerLazy());
}
return false;
}
@@ -455,7 +444,7 @@
pr.getGroup().getUUID().get(),
pr);
LoggingContext.getInstance().addAclLogRecord(logMessage);
- logger.atFine().log("%s (caller: %s)", logMessage, callerFinder.findCallerLazy());
+ logger.atFine().log(logMessage);
}
return true;
}
@@ -471,7 +460,7 @@
projectControl.getProject().getName(),
refName);
LoggingContext.getInstance().addAclLogRecord(logMessage);
- logger.atFine().log("%s (caller: %s)", logMessage, callerFinder.findCallerLazy());
+ logger.atFine().log(logMessage);
}
return false;
}
diff --git a/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java b/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
index aa2c297..aab1cc5 100644
--- a/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
+++ b/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
@@ -21,14 +21,11 @@
import com.google.gerrit.entities.SubmitRecord;
import com.google.gerrit.entities.SubmitTypeRecord;
import com.google.gerrit.exceptions.StorageException;
-import com.google.gerrit.extensions.api.changes.ChangeApi;
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.change.ChangeJson;
import com.google.gerrit.server.index.OnlineReindexMode;
-import com.google.gerrit.server.logging.CallerFinder;
import com.google.gerrit.server.logging.Metadata;
import com.google.gerrit.server.logging.TraceContext;
import com.google.gerrit.server.logging.TraceContext.TraceTimer;
@@ -80,7 +77,6 @@
private final PluginSetContext<SubmitRule> submitRules;
private final Metrics metrics;
private final SubmitRuleOptions opts;
- private final CallerFinder callerFinder;
@Inject
private SubmitRuleEvaluator(
@@ -95,15 +91,6 @@
this.metrics = metrics;
this.opts = options;
-
- this.callerFinder =
- CallerFinder.builder()
- .addTarget(ChangeApi.class)
- .addTarget(ChangeJson.class)
- .addTarget(ChangeData.class)
- .addTarget(SubmitRequirementsEvaluatorImpl.class)
- .addTarget(SubmitRequirementsAdapter.class)
- .build();
}
/**
@@ -117,10 +104,7 @@
try (TraceTimer timer =
TraceContext.newTimer(
"Evaluate submit rules",
- Metadata.builder()
- .changeId(cd.change().getId().get())
- .caller(callerFinder.findCaller())
- .build());
+ Metadata.builder().changeId(cd.change().getId().get()).build());
Timer0.Context ignored = metrics.submitRuleEvaluationLatency.start()) {
if (cd.change() == null) {
throw new StorageException("Change not found");
