summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPatrick Hiesel <hiesel@google.com>2024-04-11 10:55:13 +0200
committerEdwin Kempin <ekempin@google.com>2024-04-11 09:59:10 +0000
commit6869130f5e502c742cb38719bc05df6ce3d80881 (patch)
treee521d545d9e248b90065a91b99920786d609e1b0
parent9761c02675de0e027f75649698701773a4c17558 (diff)
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
-rw-r--r--java/com/google/gerrit/index/query/QueryProcessor.java13
-rw-r--r--java/com/google/gerrit/server/account/AccountCacheImpl.java16
-rw-r--r--java/com/google/gerrit/server/account/externalids/storage/notedb/ExternalIdNotes.java19
-rw-r--r--java/com/google/gerrit/server/change/RebaseChangeOp.java11
-rw-r--r--java/com/google/gerrit/server/logging/CallerFinder.java275
-rw-r--r--java/com/google/gerrit/server/patch/AutoMerger.java9
-rw-r--r--java/com/google/gerrit/server/patch/DiffOperationsImpl.java15
-rw-r--r--java/com/google/gerrit/server/permissions/RefControl.java15
-rw-r--r--java/com/google/gerrit/server/project/SubmitRuleEvaluator.java18
9 files changed, 15 insertions, 376 deletions
diff --git a/java/com/google/gerrit/index/query/QueryProcessor.java b/java/com/google/gerrit/index/query/QueryProcessor.java
index b8eb8bb6ea..cfb9f33831 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.Description;
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 @@ public abstract class QueryProcessor<T> {
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 @@ public abstract class QueryProcessor<T> {
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 @@ public abstract class QueryProcessor<T> {
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 9a535efb33..d269b71e59 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.common.flogger.FluentLogger;
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.AllUsersName;
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 class AccountCacheImpl implements AccountCache {
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 @@ public class AccountCacheImpl implements AccountCache {
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 f0632e4185..6137884f23 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.DuplicateExternalIdKeyExcept
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 @@ public class ExternalIdNotes extends VersionedMetaData {
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 @@ public class ExternalIdNotes extends VersionedMetaData {
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 @@ public class ExternalIdNotes extends VersionedMetaData {
@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 5daac75abd..a430034632 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.CodeReviewCommit.CodeReviewRevWalk;
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 @@ public class RebaseChangeOp implements BatchUpdateOp {
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 @@ public class RebaseChangeOp implements BatchUpdateOp {
this.notes = notes;
this.projectName = notes.getProjectName();
this.originalPatchSet = originalPatchSet;
- this.callerFinder = CallerFinder.builder().addTarget(RebaseChangeOp.class).build();
}
@CanIgnoreReturnValue
@@ -500,8 +497,8 @@ public class RebaseChangeOp implements BatchUpdateOp {
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 @@ public class RebaseChangeOp implements BatchUpdateOp {
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 a6c2131f77..0000000000
--- 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 1dacde7027..74f588675d 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.GerritPersonIdent;
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 @@ public class AutoMerger {
private final boolean save;
private final boolean useDiff3;
private final ThreeWayMergeStrategy configuredMergeStrategy;
- private final CallerFinder callerFinder;
@Inject
AutoMerger(
@@ -126,7 +124,6 @@ public class AutoMerger {
this.useDiff3 = diff3ConflictView(cfg);
this.gerritIdentProvider = gerritIdentProvider;
this.configuredMergeStrategy = MergeUtil.getMergeStrategy(cfg);
- this.callerFinder = CallerFinder.builder().addTarget(AutoMerger.class).build();
}
/**
@@ -252,8 +249,7 @@ public class AutoMerger {
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 @@ public class AutoMerger {
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 27c6ca67de..44810e87d0 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;
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 @@ public class DiffOperationsImpl implements DiffOperations {
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 @@ public class DiffOperationsImpl implements DiffOperations {
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 @@ public class DiffOperationsImpl implements DiffOperations {
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 @@ public class DiffOperationsImpl implements DiffOperations {
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 aba952275d..0aa68e29a9 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.conditions.BooleanCondition;
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 @@ class RefControl {
/** 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 @@ class RefControl {
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 @@ class RefControl {
projectControl.getProject().getName(),
refName);
LoggingContext.getInstance().addAclLogRecord(logMessage);
- logger.atFine().log("%s (caller: %s)", logMessage, callerFinder.findCallerLazy());
}
return false;
}
@@ -455,7 +444,7 @@ class RefControl {
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 @@ class RefControl {
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 aa2c297504..aab1cc515a 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.common.collect.Streams;
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 @@ public class SubmitRuleEvaluator {
private final PluginSetContext<SubmitRule> submitRules;
private final Metrics metrics;
private final SubmitRuleOptions opts;
- private final CallerFinder callerFinder;
@Inject
private SubmitRuleEvaluator(
@@ -95,15 +91,6 @@ public class SubmitRuleEvaluator {
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 @@ public class SubmitRuleEvaluator {
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");