New approach to naming conventions

with this change we're merging semantic name provider and key values provider into a single ObservationConvention construct
users can register their conventions in the observationregistry configuration and then in the instrumentations ask for a registered convention (and provide a default if one is not registered)

Author: marcingrzejszczak

micrometer-commons/src/main/java/io/micrometer/common/docs/SemanticNameProvider.java

@@ -38,7 +38,7 @@
 
 @NonNullApi
 @NonNullFields
-public class DefaultOkHttpKeyValuesProvider implements OkHttpKeyValuesProvider {
+public class DefaultOkHttpObservationConvention implements OkHttpObservationConvention {
 
     static final boolean REQUEST_TAG_CLASS_EXISTS;
 
@@ -67,6 +67,12 @@ 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);
 
+    private final String metricName;
+
+    public DefaultOkHttpObservationConvention(String metricName) {
+        this.metricName = metricName;
+    }
+
     @Override
     public KeyValues getLowCardinalityKeyValues(OkHttpContext context) {
         OkHttpMetricsEventListener.CallState state = context.getState();
@@ -143,4 +149,9 @@ private KeyValues generateTagsForRoute(@Nullable Request request) {
                 TAG_TARGET_PORT, Integer.toString(request.url().port()));
     }
 
+    @Override
+    public String getName() {
+        return this.metricName;
+    }
+
 }

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

@@ -31,8 +31,7 @@
 public enum OkHttpDocumentedObservation implements DocumentedObservation {
 
     /**
-     * Observation when using
-     * {@link ObservationRegistry.ObservationNamingConfiguration#DEFAULT} mode.
+     * Default observation for OK HTTP.
      */
     DEFAULT {
         @Override
@@ -51,29 +50,26 @@ public KeyName[] getLowCardinalityKeyNames() {
      * @param registry observation registry
      * @param okHttpContext the ok http context
      * @param requestsMetricName name of the observation
-     * @param keyValuesProvider key values provider. If {@code null} then the default
-     * provider will be used
+     * @param customConvention custom convention. If {@code null}, the
+     * {@link DefaultOkHttpObservationConvention} will be used.
      * @return a new {@link OkHttpDocumentedObservation}
      */
+    @SuppressWarnings("unchecked")
     static Observation of(@NonNull ObservationRegistry registry, @NonNull OkHttpContext okHttpContext,
             @NonNull String requestsMetricName,
-            @Nullable Observation.KeyValuesProvider<OkHttpContext> keyValuesProvider) {
-        Observation.KeyValuesProvider<?> provider = null;
+            @Nullable Observation.ObservationConvention<OkHttpContext> customConvention) {
+        Observation.ObservationConvention<OkHttpContext> convention = null;
         if (registry.isNoop()) {
-            provider = Observation.KeyValuesProvider.EMPTY;
+            convention = Observation.ObservationConvention.noOp();
         }
-        else if (keyValuesProvider != null) {
-            provider = keyValuesProvider;
-        }
-        else if (registry.observationConfig()
-                .getObservationNamingConfiguration() == ObservationRegistry.ObservationNamingConfiguration.DEFAULT) {
-            provider = new DefaultOkHttpKeyValuesProvider();
+        else if (customConvention != null) {
+            convention = customConvention;
         }
         else {
-            throw new IllegalStateException(
-                    "You've provided a STANDARDIZED naming configuration but haven't provided a key values provider");
+            convention = registry.observationConfig().getObservationConvention(okHttpContext,
+                    new DefaultOkHttpObservationConvention(requestsMetricName));
         }
-        return Observation.createNotStarted(requestsMetricName, okHttpContext, registry).keyValuesProvider(provider);
+        return Observation.createNotStarted(convention, okHttpContext, registry);
     }
 
     enum OkHttpLegacyLowCardinalityTags implements KeyName {

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

@@ -67,7 +67,7 @@ public class OkHttpMetricsEventListener extends EventListener {
 
     private final ObservationRegistry observationRegistry;
 
-    private final Observation.KeyValuesProvider<OkHttpContext> keyValuesProvider;
+    private final Observation.ObservationConvention<OkHttpContext> keyValuesProvider;
 
     private final String requestsMetricName;
 
@@ -91,12 +91,12 @@ public class OkHttpMetricsEventListener extends EventListener {
     protected OkHttpMetricsEventListener(MeterRegistry registry, String requestsMetricName,
             Function<Request, String> urlMapper, Iterable<Tag> extraTags,
             Iterable<BiFunction<Request, Response, Tag>> contextSpecificTags) {
-        this(registry, ObservationRegistry.NOOP, new DefaultOkHttpKeyValuesProvider(), requestsMetricName, urlMapper,
-                extraTags, contextSpecificTags, emptyList(), true);
+        this(registry, ObservationRegistry.NOOP, new DefaultOkHttpObservationConvention(requestsMetricName),
+                requestsMetricName, urlMapper, extraTags, contextSpecificTags, emptyList(), true);
     }
 
     OkHttpMetricsEventListener(MeterRegistry registry, ObservationRegistry observationRegistry,
-            Observation.KeyValuesProvider<OkHttpContext> keyValuesProvider, String requestsMetricName,
+            Observation.ObservationConvention<OkHttpContext> keyValuesProvider, String requestsMetricName,
             Function<Request, String> urlMapper, Iterable<Tag> extraTags,
             Iterable<BiFunction<Request, Response, Tag>> contextSpecificTags, Iterable<String> requestTagKeys,
             boolean includeHostTag) {
@@ -284,7 +284,7 @@ public static class Builder {
 
         private Iterable<String> requestTagKeys = Collections.emptyList();
 
-        private OkHttpKeyValuesProvider keyValuesProvider;
+        private OkHttpObservationConvention observationConvention;
 
         Builder(MeterRegistry registry, String name) {
             this.registry = registry;
@@ -301,8 +301,8 @@ public Builder observationRegistry(ObservationRegistry observationRegistry) {
             return this;
         }
 
-        public Builder keyValuesProvider(OkHttpKeyValuesProvider keyValuesProvider) {
-            this.keyValuesProvider = keyValuesProvider;
+        public Builder observationConvention(OkHttpObservationConvention observationConvention) {
+            this.observationConvention = observationConvention;
             return this;
         }
 
@@ -379,7 +379,7 @@ public Builder requestTagKeys(Iterable<String> requestTagKeys) {
 
         @SuppressWarnings("unchecked")
         public OkHttpMetricsEventListener build() {
-            return new OkHttpMetricsEventListener(registry, observationRegistry, keyValuesProvider, name, uriMapper,
+            return new OkHttpMetricsEventListener(registry, observationRegistry, observationConvention, name, uriMapper,
                     tags, contextSpecificTags, requestTagKeys, includeHostTag);
         }
 

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

@@ -24,7 +24,7 @@
  * @author Marcin Grzejszczak
  * @since 1.10.0
  */
-public interface OkHttpKeyValuesProvider extends HttpKeyValueProvider<OkHttpContext> {
+public interface OkHttpObservationConvention extends Observation.ObservationConvention<OkHttpContext> {
 
     @Override
     default boolean supportsContext(Observation.Context context) {

micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpKeyValuesProvider.java renamed to micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpObservationConvention.java

@@ -110,69 +110,22 @@ void timeSuccessfulWithDefaultObservation(@WiremockResolver.Wiremock WireMockSer
     }
 
     @Test
-    void timeSuccessfulWithKeyValuesProvider(@WiremockResolver.Wiremock WireMockServer server) throws IOException {
-        ObservationRegistry observationRegistry = ObservationRegistry.create();
-        TestHandler testHandler = new TestHandler();
-        observationRegistry.observationConfig().observationHandler(testHandler);
-        observationRegistry.observationConfig().observationHandler(new TimerObservationHandler(registry));
-        client = new OkHttpClient.Builder()
-                .eventListener(defaultListenerBuilder().observationRegistry(observationRegistry)
-                        // Example of a custom key values provider
-                        .keyValuesProvider(new OkHttpKeyValuesProvider() {
-                            @Override
-                            public KeyValues getLowCardinalityKeyValues(OkHttpContext context) {
-                                return KeyValues.of("baz", "baz2");
-                            }
-                        }).build())
-                .build();
-        server.stubFor(any(anyUrl()));
-        Request request = new Request.Builder().url(server.baseUrl()).build();
-
-        client.newCall(request).execute().close();
-
-        assertThat(registry.get("okhttp.requests").tags("baz", "baz2").timer().count()).isEqualTo(1L);
-    }
-
-    @Test
-    void timeSuccessfulWithKeyValueConvention(@WiremockResolver.Wiremock WireMockServer server) throws IOException {
+    void timeSuccessfulWithObservationConvention(@WiremockResolver.Wiremock WireMockServer server) throws IOException {
         ObservationRegistry observationRegistry = ObservationRegistry.create();
         TestHandler testHandler = new TestHandler();
         MyConvention myConvention = new MyConvention();
-        observationRegistry.observationConfig()
-                .namingConfiguration(ObservationRegistry.ObservationNamingConfiguration.STANDARDIZED);
         observationRegistry.observationConfig().observationHandler(testHandler);
         observationRegistry.observationConfig().observationHandler(new TimerObservationHandler(registry));
         client = new OkHttpClient.Builder()
                 .eventListener(defaultListenerBuilder().observationRegistry(observationRegistry)
-                        // Example of a custom key values provider that uses the
-                        // conventions mechanism
-                        .keyValuesProvider(new StandardizedOkHttpKeyValuesProvider(myConvention)).build())
+                        .observationConvention(new StandardizedOkHttpObservationConvention(myConvention)).build())
                 .build();
         server.stubFor(any(anyUrl()));
         Request request = new Request.Builder().url(server.baseUrl()).build();
 
         client.newCall(request).execute().close();
 
-        assertThat(registry.get("okhttp.requests").tags("peer", "name").timer().count()).isEqualTo(1L);
-    }
-
-    @Test
-    void timeIllegalStateExceptionWhenStandardOptionOnAndNoKeyValuesProviderSet(
-            @WiremockResolver.Wiremock WireMockServer server) {
-        ObservationRegistry observationRegistry = ObservationRegistry.create();
-        // We're explicitly turning on the standard mode
-        observationRegistry.observationConfig()
-                .namingConfiguration(ObservationRegistry.ObservationNamingConfiguration.STANDARDIZED);
-        observationRegistry.observationConfig().observationHandler(new TimerObservationHandler(registry));
-        client = new OkHttpClient.Builder()
-                // We're NOT setting the key values provider
-                .eventListener(defaultListenerBuilder().observationRegistry(observationRegistry).build()).build();
-        server.stubFor(any(anyUrl()));
-        Request request = new Request.Builder().url(server.baseUrl()).build();
-
-        assertThatThrownBy(() -> client.newCall(request).execute().close()).isInstanceOf(IllegalStateException.class)
-                .hasMessageContaining(
-                        "You've provided a STANDARDIZED naming configuration but haven't provided a key values provider");
+        assertThat(registry.get("new.name").tags("peer", "name").timer().count()).isEqualTo(1L);
     }
 
     @Test
@@ -375,11 +328,11 @@ public boolean supportsContext(Observation.Context context) {
 
     }
 
-    static class StandardizedOkHttpKeyValuesProvider implements OkHttpKeyValuesProvider {
+    static class StandardizedOkHttpObservationConvention implements OkHttpObservationConvention {
 
         private final HttpClientKeyValuesConvention convention;
 
-        StandardizedOkHttpKeyValuesProvider(HttpClientKeyValuesConvention convention) {
+        StandardizedOkHttpObservationConvention(HttpClientKeyValuesConvention convention) {
             this.convention = convention;
         }
 
@@ -388,6 +341,11 @@ public KeyValues getLowCardinalityKeyValues(OkHttpContext context) {
             return KeyValues.of(convention.peerName(null));
         }
 
+        @Override
+        public String getName() {
+            return "new.name";
+        }
+
     }
 
     static class MyConvention implements HttpClientKeyValuesConvention {