Micrometer Observations (#1760)

* WIP on Micrometer Observations * Added verification that metrics are measured * Fixed formatting * Fixed wrong status code method call * Converted to using around * Fixed compilation issues * prepare release 11.10 * [ci skip] updating versions to next development iteration 11.11-SNAPSHOT * Preparing for next development version * build(deps): bump json from 20220320 to 20220924 (#1768) Bumps [json](https://github.com/douglascrockford/JSON-java) from 20220320 to 20220924. - [Release notes](https://github.com/douglascrockford/JSON-java/releases) - [Changelog](https://github.com/stleary/JSON-java/blob/master/docs/RELEASES.md) - [Commits](https://github.com/douglascrockford/JSON-java/commits) --- updated-dependencies: - dependency-name: org.json:json dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Updated to latest micrometer changes * Enriches via clientInterceptors * Fixed the error in the DEFAULT instance * Reverts enriching of CLientInterceptor to achieve observability * build(deps): bump slf4j.version from 2.0.2 to 2.0.3 (#1769) Bumps `slf4j.version` from 2.0.2 to 2.0.3. Updates `slf4j-simple` from 2.0.2 to 2.0.3 - [Release notes](https://github.com/qos-ch/slf4j/releases) - [Commits](qos-ch/slf4j@v_2.0.2...v_2.0.3) Updates `slf4j-nop` from 2.0.2 to 2.0.3 - [Release notes](https://github.com/qos-ch/slf4j/releases) - [Commits](qos-ch/slf4j@v_2.0.2...v_2.0.3) Updates `slf4j-api` from 2.0.2 to 2.0.3 - [Release notes](https://github.com/qos-ch/slf4j/releases) - [Commits](qos-ch/slf4j@v_2.0.2...v_2.0.3) Updates `slf4j-jdk14` from 2.0.2 to 2.0.3 - [Release notes](https://github.com/qos-ch/slf4j/releases) - [Commits](qos-ch/slf4j@v_2.0.2...v_2.0.3) --- updated-dependencies: - dependency-name: org.slf4j:slf4j-simple dependency-type: direct:development update-type: version-update:semver-patch - dependency-name: org.slf4j:slf4j-nop dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: org.slf4j:slf4j-api dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: org.slf4j:slf4j-jdk14 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump kotlin.version from 1.7.10 to 1.7.20 (#1771) Bumps `kotlin.version` from 1.7.10 to 1.7.20. Updates `kotlin-stdlib-jdk8` from 1.7.10 to 1.7.20 - [Release notes](https://github.com/JetBrains/kotlin/releases) - [Changelog](https://github.com/JetBrains/kotlin/blob/v1.7.20/ChangeLog.md) - [Commits](JetBrains/kotlin@v1.7.10...v1.7.20) Updates `kotlin-reflect` from 1.7.10 to 1.7.20 - [Release notes](https://github.com/JetBrains/kotlin/releases) - [Changelog](https://github.com/JetBrains/kotlin/blob/v1.7.20/ChangeLog.md) - [Commits](JetBrains/kotlin@v1.7.10...v1.7.20) Updates `kotlin-maven-plugin` from 1.7.10 to 1.7.20 --- updated-dependencies: - dependency-name: org.jetbrains.kotlin:kotlin-stdlib-jdk8 dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: org.jetbrains.kotlin:kotlin-reflect dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: org.jetbrains.kotlin:kotlin-maven-plugin dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * build(deps): bump asm from 9.3 to 9.4 (#1777) Bumps asm from 9.3 to 9.4. --- updated-dependencies: - dependency-name: org.ow2.asm:asm dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Applied latest changes of Micrometer * Polish * Upgraded Micrometer to 1.10.0' * Alternative micrometer observation using capability * Ban 'repositories' * Applied my own review suggestions ;) * Polish Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Marvin Froeder <marvin.froeder@dovetailstudios.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Marvin Froeder <marvin.froeder@police.govt.nz> Co-authored-by: Marvin Froeder <velo@users.noreply.github.com>
OpenFeign · Nov 14, 2022 · 72f379a · 72f379a
1 parent 1ce671a
commit 72f379a
Show file tree

Hide file tree

Showing 14 changed files with 614 additions and 45 deletions.
diff --git a/core/pom.xml b/core/pom.xml
@@ -86,7 +86,6 @@
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-enforcer-plugin</artifactId>
-        <version>3.1.0</version>
         <executions>
           <execution>
             <id>enforce-banned-dependencies</id>

diff --git a/core/src/main/java/feign/Request.java b/core/src/main/java/feign/Request.java
@@ -13,16 +13,17 @@
  */
 package feign;
 
+import static feign.Util.checkNotNull;
+import static feign.Util.valuesOrEmpty;
 import java.io.Serializable;
 import java.net.HttpURLConnection;
 import java.nio.charset.Charset;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.TimeUnit;
-import static feign.Util.checkNotNull;
-import static feign.Util.valuesOrEmpty;
 
 /**
  * An immutable request to an http server.
@@ -206,6 +207,26 @@ public Map<String, Collection<String>> headers() {
     return Collections.unmodifiableMap(headers);
   }
 
+  /**
+   * Add new entries to request Headers. It overrides existing entries
+   *
+   * @param key
+   * @param value
+   */
+  public void header(String key, String value) {
+    header(key, Arrays.asList(value));
+  }
+
+  /**
+   * Add new entries to request Headers. It overrides existing entries
+   *
+   * @param key
+   * @param values
+   */
+  public void header(String key, Collection<String> values) {
+    headers.put(key, values);
+  }
+
   /**
    * Charset of the request.
    *

diff --git a/core/src/main/java/feign/RequestTemplate.java b/core/src/main/java/feign/RequestTemplate.java
@@ -13,18 +13,32 @@
  */
 package feign;
 
+import static feign.Util.CONTENT_LENGTH;
+import static feign.Util.checkNotNull;
 import feign.Request.HttpMethod;
-import feign.template.*;
+import feign.template.BodyTemplate;
+import feign.template.HeaderTemplate;
+import feign.template.QueryTemplate;
+import feign.template.UriTemplate;
+import feign.template.UriUtils;
 import java.io.Serializable;
 import java.net.URI;
 import java.nio.charset.Charset;
 import java.util.AbstractMap.SimpleImmutableEntry;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
 import java.util.Map.Entry;
+import java.util.TreeMap;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
-import static feign.Util.*;
 
 /**
  * Request Builder for an HTTP Target.
@@ -764,12 +778,10 @@ private RequestTemplate appendHeader(String name, Iterable<String> values, boole
         } else {
           return HeaderTemplate.create(headerName, values);
         }
+      } else if (literal) {
+        return HeaderTemplate.appendLiteral(headerTemplate, values);
       } else {
-        if (literal) {
-          return HeaderTemplate.appendLiteral(headerTemplate, values);
-        } else {
-          return HeaderTemplate.append(headerTemplate, values);
-        }
+        return HeaderTemplate.append(headerTemplate, values);
       }
     });
     return this;
@@ -791,7 +803,7 @@ public RequestTemplate headers(Map<String, Collection<String>> headers) {
   }
 
   /**
-   * Returns an immutable copy of the Headers for this request.
+   * Returns an copy of the Headers for this request.
    *
    * @return the currently applied headers.
    */
@@ -802,10 +814,10 @@ public Map<String, Collection<String>> headers() {
 
       /* add the expanded collection, but only if it has values */
       if (!values.isEmpty()) {
-        headerMap.put(key, Collections.unmodifiableList(values));
+        headerMap.put(key, values);
       }
     });
-    return Collections.unmodifiableMap(headerMap);
+    return headerMap;
   }
 
   /**

diff --git a/core/src/test/java/feign/RequestTemplateTest.java b/core/src/test/java/feign/RequestTemplateTest.java
@@ -15,10 +15,13 @@
 
 import static feign.assertj.FeignAssertions.assertThat;
 import static java.util.Arrays.asList;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.data.MapEntry.entry;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
+import feign.Request.HttpMethod;
+import feign.template.UriUtils;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
@@ -27,8 +30,6 @@
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
-import feign.Request.HttpMethod;
-import feign.template.UriUtils;
 
 public class RequestTemplateTest {
 
@@ -472,16 +473,25 @@ public void shouldRetrieveHeadersWithoutNull() {
 
   }
 
-  @SuppressWarnings("ConstantConditions")
-  @Test(expected = UnsupportedOperationException.class)
-  public void shouldNotInsertHeadersImmutableMap() {
+  public void shouldNotMutateInternalHeadersMap() {
     RequestTemplate template = new RequestTemplate()
         .header("key1", "valid");
 
     assertThat(template.headers()).hasSize(1);
     assertThat(template.headers().keySet()).containsExactly("key1");
+    assertThat(template.headers().get("key1")).containsExactly("valid");
 
     template.headers().put("key2", Collections.singletonList("other value"));
+    // nothing should change
+    assertThat(template.headers()).hasSize(1);
+    assertThat(template.headers().keySet()).containsExactly("key1");
+    assertThat(template.headers().get("key1")).containsExactly("valid");
+
+    template.headers().get("key1").add("value2");
+    // nothing should change either
+    assertThat(template.headers()).hasSize(1);
+    assertThat(template.headers().keySet()).containsExactly("key1");
+    assertThat(template.headers().get("key1")).containsExactly("valid");
   }
 
   @Test

diff --git a/micrometer/pom.xml b/micrometer/pom.xml
@@ -27,6 +27,7 @@
 
   <properties>
     <main.basedir>${project.basedir}/..</main.basedir>
+    <micrometer.version>1.10.0</micrometer.version>
   </properties>
 
   <dependencies>
@@ -42,19 +43,31 @@
     <dependency>
       <groupId>io.micrometer</groupId>
       <artifactId>micrometer-core</artifactId>
-      <version>1.10.0</version>
+      <version>${micrometer.version}</version>
     </dependency>
     <dependency>
       <groupId>org.mockito</groupId>
       <artifactId>mockito-core</artifactId>
       <version>${mockito.version}</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>io.micrometer</groupId>
+      <artifactId>micrometer-test</artifactId>
+      <version>${micrometer.version}</version>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.hamcrest</groupId>
       <artifactId>hamcrest</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>${project.groupId}</groupId>
+      <artifactId>feign-okhttp</artifactId>
+      <version>${project.version}</version>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 
   <build>
@@ -71,5 +84,4 @@
       </plugin>
     </plugins>
   </build>
-
 </project>
diff --git a/micrometer/src/main/java/feign/micrometer/DefaultFeignObservationConvention.java b/micrometer/src/main/java/feign/micrometer/DefaultFeignObservationConvention.java
@@ -0,0 +1,72 @@
+/*
+ * Copyright 2012-2022 The Feign Authors
+ *
+ * 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
+ *
+ * http://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 feign.micrometer;
+
+import feign.Request;
+import feign.Response;
+import io.micrometer.common.KeyValues;
+import io.micrometer.common.lang.Nullable;
+
+/**
+ * Default implementation of {@link FeignObservationConvention}.
+ *
+ * @since 12.1
+ * @see FeignObservationConvention
+ */
+public class DefaultFeignObservationConvention implements FeignObservationConvention {
+
+  /**
+   * Singleton instance of this convention.
+   */
+  public static final DefaultFeignObservationConvention INSTANCE =
+      new DefaultFeignObservationConvention();
+
+  // There is no need to instantiate this class multiple times, but it may be extended,
+  // hence protected visibility.
+  protected DefaultFeignObservationConvention() {}
+
+  @Override
+  public String getName() {
+    return "http.client.requests";
+  }
+
+  @Override
+  public String getContextualName(FeignContext context) {
+    return "HTTP " + getMethodString(context.getCarrier());
+  }
+
+  @Override
+  public KeyValues getLowCardinalityKeyValues(FeignContext context) {
+    String templatedUrl = context.getCarrier().requestTemplate().methodMetadata().template().url();
+    return KeyValues.of(
+        FeignObservationDocumentation.HttpClientTags.METHOD
+            .withValue(getMethodString(context.getCarrier())),
+        FeignObservationDocumentation.HttpClientTags.URI
+            .withValue(templatedUrl),
+        FeignObservationDocumentation.HttpClientTags.STATUS
+            .withValue(getStatusValue(context.getResponse())));
+  }
+
+  String getStatusValue(@Nullable Response response) {
+    return response != null ? String.valueOf(response.status()) : "CLIENT_ERROR";
+  }
+
+  String getMethodString(@Nullable Request request) {
+    if (request == null) {
+      return "UNKNOWN";
+    }
+    return request.httpMethod().name();
+  }
+
+}
diff --git a/micrometer/src/main/java/feign/micrometer/FeignContext.java b/micrometer/src/main/java/feign/micrometer/FeignContext.java
@@ -0,0 +1,34 @@
+/*
+ * Copyright 2012-2022 The Feign Authors
+ *
+ * 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
+ *
+ * http://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 feign.micrometer;
+
+import feign.Request;
+import feign.Response;
+import io.micrometer.observation.transport.RequestReplySenderContext;
+import io.micrometer.observation.transport.SenderContext;
+
+/**
+ * A {@link SenderContext} for Feign.
+ *
+ * @author Marcin Grzejszczak
+ * @since 12.1
+ */
+public class FeignContext extends RequestReplySenderContext<Request, Response> {
+
+  public FeignContext(Request request) {
+    super(Request::header);
+    setCarrier(request);
+  }
+
+}
diff --git a/micrometer/src/main/java/feign/micrometer/FeignObservationConvention.java b/micrometer/src/main/java/feign/micrometer/FeignObservationConvention.java
@@ -0,0 +1,31 @@
+/*
+ * Copyright 2012-2022 The Feign Authors
+ *
+ * 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
+ *
+ * http://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 feign.micrometer;
+
+import io.micrometer.observation.Observation;
+import io.micrometer.observation.ObservationConvention;
+
+/**
+ * {@link ObservationConvention} for Feign.
+ *
+ * @since 12.1
+ */
+public interface FeignObservationConvention extends ObservationConvention<FeignContext> {
+
+  @Override
+  default boolean supportsContext(Observation.Context context) {
+    return context instanceof FeignContext;
+  }
+
+}