WIP on KeyValuesConvention

Author: marcingrzejszczak

dependencies.gradle

@@ -43,6 +43,7 @@ def VERSIONS = [
         'javax.xml.bind:jaxb-api:2.3.+',
         // TODO: Consider where this should live
         'io.micrometer:context-propagation-api:1.0.0-M2',
+        'io.opentelemetry:opentelemetry-semconv:1.14.0-alpha',
         'net.sf.ehcache:ehcache:latest.release',
         'org.apache.httpcomponents:httpasyncclient:latest.release',
         'org.apache.httpcomponents:httpclient:latest.release',

micrometer-commons/src/main/java/io/micrometer/common/KeyValue.java

@@ -15,6 +15,8 @@
  */
 package io.micrometer.common;
 
+import java.util.function.Predicate;
+
 /**
  * Key/value pair representing a dimension of a meter used to classify and drill into
  * measurements.
@@ -32,6 +34,14 @@ static KeyValue of(String key, String value) {
         return new ImmutableKeyValue(key, value);
     }
 
+    static KeyValue of(String key, Object value, Predicate<Object> validator) {
+        return new ValidatedKeyValue<>(key, value, validator);
+    }
+
+    static KeyValue ofUnknownValue(String key) {
+        return KeyValue.of(key, "UNKNOWN");
+    }
+
     @Override
     default int compareTo(KeyValue o) {
         return getKey().compareTo(o.getKey());

micrometer-commons/src/main/java/io/micrometer/common/ValidatedKeyValue.java

@@ -0,0 +1,54 @@
+/**
+ * Copyright 2022 VMware, Inc.
+ * <p>
+ * 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
+ * <p>
+ * https://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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 io.micrometer.common;
+
+import java.util.function.Predicate;
+
+/**
+ * {@link KeyValue} with value validation.
+ *
+ * @param <T>
+ * @author Marcin Grzejszczak
+ * @since 1.10.0
+ */
+class ValidatedKeyValue<T> implements KeyValue {
+
+    private final String key;
+
+    private final T value;
+
+    ValidatedKeyValue(String key, T value, Predicate<Object> validator) {
+        this.key = key;
+        this.value = assertValue(validator, value);
+    }
+
+    @Override
+    public String getKey() {
+        return this.key;
+    }
+
+    @Override
+    public String getValue() {
+        return String.valueOf(this.value);
+    }
+
+    private T assertValue(Predicate<Object> validator, T value) {
+        if (!validator.test(value)) {
+            throw new IllegalArgumentException("Argument [" + this.value + "] does not follow required format for key [" + this.key + "]");
+        }
+        return value;
+    }
+}

micrometer-core/build.gradle

@@ -101,6 +101,8 @@ dependencies {
     testImplementation 'org.postgresql:postgresql'
 
     testImplementation 'org.testcontainers:mongodb'
+
+    testImplementation 'io.opentelemetry:opentelemetry-semconv'
 }
 
 task shenandoahTest(type: Test) {

micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/DefaultOkHttpKeyValuesProvider.java

@@ -68,6 +68,7 @@ private static Method getMethod(Class<?>... parameterTypes) {
     private static final KeyValues TAGS_TARGET_UNKNOWN = KeyValues.of(TAG_TARGET_SCHEME, TAG_VALUE_UNKNOWN, TAG_TARGET_HOST,
             TAG_VALUE_UNKNOWN, TAG_TARGET_PORT, TAG_VALUE_UNKNOWN);
 
+
     @Override
     public KeyValues getLowCardinalityKeyValues(OkHttpContext context) {
         OkHttpMetricsEventListener.CallState state = context.getState();
@@ -78,7 +79,6 @@ public KeyValues getLowCardinalityKeyValues(OkHttpContext context) {
         Iterable<BiFunction<Request, Response, Tag>> contextSpecificTags = context.getContextSpecificTags();
         Iterable<Tag> unknownRequestTags = context.getUnknownRequestTags();
         boolean includeHostTag = context.isIncludeHostTag();
-
         KeyValues keyValues = KeyValues.of("method", requestAvailable ? request.method() : TAG_VALUE_UNKNOWN, "uri", getUriTag(urlMapper, state, request),
                         "status", getStatusMessage(state.response, state.exception))
                 .and(tagsToKeyValues(stream(extraTags.spliterator(), false)))
@@ -88,11 +88,9 @@ public KeyValues getLowCardinalityKeyValues(OkHttpContext context) {
                         .collect(toList()))
                 .and(getRequestTags(request, tagsToKeyValues(stream(unknownRequestTags.spliterator(), false))))
                 .and(generateTagsForRoute(request));
-
         if (includeHostTag) {
             keyValues = KeyValues.of(keyValues).and("host", requestAvailable ? request.url().host() : TAG_VALUE_UNKNOWN);
         }
-
         return keyValues;
     }
 

micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListener.java

@@ -28,6 +28,7 @@
 import io.micrometer.observation.ObservationRegistry;
 import io.micrometer.observation.transport.http.HttpClientRequest;
 import io.micrometer.observation.transport.http.HttpClientResponse;
+import io.micrometer.observation.transport.http.tags.HttpClientKeyValuesConvention;
 import okhttp3.EventListener;
 import okhttp3.*;
 
@@ -70,7 +71,7 @@ public class OkHttpMetricsEventListener extends EventListener {
 
     private final ObservationRegistry observationRegistry;
 
-    private final OkHttpKeyValuesProvider keyValuesProvider;
+    private final Observation.KeyValuesProvider<OkHttpContext> keyValuesProvider;
 
     private final String requestsMetricName;
 
@@ -95,7 +96,7 @@ protected OkHttpMetricsEventListener(MeterRegistry registry, String requestsMetr
         this(registry, ObservationRegistry.NOOP, new DefaultOkHttpKeyValuesProvider(), requestsMetricName, urlMapper, extraTags, contextSpecificTags, emptyList(), true);
     }
 
-    OkHttpMetricsEventListener(MeterRegistry registry, ObservationRegistry observationRegistry, OkHttpKeyValuesProvider keyValuesProvider, String requestsMetricName, Function<Request, String> urlMapper,
+    OkHttpMetricsEventListener(MeterRegistry registry, ObservationRegistry observationRegistry, Observation.KeyValuesProvider<OkHttpContext> keyValuesProvider, String requestsMetricName, Function<Request, String> urlMapper,
             Iterable<Tag> extraTags, Iterable<BiFunction<Request, Response, Tag>> contextSpecificTags,
             Iterable<String> requestTagKeys, boolean includeHostTag) {
         this.registry = registry;
@@ -280,7 +281,7 @@ public static class Builder {
 
         private Iterable<String> requestTagKeys = Collections.emptyList();
 
-        private OkHttpKeyValuesProvider keyValuesProvider = new DefaultOkHttpKeyValuesProvider();
+        private OkHttpKeyValuesProvider keyValuesProvider;
 
         Builder(MeterRegistry registry, String name) {
             this.registry = registry;
@@ -373,8 +374,24 @@ public Builder requestTagKeys(Iterable<String> requestTagKeys) {
             return this;
         }
 
+        @SuppressWarnings("unchecked")
         public OkHttpMetricsEventListener build() {
-            return new OkHttpMetricsEventListener(registry, observationRegistry, keyValuesProvider, name, uriMapper, tags, contextSpecificTags, requestTagKeys,
+            Observation.KeyValuesProvider provider = null;
+            if (this.keyValuesProvider != null) {
+                provider = this.keyValuesProvider;
+            }
+            else if (observationRegistry.isNoOp() || observationRegistry.observationConfig().getKeyValuesConfiguration() == ObservationRegistry.KeyValuesConfiguration.LEGACY) {
+                provider = new DefaultOkHttpKeyValuesProvider();
+            }
+            else if (observationRegistry.observationConfig().getKeyValuesConfiguration() == ObservationRegistry.KeyValuesConfiguration.STANDARDIZED) {
+                // TODO: Isn't this too much - maybe we should just require the user to set this manually?
+                provider = new StandardizedOkHttpKeyValuesProvider(observationRegistry.observationConfig().getKeyValuesConvention(HttpClientKeyValuesConvention.class));
+            }
+            else {
+                provider = new Observation.KeyValuesProvider.CompositeKeyValuesProvider(new DefaultOkHttpKeyValuesProvider(), new StandardizedOkHttpKeyValuesProvider(observationRegistry.observationConfig().getKeyValuesConvention(HttpClientKeyValuesConvention.class)));
+            }
+
+            return new OkHttpMetricsEventListener(registry, observationRegistry, provider, name, uriMapper, tags, contextSpecificTags, requestTagKeys,
                     includeHostTag);
         }
 

micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/StandardizedOkHttpKeyValuesProvider.java

@@ -0,0 +1,55 @@
+/*
+ * Copyright 2017 VMware, Inc.
+ *
+ * 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
+ *
+ * https://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 io.micrometer.core.instrument.binder.okhttp3;
+
+import io.micrometer.common.KeyValue;
+import io.micrometer.common.KeyValues;
+import io.micrometer.common.lang.NonNullApi;
+import io.micrometer.common.lang.NonNullFields;
+import io.micrometer.core.instrument.Tag;
+import io.micrometer.observation.transport.http.tags.HttpClientKeyValuesConvention;
+import okhttp3.Request;
+import okhttp3.Response;
+
+import java.util.function.BiFunction;
+
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.StreamSupport.stream;
+
+@NonNullApi
+@NonNullFields
+public class StandardizedOkHttpKeyValuesProvider implements OkHttpKeyValuesProvider {
+
+    private final HttpClientKeyValuesConvention keyValuesConvention;
+
+    public StandardizedOkHttpKeyValuesProvider(HttpClientKeyValuesConvention keyValuesConvention) {
+        this.keyValuesConvention = keyValuesConvention;
+    }
+
+    @Override
+    public KeyValues getLowCardinalityKeyValues(OkHttpContext context) {
+        OkHttpMetricsEventListener.CallState state = context.getState();
+        Request request = state.request;
+        Iterable<BiFunction<Request, Response, Tag>> contextSpecificTags = context.getContextSpecificTags();
+        // TODO: What to do when there is no request or response - do we set UNKNOWN ?
+        return KeyValues.of(keyValuesConvention.all(context.getRequest(), context.getResponse()))
+                .and(stream(contextSpecificTags.spliterator(), false)
+                        .map(contextTag -> contextTag.apply(request, state.response))
+                        .map(tag -> KeyValue.of(tag.getKey(), tag.getValue()))
+                        .collect(toList()));
+    }
+
+}

micrometer-core/src/test/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListenerTest.java

@@ -27,6 +27,7 @@
 import io.micrometer.observation.Observation;
 import io.micrometer.observation.ObservationHandler;
 import io.micrometer.observation.ObservationRegistry;
+import io.micrometer.observation.transport.http.tags.OpenTelemetryHttpClientKeyValuesConvention;
 import okhttp3.Cache;
 import okhttp3.OkHttpClient;
 import okhttp3.Request;
@@ -88,6 +89,8 @@ void timeSuccessful(@WiremockResolver.Wiremock WireMockServer server) throws IOE
     void timeSuccessfulWithObservation(@WiremockResolver.Wiremock WireMockServer server) throws IOException {
         ObservationRegistry observationRegistry = ObservationRegistry.create();
         TestHandler testHandler = new TestHandler();
+        observationRegistry.observationConfig().keyValuesConfiguration(ObservationRegistry.KeyValuesConfiguration.LEGACY_WITH_STANDARDIZED);
+        observationRegistry.observationConfig().keyValuesConvention(new OpenTelemetryHttpClientKeyValuesConvention());
         observationRegistry.observationConfig().observationHandler(testHandler);
         observationRegistry.observationConfig().observationHandler(new TimerObservationHandler(registry));
         client = new OkHttpClient.Builder().eventListener(defaultListenerBuilder()

micrometer-observation/build.gradle

@@ -5,6 +5,8 @@ dependencies {
 
     optionalApi 'io.micrometer:context-propagation-api'
 
+    optionalApi 'io.opentelemetry:opentelemetry-semconv'
+
     // HttpServlet KeyValueProvider
     optionalApi 'javax.servlet:javax.servlet-api'
 

micrometer-observation/src/main/java/io/micrometer/observation/Observation.java

@@ -659,6 +659,16 @@ interface KeyValuesProviderAware<T extends KeyValuesProvider<?>> {
 
     }
 
+    /**
+     * A marker interface for conventions of {@link KeyValues} naming.
+     *
+     * @author Marcin Grzejszczak
+     * @since 1.10.0
+     */
+    interface KeyValuesConvention {
+
+    }
+
     /**
      * A provider of key values.
      *