Micrometer observations

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>

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.
    *

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;
   }
 
   /**

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

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>

micrometer/src/main/java/feign/micrometer/DefaultFeignObservationConvention.java

@@ -0,0 +1,69 @@
+/*
+ * 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 1.10.0
+ * @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) {
+    return request.httpMethod().name();
+  }
+
+}

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 1.10.0
+ */
+public class FeignContext extends RequestReplySenderContext<Request, Response> {
+
+  public FeignContext(Request request) {
+    super(Request::header);
+    setCarrier(request);
+  }
+
+}

micrometer/src/main/java/feign/micrometer/FeignObservationConvention.java

@@ -0,0 +1,33 @@
+/*
+ * 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.core.instrument.binder.httpcomponents.DefaultApacheHttpClientObservationConvention;
+import io.micrometer.observation.Observation;
+import io.micrometer.observation.ObservationConvention;
+
+/**
+ * {@link ObservationConvention} for Feign.
+ *
+ * @since 1.10.0
+ * @see DefaultApacheHttpClientObservationConvention
+ */
+public interface FeignObservationConvention extends ObservationConvention<FeignContext> {
+
+  @Override
+  default boolean supportsContext(Observation.Context context) {
+    return context instanceof FeignContext;
+  }
+
+}