Record RestModifyView name in NoteDb reflog

Instead of using a generic string "Update NoteDb refs", record the
class name of the RestModifyView that caused this operation to
execute.  This makes inspection of the history of a change much
easier, as its possible to correlate events to specific sections of
the Gerrit server.

The implementation favors going from the RestApiServlet down the
stack, so that plugin handlers are caught first and logged as the
plugin's RestModifyView implementation, even if the plugin is using
an internal API call to a standard view like PutTopic.

Change-Id: Ifd6a9843b6bd5f3e7f1ff7e145fa433f237b147a
This commit is contained in:
Shawn Pearce 2017-08-10 05:53:23 -07:00
parent 7bd77e4856
commit 6f42e5d2d5
3 changed files with 111 additions and 1 deletions

View File

@ -0,0 +1,57 @@
// Copyright (C) 2017 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.acceptance.server.notedb;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.reviewdb.client.RefNames.changeMetaRef;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.UseLocalDisk;
import com.google.gerrit.reviewdb.client.Change;
import java.io.File;
import org.eclipse.jgit.lib.ReflogEntry;
import org.eclipse.jgit.lib.Repository;
import org.junit.Before;
import org.junit.Test;
@UseLocalDisk
public class ReflogIT extends AbstractDaemonTest {
@Before
public void setUp() throws Exception {
assume().that(notesMigration.disableChangeReviewDb()).isTrue();
}
@Test
public void guessRestApiInReflog() throws Exception {
assume().that(notesMigration.disableChangeReviewDb()).isTrue();
PushOneCommit.Result r = createChange();
Change.Id id = r.getChange().getId();
try (Repository repo = repoManager.openRepository(r.getChange().project())) {
File log = new File(repo.getDirectory(), "logs/" + changeMetaRef(id));
if (!log.exists()) {
log.getParentFile().mkdirs();
assertThat(log.createNewFile()).isTrue();
}
gApi.changes().id(id.get()).topic("foo");
ReflogEntry last = repo.getReflogReader(changeMetaRef(id)).getLastEntry();
assertThat(last).named("last RefLogEntry").isNotNull();
assertThat(last.getComment()).isEqualTo("change.PutTopic");
}
}
}

View File

@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
// WARNING: NoteDbUpdateManager cares about the package name RestApiServlet lives in.
package com.google.gerrit.httpd.restapi;
import static com.google.common.base.Preconditions.checkNotNull;

View File

@ -24,10 +24,12 @@ import static com.google.gerrit.server.notedb.NoteDbTable.CHANGES;
import com.google.auto.value.AutoValue;
import com.google.common.collect.HashBasedTable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.MultimapBuilder;
import com.google.common.collect.Table;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.metrics.Timer1;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
@ -41,6 +43,7 @@ import com.google.gerrit.server.git.InsertedObject;
import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.update.ChainedReceiveCommands;
import com.google.gerrit.server.update.RefUpdateUtil;
import com.google.gerrit.server.update.RetryingRestModifyView;
import com.google.gwtorm.server.OrmConcurrencyException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@ -77,6 +80,12 @@ import org.eclipse.jgit.transport.ReceiveCommand;
public class NoteDbUpdateManager implements AutoCloseable {
public static final String CHANGES_READ_ONLY = "NoteDb changes are read-only";
private static final ImmutableList<String> PACKAGE_PREFIXES =
ImmutableList.of("com.google.gerrit.server.", "com.google.gerrit.");
private static final ImmutableSet<String> SERVLET_NAMES =
ImmutableSet.of(
"com.google.gerrit.httpd.restapi.RestApiServlet", RetryingRestModifyView.class.getName());
public interface Factory {
NoteDbUpdateManager create(Project.NameKey projectName);
}
@ -539,7 +548,11 @@ public class NoteDbUpdateManager implements AutoCloseable {
BatchRefUpdate bru = or.repo.getRefDatabase().newBatchUpdate();
bru.setPushCertificate(pushCert);
bru.setRefLogMessage(firstNonNull(refLogMessage, "Update NoteDb refs"), false);
if (refLogMessage != null) {
bru.setRefLogMessage(refLogMessage, false);
} else {
bru.setRefLogMessage(firstNonNull(guessRestApiHandler(), "Update NoteDb refs"), false);
}
bru.setRefLogIdent(refLogIdent != null ? refLogIdent : serverIdent.get());
or.cmds.addTo(bru);
bru.setAllowNonFastForwards(true);
@ -550,6 +563,45 @@ public class NoteDbUpdateManager implements AutoCloseable {
return bru;
}
private static String guessRestApiHandler() {
StackTraceElement[] trace = Thread.currentThread().getStackTrace();
int i = findRestApiServlet(trace);
if (i < 0) {
return null;
}
try {
for (i--; i >= 0; i--) {
String cn = trace[i].getClassName();
Class<?> cls = Class.forName(cn);
if (RestModifyView.class.isAssignableFrom(cls) && cls != RetryingRestModifyView.class) {
return viewName(cn);
}
}
return null;
} catch (ClassNotFoundException e) {
return null;
}
}
private static String viewName(String cn) {
String impl = cn.replace('$', '.');
for (String p : PACKAGE_PREFIXES) {
if (impl.startsWith(p)) {
return impl.substring(p.length());
}
}
return impl;
}
private static int findRestApiServlet(StackTraceElement[] trace) {
for (int i = 0; i < trace.length; i++) {
if (SERVLET_NAMES.contains(trace[i].getClassName())) {
return i;
}
}
return -1;
}
private void addCommands() throws OrmException, IOException {
if (isEmpty()) {
return;