From 4234a49aec6f527ae976b9d7ddad4df540151ec5 Mon Sep 17 00:00:00 2001 From: minwoox Date: Mon, 28 Aug 2023 17:51:39 +0900 Subject: [PATCH 01/10] Record metrics when INIT revision or an old revision is used Motivation: Before we apply https://github.com/line/centraldogma/pull/681 that removes old commits, it would be nice if we can find any usages that access data with the INIT Revision or revisions that are more than 5000 (default minimum number of commit retentions) commits ahead from the head. Modifications: - Add a counter that is incresed when INIT Revision or revisions that are more than 5000 ahead from the head are used. Result: - Track the number of usages. - This commit will be reverted after we find the usage and fix it. --- .../server/internal/api/ContentServiceV1.java | 34 +++++++++---- .../internal/api/RepositoryServiceV1.java | 40 +++++++++++++++- .../thrift/CentralDogmaServiceImpl.java | 48 ++++++++++++++----- .../centraldogma/server/MetricsTest.java | 28 +++++++---- 4 files changed, 120 insertions(+), 30 deletions(-) diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/ContentServiceV1.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/ContentServiceV1.java index 757194d00..3e8c04fac 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/ContentServiceV1.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/ContentServiceV1.java @@ -24,6 +24,7 @@ import static com.linecorp.centraldogma.internal.Util.isValidFilePath; import static com.linecorp.centraldogma.server.internal.api.DtoConverter.convert; import static com.linecorp.centraldogma.server.internal.api.HttpApiUtil.returnOrThrow; +import static com.linecorp.centraldogma.server.internal.api.RepositoryServiceV1.increaseCounterIfOldRevisionUsed; import static com.linecorp.centraldogma.server.internal.storage.repository.DefaultMetaRepository.metaRepoFiles; import static java.util.Objects.requireNonNull; @@ -114,11 +115,13 @@ public ContentServiceV1(ProjectManager projectManager, CommandExecutor executor, *

Returns the list of files in the path. */ @Get("regex:/projects/(?[^/]+)/repos/(?[^/]+)/list(?(|/.*))$") - public CompletableFuture>> listFiles(@Param String path, + public CompletableFuture>> listFiles(ServiceRequestContext ctx, + @Param String path, @Param @Default("-1") String revision, Repository repository) { final String normalizedPath = normalizePath(path); final Revision normalizedRev = repository.normalizeNow(new Revision(revision)); + increaseCounterIfOldRevisionUsed(ctx, repository, normalizedRev); final CompletableFuture>> future = new CompletableFuture<>(); listFiles(repository, normalizedPath, normalizedRev, false, future); return future; @@ -214,12 +217,14 @@ private CompletableFuture push(long commitTimeMills, Author author, Re */ @Post("/projects/{projectName}/repos/{repoName}/preview") public CompletableFuture>> preview( + ServiceRequestContext ctx, @Param @Default("-1") String revision, Repository repository, @RequestConverter(ChangesRequestConverter.class) Iterable> changes) { - + final Revision baseRevision = new Revision(revision); + increaseCounterIfOldRevisionUsed(ctx, repository, baseRevision); final CompletableFuture>> changesFuture = - repository.previewDiff(new Revision(revision), changes); + repository.previewDiff(baseRevision, changes); return changesFuture.thenApply(previewDiffs -> previewDiffs.values().stream() .map(DtoConverter::convert) @@ -245,6 +250,7 @@ public CompletableFuture getFiles( Repository repository, @RequestConverter(WatchRequestConverter.class) @Nullable WatchRequest watchRequest, @RequestConverter(QueryRequestConverter.class) @Nullable Query query) { + increaseCounterIfOldRevisionUsed(ctx, repository, new Revision(revision)); final String normalizedPath = normalizePath(path); // watch repository or a file @@ -325,7 +331,8 @@ private static Object handleWatchFailure(Throwable thrown) { * specify {@code to}, this will return the list of commits. */ @Get("regex:/projects/(?[^/]+)/repos/(?[^/]+)/commits(?(|/.*))$") - public CompletableFuture listCommits(@Param String revision, + public CompletableFuture listCommits(ServiceRequestContext ctx, + @Param String revision, @Param @Default("/**") String path, @Param @Nullable String to, @Param @Nullable Integer maxCommits, @@ -346,6 +353,10 @@ public CompletableFuture listCommits(@Param String revision, } final RevisionRange range = repository.normalizeNow(fromRevision, toRevision).toDescending(); + + increaseCounterIfOldRevisionUsed(ctx, repository, range.from()); + increaseCounterIfOldRevisionUsed(ctx, repository, range.to()); + final int maxCommits0 = firstNonNull(maxCommits, Repository.DEFAULT_MAX_COMMITS); return repository .history(range.from(), range.to(), normalizePath(path), maxCommits0) @@ -368,17 +379,21 @@ public CompletableFuture listCommits(@Param String revision, */ @Get("/projects/{projectName}/repos/{repoName}/compare") public CompletableFuture getDiff( + ServiceRequestContext ctx, @Param @Default("/**") String pathPattern, @Param @Default("1") String from, @Param @Default("head") String to, Repository repository, @RequestConverter(QueryRequestConverter.class) @Nullable Query query) { - + final Revision fromRevision = new Revision(from); + final Revision toRevision = new Revision(to); + increaseCounterIfOldRevisionUsed(ctx, repository, fromRevision); + increaseCounterIfOldRevisionUsed(ctx, repository, toRevision); if (query != null) { - return repository.diff(new Revision(from), new Revision(to), query) + return repository.diff(fromRevision, toRevision, query) .thenApply(DtoConverter::convert); } else { return repository - .diff(new Revision(from), new Revision(to), normalizePath(pathPattern)) + .diff(fromRevision, toRevision, normalizePath(pathPattern)) .thenApply(changeMap -> changeMap.values().stream() .map(DtoConverter::convert).collect(toImmutableList())); } @@ -402,9 +417,12 @@ private static Object objectOrList(Collection collection, boolean toList, */ @Get("/projects/{projectName}/repos/{repoName}/merge") public CompletableFuture> mergeFiles( + ServiceRequestContext ctx, @Param @Default("-1") String revision, Repository repository, @RequestConverter(MergeQueryRequestConverter.class) MergeQuery query) { - return repository.mergeFiles(new Revision(revision), query).thenApply(DtoConverter::convert); + final Revision rev = new Revision(revision); + increaseCounterIfOldRevisionUsed(ctx, repository, rev); + return repository.mergeFiles(rev, query).thenApply(DtoConverter::convert); } /** diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1.java index 655d7ea99..99533bd2c 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1.java @@ -16,6 +16,7 @@ package com.linecorp.centraldogma.server.internal.api; +import static com.google.common.base.MoreObjects.firstNonNull; import static com.google.common.collect.ImmutableList.toImmutableList; import static com.linecorp.centraldogma.server.internal.api.HttpApiUtil.checkUnremoveArgument; import static com.linecorp.centraldogma.server.internal.api.HttpApiUtil.returnOrThrow; @@ -28,9 +29,11 @@ import javax.annotation.Nullable; import com.fasterxml.jackson.databind.JsonNode; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.common.logging.RequestLog; import com.linecorp.armeria.server.ServiceRequestContext; import com.linecorp.armeria.server.annotation.Consumes; import com.linecorp.armeria.server.annotation.Delete; @@ -58,6 +61,8 @@ import com.linecorp.centraldogma.server.storage.project.ProjectManager; import com.linecorp.centraldogma.server.storage.repository.Repository; +import io.micrometer.core.instrument.Tag; + /** * Annotated service object for managing repositories. */ @@ -186,8 +191,41 @@ public CompletableFuture patchRepository(@Param String repoName, */ @Get("/projects/{projectName}/repos/{repoName}/revision/{revision}") @RequiresReadPermission - public Map normalizeRevision(Repository repository, @Param String revision) { + public Map normalizeRevision(ServiceRequestContext ctx, + Repository repository, @Param String revision) { final Revision normalizedRevision = repository.normalizeNow(new Revision(revision)); + final Revision head = repository.normalizeNow(Revision.HEAD); + increaseCounterIfOldRevisionUsed(ctx, repository.parent().name(), repository.name(), + normalizedRevision, head); return ImmutableMap.of("revision", normalizedRevision.major()); } + + static void increaseCounterIfOldRevisionUsed(ServiceRequestContext ctx, Repository repository, + Revision revision) { + final Revision normalized = repository.normalizeNow(revision); + final Revision head = repository.normalizeNow(Revision.HEAD); + increaseCounterIfOldRevisionUsed(ctx, repository.parent().name(), repository.name(), normalized, head); + } + + public static void increaseCounterIfOldRevisionUsed( + ServiceRequestContext ctx, String projectName, String repoName, + Revision normalized, Revision head) { + if (normalized.major() == 1 || head.major() - normalized.major() >= 5000) { + ctx.log().whenComplete().thenAccept(log -> { + ctx.meterRegistry().counter("dogma.old.revision", + generateTags(projectName, repoName, log, normalized, head)) + .increment(); + }); + } + } + + private static List generateTags(String projectName, String repoName, + RequestLog log, Revision normalized, Revision head) { + return ImmutableList.of(Tag.of("project", projectName), + Tag.of("repo", repoName), + Tag.of("service", firstNonNull(log.serviceName(), "none")), + Tag.of("method", log.name()), + Tag.of("normalized", Integer.toString(normalized.major())), + Tag.of("head", Integer.toString(head.major()))); + } } diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/thrift/CentralDogmaServiceImpl.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/thrift/CentralDogmaServiceImpl.java index 8265629c0..8faf4b3d6 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/thrift/CentralDogmaServiceImpl.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/thrift/CentralDogmaServiceImpl.java @@ -17,7 +17,9 @@ import static com.google.common.base.MoreObjects.firstNonNull; import static com.linecorp.centraldogma.common.Author.SYSTEM; +import static com.linecorp.centraldogma.common.Revision.HEAD; import static com.linecorp.centraldogma.server.internal.api.ContentServiceV1.checkPush; +import static com.linecorp.centraldogma.server.internal.api.RepositoryServiceV1.increaseCounterIfOldRevisionUsed; import static com.linecorp.centraldogma.server.internal.thrift.Converter.convert; import static com.linecorp.centraldogma.server.storage.project.Project.isReservedRepoName; import static com.linecorp.centraldogma.server.storage.repository.FindOptions.FIND_ALL_WITHOUT_CONTENT; @@ -36,6 +38,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.linecorp.armeria.common.RequestContext; import com.linecorp.armeria.common.util.Exceptions; import com.linecorp.centraldogma.internal.thrift.Author; import com.linecorp.centraldogma.internal.thrift.CentralDogmaConstants; @@ -218,15 +221,27 @@ public void normalizeRevision(String projectName, String repositoryName, Revisio AsyncMethodCallback resultHandler) { final com.linecorp.centraldogma.common.Revision normalized = - projectManager.get(projectName).repos().get(repositoryName) - .normalizeNow(convert(revision)); + normalizeRevision(projectName, repositoryName, revision); + resultHandler.onComplete(convert(normalized)); } + private com.linecorp.centraldogma.common.Revision normalizeRevision( + String projectName, String repositoryName, Revision revision) { + final Repository repository = projectManager.get(projectName).repos().get(repositoryName); + final com.linecorp.centraldogma.common.Revision normalized = + repository.normalizeNow(convert(revision)); + final com.linecorp.centraldogma.common.Revision head = repository.normalizeNow(HEAD); + increaseCounterIfOldRevisionUsed(RequestContext.current(), projectName, repositoryName, + normalized, head); + return normalized; + } + @Override public void listFiles(String projectName, String repositoryName, Revision revision, String pathPattern, AsyncMethodCallback resultHandler) { - + // Call normalizeRevision() first to check if the specified revision needs to be recorded. + normalizeRevision(projectName, repositoryName, revision); handle(projectManager.get(projectName).repos().get(repositoryName) .find(convert(revision), pathPattern, FIND_ALL_WITHOUT_CONTENT) .thenApply(entries -> { @@ -241,7 +256,8 @@ public void listFiles(String projectName, String repositoryName, Revision revisi @Override public void getFiles(String projectName, String repositoryName, Revision revision, String pathPattern, AsyncMethodCallback resultHandler) { - + // Call normalizeRevision() first to check if the specified revision needs to be recorded. + normalizeRevision(projectName, repositoryName, revision); handle(projectManager.get(projectName).repos().get(repositoryName) .find(convert(revision), pathPattern) .thenApply(entries -> { @@ -257,7 +273,9 @@ public void getFiles(String projectName, String repositoryName, Revision revisio @Override public void getHistory(String projectName, String repositoryName, Revision from, Revision to, String pathPattern, AsyncMethodCallback resultHandler) { - + // Call normalizeRevision() first to check if the specified revision needs to be recorded. + normalizeRevision(projectName, repositoryName, from); + normalizeRevision(projectName, repositoryName, to); handle(projectManager.get(projectName).repos().get(repositoryName) .history(convert(from), convert(to), pathPattern) .thenApply(commits -> commits.stream() @@ -269,7 +287,9 @@ public void getHistory(String projectName, String repositoryName, Revision from, @Override public void getDiffs(String projectName, String repositoryName, Revision from, Revision to, String pathPattern, AsyncMethodCallback resultHandler) { - + // Call normalizeRevision() first to check if the specified revision needs to be recorded. + normalizeRevision(projectName, repositoryName, from); + normalizeRevision(projectName, repositoryName, to); handle(projectManager.get(projectName).repos().get(repositoryName) .diff(convert(from), convert(to), pathPattern) .thenApply(diffs -> convert(diffs.values(), Converter::convert)), @@ -279,7 +299,8 @@ public void getDiffs(String projectName, String repositoryName, Revision from, R @Override public void getPreviewDiffs(String projectName, String repositoryName, Revision baseRevision, List changes, AsyncMethodCallback resultHandler) { - + // Call normalizeRevision() first to check if the specified revision needs to be recorded. + normalizeRevision(projectName, repositoryName, baseRevision); handle(projectManager.get(projectName).repos().get(repositoryName) .previewDiff(convert(baseRevision), convert(changes, Converter::convert)) .thenApply(diffs -> convert(diffs.values(), Converter::convert)), @@ -313,7 +334,8 @@ public void push(String projectName, String repositoryName, Revision baseRevisio @Override public void getFile(String projectName, String repositoryName, Revision revision, Query query, AsyncMethodCallback resultHandler) { - + // Call normalizeRevision() first to check if the specified revision needs to be recorded. + normalizeRevision(projectName, repositoryName, revision); handle(projectManager.get(projectName).repos().get(repositoryName) .get(convert(revision), convert(query)) .thenApply(res -> new GetFileResult(convert(res.type()), res.contentAsText())), @@ -323,7 +345,9 @@ public void getFile(String projectName, String repositoryName, Revision revision @Override public void diffFile(String projectName, String repositoryName, Revision from, Revision to, Query query, AsyncMethodCallback resultHandler) { - + // Call normalizeRevision() first to check if the specified revision needs to be recorded. + normalizeRevision(projectName, repositoryName, from); + normalizeRevision(projectName, repositoryName, to); // FIXME(trustin): Remove the firstNonNull() on the change content once we make it optional. handle(projectManager.get(projectName).repos().get(repositoryName) .diff(convert(from), convert(to), convert(query)) @@ -348,7 +372,8 @@ public void mergeFiles(String projectName, String repositoryName, Revision revis public void watchRepository( String projectName, String repositoryName, Revision lastKnownRevision, String pathPattern, long timeoutMillis, AsyncMethodCallback resultHandler) { - + // Call normalizeRevision() first to check if the specified revision needs to be recorded. + normalizeRevision(projectName, repositoryName, lastKnownRevision); if (timeoutMillis <= 0) { rejectInvalidWatchTimeout("watchRepository", resultHandler); return; @@ -384,7 +409,8 @@ private static void handleWatchRepositoryResult( public void watchFile( String projectName, String repositoryName, Revision lastKnownRevision, Query query, long timeoutMillis, AsyncMethodCallback resultHandler) { - + // Call normalizeRevision() first to check if the specified revision needs to be recorded. + normalizeRevision(projectName, repositoryName, lastKnownRevision); if (timeoutMillis <= 0) { rejectInvalidWatchTimeout("watchFile", resultHandler); return; diff --git a/server/src/test/java/com/linecorp/centraldogma/server/MetricsTest.java b/server/src/test/java/com/linecorp/centraldogma/server/MetricsTest.java index 1d98fefd0..aad00c639 100644 --- a/server/src/test/java/com/linecorp/centraldogma/server/MetricsTest.java +++ b/server/src/test/java/com/linecorp/centraldogma/server/MetricsTest.java @@ -20,12 +20,16 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import com.fasterxml.jackson.databind.JsonNode; + import com.linecorp.armeria.common.AggregatedHttpResponse; import com.linecorp.armeria.common.HttpStatus; import com.linecorp.armeria.common.MediaType; import com.linecorp.centraldogma.client.CentralDogma; +import com.linecorp.centraldogma.client.WatchRequest; import com.linecorp.centraldogma.common.Change; import com.linecorp.centraldogma.common.Query; +import com.linecorp.centraldogma.common.Revision; import com.linecorp.centraldogma.testing.junit.CentralDogmaExtension; import io.micrometer.core.instrument.MeterRegistry; @@ -55,7 +59,7 @@ void metrics() { assertThat(((CompositeMeterRegistry) meterRegistry).getRegistries()) .hasAtLeastOneElementOfType(PrometheusMeterRegistry.class); - AggregatedHttpResponse res = dogma.httpClient().get("/monitor/metrics").aggregate().join(); + final AggregatedHttpResponse res = dogma.httpClient().get("/monitor/metrics").aggregate().join(); String content = res.contentUtf8(); assertThat(res.status()).isEqualTo(HttpStatus.OK); assertThat(res.contentType()).isEqualTo(MediaType.parse(TextFormat.CONTENT_TYPE_004)); @@ -63,15 +67,19 @@ void metrics() { assertThat(content).doesNotContain( "com.linecorp.centraldogma.server.internal.api.WatchContentServiceV1"); - dogma.client() - .forRepo("foo", "bar") - .watch(Query.ofJson("/foo.json")) - .timeoutMillis(100) - .errorOnEntryNotFound(false) - .start() - .join(); - res = dogma.httpClient().get("/monitor/metrics").aggregate().join(); - content = res.contentUtf8(); + final WatchRequest jsonNodeWatchRequest = dogma.client() + .forRepo("foo", "bar") + .watch(Query.ofJson("/foo.json")) + .timeoutMillis(100) + .errorOnEntryNotFound(false); + jsonNodeWatchRequest.start().join(); + content = dogma.httpClient().get("/monitor/metrics").aggregate().join().contentUtf8(); assertThat(content).contains("com.linecorp.centraldogma.server.internal.api.WatchContentServiceV1"); + assertThat(content).doesNotContain("dogma_old_revision"); + + // Trigger old revision recording + jsonNodeWatchRequest.start(Revision.INIT).join(); + content = dogma.httpClient().get("/monitor/metrics").aggregate().join().contentUtf8(); + assertThat(content).contains("dogma_old_revision"); } } From a302f382ffec0fccabe92a440808f6eb31973b97 Mon Sep 17 00:00:00 2001 From: minwoox Date: Mon, 28 Aug 2023 19:51:19 +0900 Subject: [PATCH 02/10] Address comments from @jrhee17 --- .../internal/api/RepositoryServiceV1.java | 20 +++++++++++-------- .../thrift/CentralDogmaServiceImpl.java | 2 ++ .../centraldogma/server/MetricsTest.java | 4 ++-- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1.java index 99533bd2c..65dc483f8 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1.java @@ -210,22 +210,26 @@ static void increaseCounterIfOldRevisionUsed(ServiceRequestContext ctx, Reposito public static void increaseCounterIfOldRevisionUsed( ServiceRequestContext ctx, String projectName, String repoName, Revision normalized, Revision head) { - if (normalized.major() == 1 || head.major() - normalized.major() >= 5000) { + if (normalized.major() == 1) { ctx.log().whenComplete().thenAccept(log -> { - ctx.meterRegistry().counter("dogma.old.revision", - generateTags(projectName, repoName, log, normalized, head)) + ctx.meterRegistry() + .counter("init.revisions", generateTags(projectName, repoName, log)) .increment(); }); } + if (head.major() - normalized.major() >= 5000) { + ctx.log().whenComplete().thenAccept(log -> { + ctx.meterRegistry() + .summary("old.revisions", generateTags(projectName, repoName, log)) + .record(head.major() - normalized.major()); + }); + } } - private static List generateTags(String projectName, String repoName, - RequestLog log, Revision normalized, Revision head) { + private static List generateTags(String projectName, String repoName, RequestLog log) { return ImmutableList.of(Tag.of("project", projectName), Tag.of("repo", repoName), Tag.of("service", firstNonNull(log.serviceName(), "none")), - Tag.of("method", log.name()), - Tag.of("normalized", Integer.toString(normalized.major())), - Tag.of("head", Integer.toString(head.major()))); + Tag.of("method", log.name())); } } diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/thrift/CentralDogmaServiceImpl.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/thrift/CentralDogmaServiceImpl.java index 8faf4b3d6..ff036c9b0 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/thrift/CentralDogmaServiceImpl.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/thrift/CentralDogmaServiceImpl.java @@ -359,6 +359,8 @@ public void diffFile(String projectName, String repositoryName, Revision from, R @Override public void mergeFiles(String projectName, String repositoryName, Revision revision, MergeQuery mergeQuery, AsyncMethodCallback resultHandler) { + // Call normalizeRevision() first to check if the specified revision needs to be recorded. + normalizeRevision(projectName, repositoryName, revision); handle(projectManager.get(projectName).repos().get(repositoryName) .mergeFiles(convert(revision), convert(mergeQuery)) .thenApply(merged -> new MergedEntry(convert(merged.revision()), diff --git a/server/src/test/java/com/linecorp/centraldogma/server/MetricsTest.java b/server/src/test/java/com/linecorp/centraldogma/server/MetricsTest.java index aad00c639..dfe24d4d4 100644 --- a/server/src/test/java/com/linecorp/centraldogma/server/MetricsTest.java +++ b/server/src/test/java/com/linecorp/centraldogma/server/MetricsTest.java @@ -75,11 +75,11 @@ void metrics() { jsonNodeWatchRequest.start().join(); content = dogma.httpClient().get("/monitor/metrics").aggregate().join().contentUtf8(); assertThat(content).contains("com.linecorp.centraldogma.server.internal.api.WatchContentServiceV1"); - assertThat(content).doesNotContain("dogma_old_revision"); + assertThat(content).doesNotContain("init_revisions"); // Trigger old revision recording jsonNodeWatchRequest.start(Revision.INIT).join(); content = dogma.httpClient().get("/monitor/metrics").aggregate().join().contentUtf8(); - assertThat(content).contains("dogma_old_revision"); + assertThat(content).contains("init_revisions"); } } From 37e6a854d40bd640c160a742331e09eb3bd74f22 Mon Sep 17 00:00:00 2001 From: minwoox Date: Mon, 28 Aug 2023 20:00:35 +0900 Subject: [PATCH 03/10] Update tag --- .../internal/api/RepositoryServiceV1.java | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1.java index 65dc483f8..a47262eb0 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1.java @@ -211,25 +211,28 @@ public static void increaseCounterIfOldRevisionUsed( ServiceRequestContext ctx, String projectName, String repoName, Revision normalized, Revision head) { if (normalized.major() == 1) { - ctx.log().whenComplete().thenAccept(log -> { - ctx.meterRegistry() - .counter("init.revisions", generateTags(projectName, repoName, log)) - .increment(); - }); + ctx.log().whenComplete().thenAccept( + log -> ctx.meterRegistry() + .counter("init.revisions", generateTags(projectName, repoName, log).build()) + .increment()); } if (head.major() - normalized.major() >= 5000) { - ctx.log().whenComplete().thenAccept(log -> { - ctx.meterRegistry() - .summary("old.revisions", generateTags(projectName, repoName, log)) - .record(head.major() - normalized.major()); - }); + ctx.log().whenComplete().thenAccept( + log -> ctx.meterRegistry() + .summary("old.revisions", + generateTags(projectName, repoName, log) + .add(Tag.of("init", Boolean.toString(normalized.major() == 1))) + .build()) + .record(head.major() - normalized.major())); } } - private static List generateTags(String projectName, String repoName, RequestLog log) { - return ImmutableList.of(Tag.of("project", projectName), - Tag.of("repo", repoName), - Tag.of("service", firstNonNull(log.serviceName(), "none")), - Tag.of("method", log.name())); + private static ImmutableList.Builder generateTags( + String projectName, String repoName, RequestLog log) { + final ImmutableList.Builder builder = ImmutableList.builder(); + return builder.add(Tag.of("project", projectName), + Tag.of("repo", repoName), + Tag.of("service", firstNonNull(log.serviceName(), "none")), + Tag.of("method", log.name())); } } From 82fa47ccb44f7c0448219d3afd550345bc20427e Mon Sep 17 00:00:00 2001 From: minwoox Date: Tue, 29 Aug 2023 11:35:05 +0900 Subject: [PATCH 04/10] Add projectName to mirroring metrics --- .../internal/mirror/MirroringTaskTest.java | 6 +- .../mirror/DefaultMirroringService.java | 62 ++++++++++--------- .../server/internal/mirror/MirroringTask.java | 7 ++- 3 files changed, 40 insertions(+), 35 deletions(-) diff --git a/server-mirror-git/src/test/java/com/linecorp/centraldogma/server/internal/mirror/MirroringTaskTest.java b/server-mirror-git/src/test/java/com/linecorp/centraldogma/server/internal/mirror/MirroringTaskTest.java index 4b9381365..a8bc71149 100644 --- a/server-mirror-git/src/test/java/com/linecorp/centraldogma/server/internal/mirror/MirroringTaskTest.java +++ b/server-mirror-git/src/test/java/com/linecorp/centraldogma/server/internal/mirror/MirroringTaskTest.java @@ -45,7 +45,7 @@ void testSuccessMetrics() { Mirror mirror = newMirror("git://a.com/b.git", GitMirror.class, "foo", "bar"); mirror = spy(mirror); doNothing().when(mirror).mirror(any(), any(), anyInt(), anyLong()); - new MirroringTask(mirror, meterRegistry).run(null, null, 0, 0L); + new MirroringTask(mirror, projectName, meterRegistry).run(null, null, 0, 0L); assertThat(MoreMeters.measureAll(meterRegistry)) .contains(entry("mirroring.result#count{direction=LOCAL_TO_REMOTE,localPath=/," + "localRepo=bar,remoteBranch=,remotePath=/,success=true}", 1.0)); @@ -58,7 +58,7 @@ void testFailureMetrics() { mirror = spy(mirror); final RuntimeException e = new RuntimeException(); doThrow(e).when(mirror).mirror(any(), any(), anyInt(), anyLong()); - final MirroringTask task = new MirroringTask(mirror, meterRegistry); + final MirroringTask task = new MirroringTask(mirror, projectName, meterRegistry); assertThatThrownBy(() -> task.run(null, null, 0, 0L)) .isSameAs(e); assertThat(MoreMeters.measureAll(meterRegistry)) @@ -76,7 +76,7 @@ void testTimerMetrics() { Thread.sleep(1000); return null; }).when(mirror).mirror(any(), any(), anyInt(), anyLong()); - new MirroringTask(mirror, meterRegistry).run(null, null, 0, 0L); + new MirroringTask(mirror, projectName, meterRegistry).run(null, null, 0, 0L); assertThat(MoreMeters.measureAll(meterRegistry)) .hasEntrySatisfying( "mirroring.task#total{direction=LOCAL_TO_REMOTE,localPath=/," + diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/mirror/DefaultMirroringService.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/mirror/DefaultMirroringService.java index 89e37756c..a0199e1ec 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/mirror/DefaultMirroringService.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/mirror/DefaultMirroringService.java @@ -22,6 +22,7 @@ import java.io.File; import java.time.Duration; import java.time.ZonedDateTime; +import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -29,7 +30,6 @@ import java.util.concurrent.SynchronousQueue; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; -import java.util.stream.Stream; import javax.annotation.Nullable; @@ -188,35 +188,25 @@ private void schedulePendingMirrors() { final ZonedDateTime currentLastExecutionTime = lastExecutionTime; lastExecutionTime = now; - projectManager.list().values().stream() - .map(Project::metaRepo) - .flatMap(r -> { + projectManager.list() + .values() + .forEach(project -> { + final Set mirrors; try { - return r.mirrors().stream(); + mirrors = project.metaRepo().mirrors(); } catch (Exception e) { - logger.warn("Failed to load the mirror list from: {}", r.parent().name(), e); - return Stream.empty(); + logger.warn("Failed to load the mirror list from: {}", project.name(), e); + return; } - }) - .filter(m -> { - try { - return m.nextExecutionTime(currentLastExecutionTime).compareTo(now) < 0; - } catch (Exception e) { - logger.warn("Failed to calculate the next execution time of: {}", m, e); - return false; - } - }) - .forEach(m -> { - final ListenableFuture future = worker.submit(() -> run(m, true)); - Futures.addCallback(future, new FutureCallback() { - @Override - public void onSuccess(@Nullable Object result) {} - - @Override - public void onFailure(Throwable cause) { - logger.warn("Unexpected Git mirroring failure: {}", m, cause); + mirrors.forEach(m -> { + try { + if (m.nextExecutionTime(currentLastExecutionTime).compareTo(now) < 0) { + run(project, m); + } + } catch (Exception e) { + logger.warn("Unexpected exception while mirroring: {}", m, e); } - }, MoreExecutors.directExecutor()); + }); }); } @@ -229,14 +219,28 @@ public CompletableFuture mirror() { return CompletableFuture.runAsync( () -> projectManager.list().values() .forEach(p -> p.metaRepo().mirrors() - .forEach(m -> run(m, false))), + .forEach(m -> run(m, p.name(), false))), worker); } - private void run(Mirror m, boolean logOnFailure) { + private void run(Project project, Mirror m) { + final ListenableFuture future = + worker.submit(() -> run(m, project.name(), true)); + Futures.addCallback(future, new FutureCallback() { + @Override + public void onSuccess(@Nullable Object result) {} + + @Override + public void onFailure(Throwable cause) { + logger.warn("Unexpected Git mirroring failure: {}", m, cause); + } + }, MoreExecutors.directExecutor()); + } + + private void run(Mirror m, String projectName, boolean logOnFailure) { logger.info("Mirroring: {}", m); try { - new MirroringTask(m, meterRegistry) + new MirroringTask(m, projectName, meterRegistry) .run(workDir, commandExecutor, maxNumFilesPerMirror, maxNumBytesPerMirror); } catch (Exception e) { if (logOnFailure) { diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/mirror/MirroringTask.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/mirror/MirroringTask.java index 14fd9ad57..1342599b0 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/mirror/MirroringTask.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/mirror/MirroringTask.java @@ -31,8 +31,9 @@ final class MirroringTask { - private static Iterable generateTags(Mirror mirror) { + private static Iterable generateTags(Mirror mirror, String projectName) { return ImmutableList.of( + Tag.of("project", projectName), Tag.of("direction", mirror.direction().name()), Tag.of("remoteBranch", firstNonNull(mirror.remoteBranch(), "")), Tag.of("remotePath", mirror.remotePath()), @@ -44,10 +45,10 @@ private static Iterable generateTags(Mirror mirror) { private final Mirror mirror; private final Iterable tags; - MirroringTask(Mirror mirror, MeterRegistry meterRegistry) { + MirroringTask(Mirror mirror, String projectName, MeterRegistry meterRegistry) { this.mirror = mirror; this.meterRegistry = meterRegistry; - tags = generateTags(mirror); + tags = generateTags(mirror, projectName); } private Counter counter(boolean success) { From 2b5fd93a6849591bd9a19404e4c3772b631fd678 Mon Sep 17 00:00:00 2001 From: minwoox Date: Tue, 29 Aug 2023 11:36:24 +0900 Subject: [PATCH 05/10] lint --- .../server/internal/mirror/DefaultMirroringService.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/mirror/DefaultMirroringService.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/mirror/DefaultMirroringService.java index a0199e1ec..fefb7f579 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/mirror/DefaultMirroringService.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/mirror/DefaultMirroringService.java @@ -224,8 +224,7 @@ public CompletableFuture mirror() { } private void run(Project project, Mirror m) { - final ListenableFuture future = - worker.submit(() -> run(m, project.name(), true)); + final ListenableFuture future = worker.submit(() -> run(m, project.name(), true)); Futures.addCallback(future, new FutureCallback() { @Override public void onSuccess(@Nullable Object result) {} From a89d7817b15402270b71a4f5a2150f7b88cae130 Mon Sep 17 00:00:00 2001 From: minwoox Date: Tue, 29 Aug 2023 11:40:52 +0900 Subject: [PATCH 06/10] Fix tests --- .../server/internal/mirror/MirroringTaskTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server-mirror-git/src/test/java/com/linecorp/centraldogma/server/internal/mirror/MirroringTaskTest.java b/server-mirror-git/src/test/java/com/linecorp/centraldogma/server/internal/mirror/MirroringTaskTest.java index a8bc71149..ea709d018 100644 --- a/server-mirror-git/src/test/java/com/linecorp/centraldogma/server/internal/mirror/MirroringTaskTest.java +++ b/server-mirror-git/src/test/java/com/linecorp/centraldogma/server/internal/mirror/MirroringTaskTest.java @@ -45,7 +45,7 @@ void testSuccessMetrics() { Mirror mirror = newMirror("git://a.com/b.git", GitMirror.class, "foo", "bar"); mirror = spy(mirror); doNothing().when(mirror).mirror(any(), any(), anyInt(), anyLong()); - new MirroringTask(mirror, projectName, meterRegistry).run(null, null, 0, 0L); + new MirroringTask(mirror, "foo", meterRegistry).run(null, null, 0, 0L); assertThat(MoreMeters.measureAll(meterRegistry)) .contains(entry("mirroring.result#count{direction=LOCAL_TO_REMOTE,localPath=/," + "localRepo=bar,remoteBranch=,remotePath=/,success=true}", 1.0)); @@ -58,7 +58,7 @@ void testFailureMetrics() { mirror = spy(mirror); final RuntimeException e = new RuntimeException(); doThrow(e).when(mirror).mirror(any(), any(), anyInt(), anyLong()); - final MirroringTask task = new MirroringTask(mirror, projectName, meterRegistry); + final MirroringTask task = new MirroringTask(mirror, "foo", meterRegistry); assertThatThrownBy(() -> task.run(null, null, 0, 0L)) .isSameAs(e); assertThat(MoreMeters.measureAll(meterRegistry)) @@ -76,7 +76,7 @@ void testTimerMetrics() { Thread.sleep(1000); return null; }).when(mirror).mirror(any(), any(), anyInt(), anyLong()); - new MirroringTask(mirror, projectName, meterRegistry).run(null, null, 0, 0L); + new MirroringTask(mirror, "foo", meterRegistry).run(null, null, 0, 0L); assertThat(MoreMeters.measureAll(meterRegistry)) .hasEntrySatisfying( "mirroring.task#total{direction=LOCAL_TO_REMOTE,localPath=/," + From 24ef8be16486de105853ffcba23684de2f572f64 Mon Sep 17 00:00:00 2001 From: minwoox Date: Tue, 29 Aug 2023 11:44:45 +0900 Subject: [PATCH 07/10] Fix again --- .../server/internal/mirror/MirroringTaskTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server-mirror-git/src/test/java/com/linecorp/centraldogma/server/internal/mirror/MirroringTaskTest.java b/server-mirror-git/src/test/java/com/linecorp/centraldogma/server/internal/mirror/MirroringTaskTest.java index ea709d018..c3ffdc930 100644 --- a/server-mirror-git/src/test/java/com/linecorp/centraldogma/server/internal/mirror/MirroringTaskTest.java +++ b/server-mirror-git/src/test/java/com/linecorp/centraldogma/server/internal/mirror/MirroringTaskTest.java @@ -48,7 +48,7 @@ void testSuccessMetrics() { new MirroringTask(mirror, "foo", meterRegistry).run(null, null, 0, 0L); assertThat(MoreMeters.measureAll(meterRegistry)) .contains(entry("mirroring.result#count{direction=LOCAL_TO_REMOTE,localPath=/," + - "localRepo=bar,remoteBranch=,remotePath=/,success=true}", 1.0)); + "localRepo=bar,project=foo,remoteBranch=,remotePath=/,success=true}", 1.0)); } @Test @@ -63,7 +63,7 @@ void testFailureMetrics() { .isSameAs(e); assertThat(MoreMeters.measureAll(meterRegistry)) .contains(entry("mirroring.result#count{direction=LOCAL_TO_REMOTE,localPath=/," + - "localRepo=bar,remoteBranch=main,remotePath=/," + + "localRepo=bar,project=foo,remoteBranch=main,remotePath=/," + "success=false}", 1.0)); } @@ -80,7 +80,7 @@ void testTimerMetrics() { assertThat(MoreMeters.measureAll(meterRegistry)) .hasEntrySatisfying( "mirroring.task#total{direction=LOCAL_TO_REMOTE,localPath=/," + - "localRepo=bar,remoteBranch=,remotePath=/}", + "localRepo=bar,project=foo,remoteBranch=,remotePath=/}", v -> assertThat(v).isCloseTo(1, withPercentage(30))); } } From f29522468b6c931f9c386690f1c550aec26f71d6 Mon Sep 17 00:00:00 2001 From: minwoox Date: Tue, 29 Aug 2023 11:47:59 +0900 Subject: [PATCH 08/10] Add dontnote --- common/build.gradle | 2 ++ 1 file changed, 2 insertions(+) diff --git a/common/build.gradle b/common/build.gradle index ad3c70b00..d16b2c9fc 100644 --- a/common/build.gradle +++ b/common/build.gradle @@ -22,5 +22,7 @@ if (tasks.findByName('trimShadedJar')) { keep "class com.linecorp.centraldogma.internal.shaded.caffeine.** { *; }" // Prevent ProGuard from removing all enum values from Option because otherwise it becomes a non-enum class. keep "class com.linecorp.centraldogma.internal.shaded.jsonpath.Option { *; }" + + dontnote } } From d1d36a220e036fb8a0532a8f043db5b81aae01fe Mon Sep 17 00:00:00 2001 From: minwoox Date: Tue, 29 Aug 2023 12:25:16 +0900 Subject: [PATCH 09/10] Address comments from @trustin --- common/build.gradle | 1 + .../internal/api/RepositoryServiceV1.java | 4 +-- .../centraldogma/server/MetricsTest.java | 26 +++++++++++++++++-- 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/common/build.gradle b/common/build.gradle index d16b2c9fc..41505e6d5 100644 --- a/common/build.gradle +++ b/common/build.gradle @@ -23,6 +23,7 @@ if (tasks.findByName('trimShadedJar')) { // Prevent ProGuard from removing all enum values from Option because otherwise it becomes a non-enum class. keep "class com.linecorp.centraldogma.internal.shaded.jsonpath.Option { *; }" + // Reduces the verbosity of ProGuardTask when running in parallel. dontnote } } diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1.java index a47262eb0..10f7c9c8f 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1.java @@ -213,13 +213,13 @@ public static void increaseCounterIfOldRevisionUsed( if (normalized.major() == 1) { ctx.log().whenComplete().thenAccept( log -> ctx.meterRegistry() - .counter("init.revisions", generateTags(projectName, repoName, log).build()) + .counter("revisions.init", generateTags(projectName, repoName, log).build()) .increment()); } if (head.major() - normalized.major() >= 5000) { ctx.log().whenComplete().thenAccept( log -> ctx.meterRegistry() - .summary("old.revisions", + .summary("revisions.old", generateTags(projectName, repoName, log) .add(Tag.of("init", Boolean.toString(normalized.major() == 1))) .build()) diff --git a/server/src/test/java/com/linecorp/centraldogma/server/MetricsTest.java b/server/src/test/java/com/linecorp/centraldogma/server/MetricsTest.java index dfe24d4d4..8a7447289 100644 --- a/server/src/test/java/com/linecorp/centraldogma/server/MetricsTest.java +++ b/server/src/test/java/com/linecorp/centraldogma/server/MetricsTest.java @@ -18,6 +18,7 @@ import static org.assertj.core.api.Assertions.assertThat; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; import org.junit.jupiter.api.extension.RegisterExtension; import com.fasterxml.jackson.databind.JsonNode; @@ -26,10 +27,12 @@ import com.linecorp.armeria.common.HttpStatus; import com.linecorp.armeria.common.MediaType; import com.linecorp.centraldogma.client.CentralDogma; +import com.linecorp.centraldogma.client.CentralDogmaRepository; import com.linecorp.centraldogma.client.WatchRequest; import com.linecorp.centraldogma.common.Change; import com.linecorp.centraldogma.common.Query; import com.linecorp.centraldogma.common.Revision; +import com.linecorp.centraldogma.testing.internal.FlakyTest; import com.linecorp.centraldogma.testing.junit.CentralDogmaExtension; import io.micrometer.core.instrument.MeterRegistry; @@ -37,6 +40,8 @@ import io.micrometer.prometheus.PrometheusMeterRegistry; import io.prometheus.client.exporter.common.TextFormat; +@FlakyTest +@Timeout(30) class MetricsTest { @RegisterExtension @@ -75,11 +80,28 @@ void metrics() { jsonNodeWatchRequest.start().join(); content = dogma.httpClient().get("/monitor/metrics").aggregate().join().contentUtf8(); assertThat(content).contains("com.linecorp.centraldogma.server.internal.api.WatchContentServiceV1"); - assertThat(content).doesNotContain("init_revisions"); + assertThat(content).doesNotContain("revisions_init"); // Trigger old revision recording jsonNodeWatchRequest.start(Revision.INIT).join(); content = dogma.httpClient().get("/monitor/metrics").aggregate().join().contentUtf8(); - assertThat(content).contains("init_revisions"); + assertThat(content).contains("revisions_init"); + assertThat(content).doesNotContain("revisions_old"); + + final CentralDogmaRepository centralDogmaRepo = dogma.client().forRepo("foo", "bar"); + for (int i = 0; i < 5000; i++) { + centralDogmaRepo.commit("Add a commit", Change.ofTextUpsert("/foo.txt", Integer.toString(i))) + .push().join(); + } + + jsonNodeWatchRequest.start(Revision.INIT).join(); + content = dogma.httpClient().get("/monitor/metrics").aggregate().join().contentUtf8(); + assertThat(content).contains("revisions_old"); + assertThat(content).contains("init=\"true\""); + assertThat(content).doesNotContain("init=\"false\""); + + jsonNodeWatchRequest.start(new Revision(2)).join(); + content = dogma.httpClient().get("/monitor/metrics").aggregate().join().contentUtf8(); + assertThat(content).contains("init=\"false\""); } } From dce63277ee83d711d7209dfc1eaf953c7518ce4c Mon Sep 17 00:00:00 2001 From: minwoox Date: Fri, 1 Sep 2023 16:59:17 +0900 Subject: [PATCH 10/10] Address comments from @ikhoon --- .../internal/api/RepositoryServiceV1.java | 18 +++++++++--------- .../thrift/CentralDogmaServiceImpl.java | 3 +-- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1.java index 10f7c9c8f..60359ef94 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/api/RepositoryServiceV1.java @@ -33,7 +33,7 @@ import com.google.common.collect.ImmutableMap; import com.linecorp.armeria.common.HttpStatus; -import com.linecorp.armeria.common.logging.RequestLog; +import com.linecorp.armeria.common.logging.RequestOnlyLog; import com.linecorp.armeria.server.ServiceRequestContext; import com.linecorp.armeria.server.annotation.Consumes; import com.linecorp.armeria.server.annotation.Delete; @@ -195,8 +195,7 @@ public Map normalizeRevision(ServiceRequestContext ctx, Repository repository, @Param String revision) { final Revision normalizedRevision = repository.normalizeNow(new Revision(revision)); final Revision head = repository.normalizeNow(Revision.HEAD); - increaseCounterIfOldRevisionUsed(ctx, repository.parent().name(), repository.name(), - normalizedRevision, head); + increaseCounterIfOldRevisionUsed(ctx, repository, normalizedRevision, head); return ImmutableMap.of("revision", normalizedRevision.major()); } @@ -204,20 +203,21 @@ static void increaseCounterIfOldRevisionUsed(ServiceRequestContext ctx, Reposito Revision revision) { final Revision normalized = repository.normalizeNow(revision); final Revision head = repository.normalizeNow(Revision.HEAD); - increaseCounterIfOldRevisionUsed(ctx, repository.parent().name(), repository.name(), normalized, head); + increaseCounterIfOldRevisionUsed(ctx, repository, normalized, head); } public static void increaseCounterIfOldRevisionUsed( - ServiceRequestContext ctx, String projectName, String repoName, - Revision normalized, Revision head) { + ServiceRequestContext ctx, Repository repository, Revision normalized, Revision head) { + final String projectName = repository.parent().name(); + final String repoName = repository.name(); if (normalized.major() == 1) { - ctx.log().whenComplete().thenAccept( + ctx.log().whenRequestComplete().thenAccept( log -> ctx.meterRegistry() .counter("revisions.init", generateTags(projectName, repoName, log).build()) .increment()); } if (head.major() - normalized.major() >= 5000) { - ctx.log().whenComplete().thenAccept( + ctx.log().whenRequestComplete().thenAccept( log -> ctx.meterRegistry() .summary("revisions.old", generateTags(projectName, repoName, log) @@ -228,7 +228,7 @@ public static void increaseCounterIfOldRevisionUsed( } private static ImmutableList.Builder generateTags( - String projectName, String repoName, RequestLog log) { + String projectName, String repoName, RequestOnlyLog log) { final ImmutableList.Builder builder = ImmutableList.builder(); return builder.add(Tag.of("project", projectName), Tag.of("repo", repoName), diff --git a/server/src/main/java/com/linecorp/centraldogma/server/internal/thrift/CentralDogmaServiceImpl.java b/server/src/main/java/com/linecorp/centraldogma/server/internal/thrift/CentralDogmaServiceImpl.java index ff036c9b0..681d5f460 100644 --- a/server/src/main/java/com/linecorp/centraldogma/server/internal/thrift/CentralDogmaServiceImpl.java +++ b/server/src/main/java/com/linecorp/centraldogma/server/internal/thrift/CentralDogmaServiceImpl.java @@ -232,8 +232,7 @@ private com.linecorp.centraldogma.common.Revision normalizeRevision( final com.linecorp.centraldogma.common.Revision normalized = repository.normalizeNow(convert(revision)); final com.linecorp.centraldogma.common.Revision head = repository.normalizeNow(HEAD); - increaseCounterIfOldRevisionUsed(RequestContext.current(), projectName, repositoryName, - normalized, head); + increaseCounterIfOldRevisionUsed(RequestContext.current(), repository, normalized, head); return normalized; }