Skip to content

Provide a way to log successful and failed requests #719

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ikhoon wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Provide a way to log successful and failed requests #719

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1fc884b
Provide a way to log successful and failed requests
ikhoon Aug 25, 2022
61438e7
Checkstyle
ikhoon Aug 29, 2022
5a862e6
Fix javadoc link and wrong tests
ikhoon Aug 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions common/src/main/java/com/linecorp/centraldogma/internal/api/v1/HttpApiV1Constants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ public final class HttpApiV1Constants {

public static final String METRICS_PATH = "/monitor/metrics";

public static final String DOCS_PATH = "/docs";

public static final String REMOVED = "/removed";

private HttpApiV1Constants() {}
Expand Down
5 changes: 4 additions & 1 deletion dist/src/conf/dogma.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,8 @@
},
"csrfTokenRequiredForThrift": null,
"accessLogFormat": "common",
"authentication": null
"authentication": null,
"requestLog": {
"targetGroups": [ "API" ]
}
}
107 changes: 93 additions & 14 deletions server/src/main/java/com/linecorp/centraldogma/server/CentralDogma.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import static com.google.common.base.Strings.isNullOrEmpty;
import static com.linecorp.centraldogma.internal.api.v1.HttpApiV1Constants.API_V0_PATH_PREFIX;
import static com.linecorp.centraldogma.internal.api.v1.HttpApiV1Constants.API_V1_PATH_PREFIX;
import static com.linecorp.centraldogma.internal.api.v1.HttpApiV1Constants.DOCS_PATH;
import static com.linecorp.centraldogma.internal.api.v1.HttpApiV1Constants.HEALTH_CHECK_PATH;
import static com.linecorp.centraldogma.internal.api.v1.HttpApiV1Constants.METRICS_PATH;
import static com.linecorp.centraldogma.server.auth.AuthProvider.BUILTIN_WEB_BASE_PATH;
Expand All @@ -38,6 +39,7 @@
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionStage;
import java.util.concurrent.Executor;
Expand Down Expand Up @@ -88,6 +90,7 @@
import com.linecorp.armeria.server.ServerPort;
import com.linecorp.armeria.server.ServiceNaming;
import com.linecorp.armeria.server.ServiceRequestContext;
import com.linecorp.armeria.server.TransientServiceOption;
import com.linecorp.armeria.server.auth.AuthService;
import com.linecorp.armeria.server.auth.Authorizer;
import com.linecorp.armeria.server.docs.DocService;
Expand All @@ -96,6 +99,7 @@
import com.linecorp.armeria.server.file.HttpFile;
import com.linecorp.armeria.server.healthcheck.HealthCheckService;
import com.linecorp.armeria.server.logging.AccessLogWriter;
import com.linecorp.armeria.server.logging.LoggingService;
import com.linecorp.armeria.server.metric.MetricCollectingService;
import com.linecorp.armeria.server.metric.PrometheusExpositionService;
import com.linecorp.armeria.server.thrift.THttpService;
Expand Down Expand Up @@ -147,11 +151,11 @@

import io.micrometer.core.instrument.MeterRegistry;
import io.micrometer.core.instrument.binder.jvm.ClassLoaderMetrics;
import io.micrometer.core.instrument.binder.jvm.DiskSpaceMetrics;
import io.micrometer.core.instrument.binder.jvm.ExecutorServiceMetrics;
import io.micrometer.core.instrument.binder.jvm.JvmGcMetrics;
import io.micrometer.core.instrument.binder.jvm.JvmMemoryMetrics;
import io.micrometer.core.instrument.binder.jvm.JvmThreadMetrics;
import io.micrometer.core.instrument.binder.system.DiskSpaceMetrics;
import io.micrometer.core.instrument.binder.system.FileDescriptorMetrics;
import io.micrometer.core.instrument.binder.system.ProcessorMetrics;
import io.micrometer.core.instrument.binder.system.UptimeMetrics;
Expand Down Expand Up @@ -206,6 +210,8 @@ public static CentralDogma forConfig(File configFile) throws IOException {
private PrometheusMeterRegistry meterRegistry;
@Nullable
private SessionManager sessionManager;
@Nullable
private JvmGcMetrics jvmGcMetrics;

CentralDogma(CentralDogmaConfig cfg) {
this.cfg = requireNonNull(cfg, "cfg");
Expand Down Expand Up @@ -385,13 +391,13 @@ private CommandExecutor startCommandExecutor(
logger.info("Starting plugins on the leader replica ..");
pluginsForLeaderOnly
.start(cfg, pm, exec, meterRegistry, purgeWorker).handle((unused, cause) -> {
if (cause == null) {
logger.info("Started plugins on the leader replica.");
} else {
logger.error("Failed to start plugins on the leader replica..", cause);
}
return null;
});
if (cause == null) {
logger.info("Started plugins on the leader replica.");
} else {
logger.error("Failed to start plugins on the leader replica..", cause);
}
return null;
});
}
};

Expand Down Expand Up @@ -521,9 +527,67 @@ private Server startServer(ProjectManager pm, CommandExecutor executor,

sb.service("/title", webAppTitleFile(cfg.webAppTitle(), SystemInfo.hostname()).asService());

sb.service(HEALTH_CHECK_PATH, HealthCheckService.of());
final RequestLogConfig requestLogConfig = cfg.requestLogConfig();
boolean logHealthCheck = false;
boolean logMetrics = false;
if (requestLogConfig != null) {
final Function<? super HttpService, LoggingService> loggingService =
LoggingService.builder()
.logger(requestLogConfig.loggerName())
.requestLogLevel(requestLogConfig.requestLogLevel())
.successfulResponseLogLevel(requestLogConfig.successfulResponseLogLevel())
.failureResponseLogLevel(requestLogConfig.failureResponseLogLevel())
.successSamplingRate(requestLogConfig.successSamplingRate())
.failureSamplingRate(requestLogConfig.failureSamplingRate())
.newDecorator();
final Set<RequestLogGroup> requestLogGroups = requestLogConfig.targetGroups();
if (requestLogGroups.contains(RequestLogGroup.ALL)) {
sb.decorator(loggingService);
logHealthCheck = true;
logMetrics = true;
} else {
for (RequestLogGroup logGroup : requestLogGroups) {

@jrhee17 jrhee17 Sep 8, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Is it possible that users may want to configure different logging parameters for different paths?

e.g.

- METRIC: debug
- API: info
- ...

@ikhoon ikhoon Sep 8, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh... It is so nice! 👍

@ikhoon ikhoon Sep 8, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can implement it by adding different LoggingServices.

@minwoox minwoox Oct 7, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about also supporting the specified path? e.g. /api/v1

switch (logGroup) {
case API:
sb.decoratorUnder("/api", loggingService);
break;
case METRICS:
sb.decorator(METRICS_PATH, loggingService);
logMetrics = true;
break;
case HEALTH:
sb.decorator(HEALTH_CHECK_PATH, loggingService);
logHealthCheck = true;
break;
case DOCS:
sb.decoratorUnder(DOCS_PATH, loggingService);
break;
case WEB:
for (String webResources : ImmutableList.of("/web", "/vendor",
"/scripts", "/styles")) {
sb.decoratorUnder(webResources, loggingService);
}
break;
case ALL:
// Should never reach here.
throw new Error();
}
}
}
}

final HealthCheckService healthCheckService;
if (logHealthCheck) {
healthCheckService =
HealthCheckService.builder()
.transientServiceOptions(TransientServiceOption.WITH_SERVICE_LOGGING)
.build();
} else {
healthCheckService = HealthCheckService.of();
}
sb.service(HEALTH_CHECK_PATH, healthCheckService);

sb.serviceUnder("/docs/",
sb.serviceUnder(DOCS_PATH,
DocService.builder()
.exampleHeaders(CentralDogmaService.class,
HttpHeaders.of(HttpHeaderNames.AUTHORIZATION,
Expand All @@ -532,7 +596,7 @@ private Server startServer(ProjectManager pm, CommandExecutor executor,

configureHttpApi(sb, pm, executor, watchService, mds, authProvider, sessionManager);

configureMetrics(sb, meterRegistry);
configureMetrics(sb, meterRegistry, logMetrics);

// Configure access log format.
final String accessLogFormat = cfg.accessLogFormat();
Expand Down Expand Up @@ -776,9 +840,18 @@ private static Function<? super HttpService, EncodingService> contentEncodingDec
.build(delegate);
}

private void configureMetrics(ServerBuilder sb, PrometheusMeterRegistry registry) {
private void configureMetrics(ServerBuilder sb, PrometheusMeterRegistry registry, boolean logMetrics) {
sb.meterRegistry(registry);
sb.service(METRICS_PATH, new PrometheusExpositionService(registry.getPrometheusRegistry()));
final PrometheusExpositionService expositionService;
if (logMetrics) {
expositionService = PrometheusExpositionService.builder(registry.getPrometheusRegistry())
.transientServiceOptions(
TransientServiceOption.WITH_SERVICE_LOGGING)
.build();
} else {
expositionService = PrometheusExpositionService.of(registry.getPrometheusRegistry());
}
sb.service(METRICS_PATH, expositionService);
sb.decorator(MetricCollectingService.newDecorator(MeterIdPrefixFunction.ofDefault("api")));

// Bind system metrics.
Expand All @@ -787,7 +860,8 @@ private void configureMetrics(ServerBuilder sb, PrometheusMeterRegistry registry
new ClassLoaderMetrics().bindTo(registry);
new UptimeMetrics().bindTo(registry);
new DiskSpaceMetrics(cfg.dataDir()).bindTo(registry);
new JvmGcMetrics().bindTo(registry);
jvmGcMetrics = new JvmGcMetrics();
jvmGcMetrics.bindTo(registry);
new JvmMemoryMetrics().bindTo(registry);
new JvmThreadMetrics().bindTo(registry);

Expand Down Expand Up @@ -817,6 +891,11 @@ private void doStop() {
meterRegistry = null;
}

if (jvmGcMetrics != null) {
jvmGcMetrics.close();
jvmGcMetrics = null;
}

logger.info("Stopping the Central Dogma ..");
if (!doStop(server, executor, pm, repositoryWorker, purgeWorker, sessionManager)) {
logger.warn("Stopped the Central Dogma with failure.");
Expand Down
14 changes: 13 additions & 1 deletion server/src/main/java/com/linecorp/centraldogma/server/CentralDogmaBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ public final class CentralDogmaBuilder {
private Object authProviderProperties;
private int writeQuota;
private int timeWindowSeconds;
@Nullable
private RequestLogConfig requestLogConfig;

/**
* Creates a new builder with the specified data directory.
Expand Down Expand Up @@ -520,6 +522,16 @@ public CentralDogmaBuilder writeQuotaPerRepository(int writeQuota, int timeWindo
return this;
}

/**
* Sets the {@link RequestLogConfig} to log requests, responses and exceptions in detail.
* If unspecified, request logging is disabled.
*/
public CentralDogmaBuilder requestLogConfig(RequestLogConfig requestLogConfig) {
requireNonNull(requestLogConfig, "requestLogConfig");
this.requestLogConfig = requestLogConfig;
return this;
}

/**
* Returns a newly-created {@link CentralDogma} server.
*/
Expand Down Expand Up @@ -553,6 +565,6 @@ private CentralDogmaConfig buildConfig() {
maxRemovedRepositoryAgeMillis, gracefulShutdownTimeout,
webAppEnabled, webAppTitle, mirroringEnabled, numMirroringThreads,
maxNumFilesPerMirror, maxNumBytesPerMirror, replicationConfig,
null, accessLogFormat, authCfg, quotaConfig);
null, accessLogFormat, authCfg, quotaConfig, requestLogConfig);
}
}
16 changes: 15 additions & 1 deletion server/src/main/java/com/linecorp/centraldogma/server/CentralDogmaConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@ public final class CentralDogmaConfig {
@Nullable
private final QuotaConfig writeQuotaPerRepository;

@Nullable
private final RequestLogConfig requestLogConfig;

CentralDogmaConfig(
@JsonProperty(value = "dataDir", required = true) File dataDir,
@JsonProperty(value = "ports", required = true)
Expand Down Expand Up @@ -165,7 +168,8 @@ public final class CentralDogmaConfig {
@JsonProperty("csrfTokenRequiredForThrift") @Nullable Boolean csrfTokenRequiredForThrift,
@JsonProperty("accessLogFormat") @Nullable String accessLogFormat,
@JsonProperty("authentication") @Nullable AuthConfig authConfig,
@JsonProperty("writeQuotaPerRepository") @Nullable QuotaConfig writeQuotaPerRepository) {
@JsonProperty("writeQuotaPerRepository") @Nullable QuotaConfig writeQuotaPerRepository,
@JsonProperty("requestLog") @Nullable RequestLogConfig requestLogConfig) {

this.dataDir = requireNonNull(dataDir, "dataDir");
this.ports = ImmutableList.copyOf(requireNonNull(ports, "ports"));
Expand Down Expand Up @@ -219,6 +223,7 @@ public final class CentralDogmaConfig {
ports.stream().anyMatch(ServerPort::hasProxyProtocol));

this.writeQuotaPerRepository = writeQuotaPerRepository;
this.requestLogConfig = requestLogConfig;
}

/**
Expand Down Expand Up @@ -456,6 +461,15 @@ public QuotaConfig writeQuotaPerRepository() {
return writeQuotaPerRepository;
}

/**
* Returns the {@link RequestLogConfig}.
*/
@JsonProperty("requestLog")
@Nullable
public RequestLogConfig requestLogConfig() {
return requestLogConfig;
}

@Override
public String toString() {
try {
Expand Down
Loading