Applied changes from the review + added automatic docs generation

dependencies.toml

@@ -81,6 +81,7 @@ logback12 = "1.2.11"
 logback14 = "1.4.7"
 micrometer = "1.11.1"
 micrometer-tracing = "1.1.2"
+micrometer-docs-generator = "1.0.2"
 micrometer13 = "1.3.20"
 mockito = "4.11.0"
 monix = "3.4.1"
@@ -791,6 +792,11 @@ module = "io.micrometer:micrometer-tracing-integration-test"
 version.ref = "micrometer-tracing"
 javadocs = "https://www.javadoc.io/doc/io.micrometer/micrometer-tracing-integration-test/1.1.2/"
 
+[libraries.micrometer-docs-generator]
+module = "io.micrometer:micrometer-docs-generator"
+version.ref = "micrometer-docs-generator"
+javadocs = "https://www.javadoc.io/doc/io.micrometer/micrometer-docs-generator/1.0.2/"
+
 [libraries.micrometer13-core]
 module = "io.micrometer:micrometer-core"
 version.ref = "micrometer13"

observation/build.gradle

@@ -16,3 +16,18 @@ dependencies {
  testImplementation libs.brave.context.slf4j
  testImplementation libs.brave.instrumentation.http.tests
 }
+
+configurations {
+ adoc
+}
+
+dependencies {
+ adoc libs.micrometer.docs.generator
+}
+
+task generateObservabilityDocs(type: JavaExec) {
+ mainClass = "io.micrometer.docs.DocsGeneratorCommand"
+ classpath configurations.adoc
+ // input folder, inclusion pattern, output folder
+ args project.projectDir.getAbsolutePath(), ".*", project.buildDir.getAbsolutePath()
+}

observation/src/main/java/com/linecorp/armeria/client/observation/DefaultHttpClientObservationConvention.java

@@ -22,6 +22,7 @@
 
 import com.linecorp.armeria.client.ClientRequestContext;
 import com.linecorp.armeria.client.observation.HttpClientObservationDocumentation.HighCardinalityKeys;
+import com.linecorp.armeria.client.observation.HttpClientObservationDocumentation.LowCardinalityKeys;
 import com.linecorp.armeria.common.SerializationFormat;
 import com.linecorp.armeria.common.SessionProtocol;
 import com.linecorp.armeria.common.annotation.Nullable;
@@ -36,39 +37,40 @@ class DefaultHttpClientObservationConvention implements HttpClientObservationCon
 
  @Override
  public KeyValues getLowCardinalityKeyValues(HttpClientContext context) {
- // TODO: What to move here?
- return HttpClientObservationConvention.super.getLowCardinalityKeyValues(context);
+ final ClientRequestContext ctx = context.getClientRequestContext();
+ KeyValues keyValues = KeyValues.of(
+ LowCardinalityKeys.HTTP_METHOD.withValue(ctx.method().name()));
+ if (context.getResponse() != null) {
+ final RequestLog log = ctx.log().ensureComplete();
+ keyValues = keyValues.and(LowCardinalityKeys.HTTP_PROTOCOL.withValue(protocol(log)));
+ final String serFmt = serializationFormat(log);
+ if (serFmt != null) {
+ keyValues = keyValues.and(LowCardinalityKeys.HTTP_SERIALIZATION_FORMAT.withValue(serFmt));
+ }
+ keyValues = keyValues.and(LowCardinalityKeys.STATUS_CODE
+ .withValue(log.responseStatus().codeAsText()));
+ }
+ return keyValues;
  }
 
  @Override
  public KeyValues getHighCardinalityKeyValues(HttpClientContext context) {
  final ClientRequestContext ctx = context.getClientRequestContext();
  KeyValues keyValues = KeyValues.of(
- HighCardinalityKeys.HTTP_METHOD.withValue(ctx.method().name()),
  HighCardinalityKeys.HTTP_PATH.withValue(ctx.path()),
  HighCardinalityKeys.HTTP_HOST.withValue(firstNonNull(ctx.authority(), "UNKNOWN")),
  HighCardinalityKeys.HTTP_URL.withValue(ctx.uri().toString())
  );
  if (context.getResponse() != null) {
  final RequestLog log = ctx.log().ensureComplete();
- keyValues = keyValues.and(HighCardinalityKeys.HTTP_PROTOCOL.withValue(protocol(log)));
-
- final String serFmt = serializationFormat(log);
- if (serFmt != null) {
- keyValues = keyValues.and(HighCardinalityKeys.HTTP_SERIALIZATION_FORMAT.withValue(serFmt));
- }
-
  final InetSocketAddress raddr = ctx.remoteAddress();
  if (raddr != null) {
  keyValues = keyValues.and(HighCardinalityKeys.ADDRESS_REMOTE.withValue(raddr.toString()));
  }
-
  final InetSocketAddress laddr = ctx.localAddress();
  if (laddr != null) {
  keyValues = keyValues.and(HighCardinalityKeys.ADDRESS_LOCAL.withValue(laddr.toString()));
  }
- keyValues = keyValues.and(HighCardinalityKeys.STATUS_CODE
- .withValue(log.responseStatus().codeAsText()));
  if (log.responseStatus().isError()) {
  keyValues = keyValues.and(HighCardinalityKeys.ERROR
  .withValue(log.responseStatus().codeAsText()));

observation/src/main/java/com/linecorp/armeria/client/observation/HttpClientObservationDocumentation.java

@@ -24,13 +24,19 @@
 
 enum HttpClientObservationDocumentation implements ObservationDocumentation {
 
+ /**
+ * TODO: Add docs.
+ */
  OBSERVATION {
  @Override
  public Class<? extends ObservationConvention<? extends Context>> getDefaultConvention() {
  return DefaultHttpClientObservationConvention.class;
  }
 
- // TODO: Figure what should be low and what high cardinality
+ @Override
+ public KeyName[] getLowCardinalityKeyNames() {
+ return LowCardinalityKeys.values();
+ }
 
  @Override
  public KeyName[] getHighCardinalityKeyNames() {
@@ -43,71 +49,104 @@ public Event[] getEvents() {
  }
  };
 
- enum HighCardinalityKeys implements KeyName {
+ enum LowCardinalityKeys implements KeyName {
 
- HTTP_PATH {
+ /**
+ * TODO: Add docs.
+ */
+ HTTP_METHOD {
  @Override
  public String asString() {
- return "http.path";
+ return "http.method";
  }
  },
 
- HTTP_METHOD {
+ /**
+ * TODO: Add docs.
+ */
+ STATUS_CODE {
  @Override
  public String asString() {
- return "http.method";
+ return "http.status_code";
  }
  },
 
- HTTP_HOST {
+ /**
+ * TODO: Add docs.
+ */
+ HTTP_PROTOCOL {
  @Override
  public String asString() {
- return "http.host";
+ return "http.protocol";
  }
  },
 
- HTTP_URL {
+ /**
+ * TODO: Add docs.
+ */
+ HTTP_SERIALIZATION_FORMAT {
  @Override
  public String asString() {
- return "http.url";
+ return "http.serfmt";
  }
- },
+ }
+ }
 
- HTTP_PROTOCOL {
+ enum HighCardinalityKeys implements KeyName {
+
+ /**
+ * TODO: Add docs.
+ */
+ HTTP_PATH {
  @Override
  public String asString() {
- return "http.protocol";
+ return "http.path";
  }
  },
 
- HTTP_SERIALIZATION_FORMAT {
+ /**
+ * TODO: Add docs.
+ */
+ HTTP_HOST {
  @Override
  public String asString() {
- return "http.serfmt";
+ return "http.host";
  }
  },
 
- ADDRESS_REMOTE {
+ /**
+ * TODO: Add docs.
+ */
+ HTTP_URL {
  @Override
  public String asString() {
- return "address.remote";
+ return "http.url";
  }
  },
 
- ADDRESS_LOCAL {
+ /**
+ * TODO: Add docs.
+ */
+ ADDRESS_REMOTE {
  @Override
  public String asString() {
- return "address.local";
+ return "address.remote";
  }
  },
 
- STATUS_CODE {
+ /**
+ * TODO: Add docs.
+ */
+ ADDRESS_LOCAL {
  @Override
  public String asString() {
- return "http.status_code";
+ return "address.local";
  }
  },
 
+ /**
+ * TODO: Add docs.
+ */
  ERROR {
  @Override
  public String asString() {
@@ -126,6 +165,11 @@ enum Events implements Event {
  public String getName() {
  return "ws";
  }
+
+ @Override
+ public String getContextualName() {
+ return "ws";
+ }
  },
 
  /**
@@ -136,6 +180,11 @@ public String getName() {
  public String getName() {
  return "wr";
  }
+
+ @Override
+ public String getContextualName() {
+ return "wr";
+ }
  }
  }
 }

observation/src/main/java/com/linecorp/armeria/client/observation/MicrometerObservationClient.java

@@ -16,6 +16,8 @@
 
 package com.linecorp.armeria.client.observation;
 
+import static java.util.Objects.requireNonNull;
+
 import java.util.function.Function;
 
 import com.linecorp.armeria.client.ClientRequestContext;
@@ -78,7 +80,7 @@ private MicrometerObservationClient(
  HttpClient delegate, ObservationRegistry observationRegistry,
  @Nullable HttpClientObservationConvention httpClientObservationConvention) {
  super(delegate);
- this.observationRegistry = observationRegistry;
+ this.observationRegistry = requireNonNull(observationRegistry, "observationRegistry");
  this.httpClientObservationConvention = httpClientObservationConvention;
  }
 

observation/src/main/java/com/linecorp/armeria/server/observation/DefaultServiceObservationConvention.java

@@ -26,6 +26,7 @@
 import com.linecorp.armeria.common.logging.RequestLog;
 import com.linecorp.armeria.server.ServiceRequestContext;
 import com.linecorp.armeria.server.observation.ServiceObservationDocumentation.HighCardinalityKeys;
+import com.linecorp.armeria.server.observation.ServiceObservationDocumentation.LowCardinalityKeys;
 
 import io.micrometer.common.KeyValues;
 
@@ -34,26 +35,37 @@ final class DefaultServiceObservationConvention implements ServiceObservationCon
  static final DefaultServiceObservationConvention INSTANCE =
  new DefaultServiceObservationConvention();
 
+ @Override
+ public KeyValues getLowCardinalityKeyValues(HttpServerContext context) {
+ final ServiceRequestContext ctx = context.getServiceRequestContext();
+ KeyValues keyValues = KeyValues.of(
+ LowCardinalityKeys.HTTP_METHOD.withValue(ctx.method().name()));
+ if (context.getResponse() != null) {
+ final RequestLog log = ctx.log().ensureComplete();
+ keyValues = keyValues.and(
+ LowCardinalityKeys.HTTP_PROTOCOL.withValue(protocol(log)));
+ final String serFmt = serializationFormat(log);
+ if (serFmt != null) {
+ keyValues = keyValues.and(
+ LowCardinalityKeys.HTTP_SERIALIZATION_FORMAT.withValue(serFmt));
+ }
+ keyValues = keyValues.and(
+ LowCardinalityKeys.STATUS_CODE.withValue(log.responseStatus().codeAsText()));
+ }
+ return keyValues;
+ }
+
  @Override
  public KeyValues getHighCardinalityKeyValues(HttpServerContext context) {
  final ServiceRequestContext ctx = context.getServiceRequestContext();
  KeyValues keyValues = KeyValues.of(
- HighCardinalityKeys.HTTP_METHOD.withValue(ctx.method().name()),
  HighCardinalityKeys.HTTP_PATH.withValue(ctx.path()),
  HighCardinalityKeys.HTTP_HOST.withValue(
  firstNonNull(context.getHttpRequest().authority(), "UNKNOWN")),
  HighCardinalityKeys.HTTP_URL.withValue(ctx.uri().toString())
  );
  if (context.getResponse() != null) {
  final RequestLog log = ctx.log().ensureComplete();
- keyValues = keyValues.and(
- HighCardinalityKeys.HTTP_PROTOCOL.withValue(protocol(log)));
-
- final String serFmt = serializationFormat(log);
- if (serFmt != null) {
- keyValues = keyValues.and(
- HighCardinalityKeys.HTTP_SERIALIZATION_FORMAT.withValue(serFmt));
- }
 
  final InetSocketAddress raddr = ctx.remoteAddress();
  if (raddr != null) {
@@ -66,8 +78,6 @@ public KeyValues getHighCardinalityKeyValues(HttpServerContext context) {
  keyValues = keyValues.and(
  HighCardinalityKeys.ADDRESS_LOCAL.withValue(laddr.toString()));
  }
- keyValues = keyValues.and(
- HighCardinalityKeys.STATUS_CODE.withValue(log.responseStatus().codeAsText()));
  if (log.responseStatus().isError()) {
  keyValues = keyValues.and(
  HighCardinalityKeys.ERROR.withValue(log.responseStatus().codeAsText()));

observation/src/main/java/com/linecorp/armeria/server/observation/MicrometerObservationService.java

@@ -16,6 +16,8 @@
 
 package com.linecorp.armeria.server.observation;
 
+import static java.util.Objects.requireNonNull;
+
 import java.util.function.Function;
 
 import com.linecorp.armeria.common.HttpRequest;
@@ -66,14 +68,13 @@ private MicrometerObservationService(HttpService delegate, ObservationRegistry o
  private MicrometerObservationService(HttpService delegate, ObservationRegistry observationRegistry,
  @Nullable ServiceObservationConvention serviceObservationConvention) {
  super(delegate);
- this.observationRegistry = observationRegistry;
+ this.observationRegistry = requireNonNull(observationRegistry, "observationRegistry");
  this.serviceObservationConvention = serviceObservationConvention;
  }
 
  @Override
  public HttpResponse serve(ServiceRequestContext ctx, HttpRequest req) throws Exception {
- // TODO: What about this?
- if (!ctx.config().transientServiceOptions().contains(TransientServiceOption.WITH_TRACING) &&
+ if (!ctx.config().transientServiceOptions().contains(TransientServiceOption.WITH_TRACING) ||
  !ctx.config().transientServiceOptions().contains(
  TransientServiceOption.WITH_METRIC_COLLECTION)) {
  return unwrap().serve(ctx, req);
@@ -92,8 +93,7 @@ public HttpResponse serve(ServiceRequestContext ctx, HttpRequest req) throws Exc
 
  enrichObservation(ctx, httpServerContext, observation);
 
- return observation.scopedChecked(
- () -> unwrap().serve(ctx, req)); // TODO: Maybe we should observation stopping here
+ return observation.scopedChecked(() -> unwrap().serve(ctx, req));
  }
 
  private void enrichObservation(ServiceRequestContext ctx, HttpServerContext httpServerContext,
@@ -119,8 +119,6 @@ private void enrichObservation(ServiceRequestContext ctx, HttpServerContext http
  .thenAccept(requestLog -> {
  httpServerContext.setResponse(requestLog);
  observation.stop();
- // TODO: ClientConnectionTimings - no hook to be there at the
- // moment of those things actually hapenning
  });
  }
 }