diff --git a/core/pom.xml b/core/pom.xml index f32245e5..de3bb11c 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -86,7 +86,6 @@ org.apache.maven.plugins maven-enforcer-plugin - 3.1.0 enforce-banned-dependencies diff --git a/core/src/main/java/feign/Request.java b/core/src/main/java/feign/Request.java index a974e467..31ae5b8d 100644 --- 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 final class Request implements Serializable { 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 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 index 9014430f..20bbb8b6 100644 --- 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 @@ public final class RequestTemplate implements Serializable { } 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 final class RequestTemplate implements Serializable { } /** - * 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 final class RequestTemplate implements Serializable { /* 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 index dbfccdd3..1becf347 100644 --- a/core/src/test/java/feign/RequestTemplateTest.java +++ b/core/src/test/java/feign/RequestTemplateTest.java @@ -15,10 +15,13 @@ package feign; 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 java.util.Map; 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 class RequestTemplateTest { } - @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 index a2e670b1..66aceda7 100644 --- a/micrometer/pom.xml +++ b/micrometer/pom.xml @@ -27,6 +27,7 @@ ${project.basedir}/.. + 1.10.0 @@ -42,7 +43,7 @@ io.micrometer micrometer-core - 1.10.0 + ${micrometer.version} org.mockito @@ -50,11 +51,23 @@ ${mockito.version} test + + io.micrometer + micrometer-test + ${micrometer.version} + test + org.hamcrest hamcrest test + + ${project.groupId} + feign-okhttp + ${project.version} + test + @@ -71,5 +84,4 @@ - diff --git a/micrometer/src/main/java/feign/micrometer/DefaultFeignObservationConvention.java b/micrometer/src/main/java/feign/micrometer/DefaultFeignObservationConvention.java new file mode 100644 index 00000000..ac000c8d --- /dev/null +++ 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 new file mode 100644 index 00000000..1b1b3f9a --- /dev/null +++ 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 { + + 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 new file mode 100644 index 00000000..1618e0df --- /dev/null +++ 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 { + + @Override + default boolean supportsContext(Observation.Context context) { + return context instanceof FeignContext; + } + +} diff --git a/micrometer/src/main/java/feign/micrometer/FeignObservationDocumentation.java b/micrometer/src/main/java/feign/micrometer/FeignObservationDocumentation.java new file mode 100644 index 00000000..74f1b861 --- /dev/null +++ b/micrometer/src/main/java/feign/micrometer/FeignObservationDocumentation.java @@ -0,0 +1,81 @@ +/* + * 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.common.docs.KeyName; +import io.micrometer.observation.Observation; +import io.micrometer.observation.ObservationConvention; +import io.micrometer.observation.docs.ObservationDocumentation; + +/** + * {@link ObservationDocumentation} for Feign. + * + * @since 12.1 + */ +public enum FeignObservationDocumentation implements ObservationDocumentation { + + DEFAULT { + @Override + public Class> getDefaultConvention() { + return DefaultFeignObservationConvention.class; + } + + @Override + public KeyName[] getLowCardinalityKeyNames() { + return HttpClientTags.values(); + } + }; + + enum HttpClientTags implements KeyName { + + STATUS { + @Override + public String asString() { + return "http.status_code"; + } + }, + METHOD { + @Override + public String asString() { + return "http.method"; + } + }, + URI { + @Override + public String asString() { + return "http.url"; + } + }, + TARGET_SCHEME { + @Override + public String asString() { + return "http.scheme"; + } + }, + TARGET_HOST { + @Override + public String asString() { + return "net.peer.host"; + } + }, + TARGET_PORT { + @Override + public String asString() { + return "net.peer.port"; + } + } + + } + +} diff --git a/micrometer/src/main/java/feign/micrometer/MicrometerObservationCapability.java b/micrometer/src/main/java/feign/micrometer/MicrometerObservationCapability.java new file mode 100644 index 00000000..92769159 --- /dev/null +++ b/micrometer/src/main/java/feign/micrometer/MicrometerObservationCapability.java @@ -0,0 +1,95 @@ +/* + * 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.AsyncClient; +import feign.Capability; +import feign.Client; +import feign.FeignException; +import feign.Response; +import io.micrometer.observation.Observation; +import io.micrometer.observation.ObservationRegistry; + +/** Warp feign {@link Client} with metrics. */ +public class MicrometerObservationCapability implements Capability { + + private final ObservationRegistry observationRegistry; + + private final FeignObservationConvention customFeignObservationConvention; + + public MicrometerObservationCapability(ObservationRegistry observationRegistry, + FeignObservationConvention customFeignObservationConvention) { + this.observationRegistry = observationRegistry; + this.customFeignObservationConvention = customFeignObservationConvention; + } + + public MicrometerObservationCapability(ObservationRegistry observationRegistry) { + this(observationRegistry, null); + } + + @Override + public Client enrich(Client client) { + return (request, options) -> { + FeignContext feignContext = new FeignContext(request); + + Observation observation = FeignObservationDocumentation.DEFAULT + .observation(this.customFeignObservationConvention, + DefaultFeignObservationConvention.INSTANCE, () -> feignContext, + this.observationRegistry) + .start(); + + try { + Response response = client.execute(request, options); + finalizeObservation(feignContext, observation, null, response); + return response; + } catch (FeignException ex) { + finalizeObservation(feignContext, observation, ex, null); + throw ex; + } + }; + } + + @Override + public AsyncClient enrich(AsyncClient client) { + return (request, options, context) -> { + FeignContext feignContext = new FeignContext(request); + + Observation observation = FeignObservationDocumentation.DEFAULT + .observation(this.customFeignObservationConvention, + DefaultFeignObservationConvention.INSTANCE, () -> feignContext, + this.observationRegistry) + .start(); + + try { + return client.execute(feignContext.getCarrier(), options, context) + .whenComplete((r, ex) -> finalizeObservation(feignContext, observation, ex, r)); + } catch (FeignException ex) { + finalizeObservation(feignContext, observation, ex, null); + + throw ex; + } + }; + } + + private void finalizeObservation(FeignContext feignContext, + Observation observation, + Throwable ex, + Response response) { + feignContext.setResponse(response); + if (ex != null) { + observation.error(ex); + } + observation.stop(); + } +} diff --git a/micrometer/src/test/java/feign/micrometer/AbstractMetricsTestBase.java b/micrometer/src/test/java/feign/micrometer/AbstractMetricsTestBase.java index a109c829..c515ddca 100644 --- a/micrometer/src/test/java/feign/micrometer/AbstractMetricsTestBase.java +++ b/micrometer/src/test/java/feign/micrometer/AbstractMetricsTestBase.java @@ -63,10 +63,10 @@ public abstract class AbstractMetricsTestBase { @Test public final void addMetricsCapability() { final SimpleSource source = - Feign.builder() + customizeBuilder(Feign.builder() .client(new MockClient().ok(HttpMethod.GET, "/get", "1234567890abcde")) - .addCapability(createMetricCapability()) - .target(new MockTarget<>(SimpleSource.class)); + .addCapability(createMetricCapability())) + .target(new MockTarget<>(SimpleSource.class)); source.get("0x3456789"); @@ -76,10 +76,10 @@ public abstract class AbstractMetricsTestBase { @Test public final void addAsyncMetricsCapability() { final CompletableSource source = - AsyncFeign.builder() + customizeBuilder(AsyncFeign.builder() .client(new MockClient().ok(HttpMethod.GET, "/get", "1234567890abcde")) - .addCapability(createMetricCapability()) - .target(new MockTarget<>(CompletableSource.class)); + .addCapability(createMetricCapability())) + .target(new MockTarget<>(CompletableSource.class)); source.get("0x3456789").join(); @@ -152,14 +152,14 @@ public abstract class AbstractMetricsTestBase { final AtomicReference notFound = new AtomicReference<>(); final SimpleSource source = - Feign.builder() + customizeBuilder(Feign.builder() .client( (request, options) -> { notFound.set(new FeignException.NotFound("test", request, null, null)); throw notFound.get(); }) - .addCapability(createMetricCapability()) - .target(new MockTarget<>(MicrometerCapabilityTest.SimpleSource.class)); + .addCapability(createMetricCapability())) + .target(new MockTarget<>(MicrometerCapabilityTest.SimpleSource.class)); try { source.get("0x3456789"); @@ -180,15 +180,15 @@ public abstract class AbstractMetricsTestBase { final AtomicReference notFound = new AtomicReference<>(); final SimpleSource source = - Feign.builder() + customizeBuilder(Feign.builder() .client(new MockClient().ok(HttpMethod.GET, "/get", "1234567890abcde")) .decoder( (response, type) -> { notFound.set(new FeignException.NotFound("test", response.request(), null, null)); throw notFound.get(); }) - .addCapability(createMetricCapability()) - .target(new MockTarget<>(MicrometerCapabilityTest.SimpleSource.class)); + .addCapability(createMetricCapability())) + .target(new MockTarget<>(MicrometerCapabilityTest.SimpleSource.class)); FeignException.NotFound thrown = assertThrows(FeignException.NotFound.class, () -> source.get("0x3456789")); @@ -198,13 +198,13 @@ public abstract class AbstractMetricsTestBase { @Test public void shouldMetricCollectionWithCustomException() { final SimpleSource source = - Feign.builder() + customizeBuilder(Feign.builder() .client( (request, options) -> { throw new RuntimeException("Test error"); }) - .addCapability(createMetricCapability()) - .target(new MockTarget<>(MicrometerCapabilityTest.SimpleSource.class)); + .addCapability(createMetricCapability())) + .target(new MockTarget<>(MicrometerCapabilityTest.SimpleSource.class)); RuntimeException thrown = assertThrows(RuntimeException.class, () -> source.get("0x3456789")); assertThat(thrown.getMessage(), equalTo("Test error")); @@ -217,10 +217,10 @@ public abstract class AbstractMetricsTestBase { @Test public void clientMetricsHaveUriLabel() { final SimpleSource source = - Feign.builder() + customizeBuilder(Feign.builder() .client(new MockClient().ok(HttpMethod.GET, "/get", "1234567890abcde")) - .addCapability(createMetricCapability()) - .target(new MockTarget<>(SimpleSource.class)); + .addCapability(createMetricCapability())) + .target(new MockTarget<>(SimpleSource.class)); source.get("0x3456789"); @@ -246,10 +246,10 @@ public abstract class AbstractMetricsTestBase { @Test public void clientMetricsHaveUriLabelWithPathExpression() { final SourceWithPathExpressions source = - Feign.builder() + customizeBuilder(Feign.builder() .client(new MockClient().ok(HttpMethod.GET, "/get/123", "1234567890abcde")) - .addCapability(createMetricCapability()) - .target(new MockTarget<>(SourceWithPathExpressions.class)); + .addCapability(createMetricCapability())) + .target(new MockTarget<>(SourceWithPathExpressions.class)); source.get("123", "0x3456789"); @@ -272,15 +272,15 @@ public abstract class AbstractMetricsTestBase { final AtomicReference notFound = new AtomicReference<>(); final SourceWithPathExpressions source = - Feign.builder() + customizeBuilder(Feign.builder() .client(new MockClient().ok(HttpMethod.GET, "/get/123", "1234567890abcde")) .decoder( (response, type) -> { notFound.set(new FeignException.NotFound("test", response.request(), null, null)); throw notFound.get(); }) - .addCapability(createMetricCapability()) - .target(new MockTarget<>(MicrometerCapabilityTest.SourceWithPathExpressions.class)); + .addCapability(createMetricCapability())) + .target(new MockTarget<>(MicrometerCapabilityTest.SourceWithPathExpressions.class)); FeignException.NotFound thrown = assertThrows(FeignException.NotFound.class, () -> source.get("123", "0x3456789")); @@ -311,4 +311,12 @@ public abstract class AbstractMetricsTestBase { protected abstract boolean doesMetricHasCounter(METRIC metric); protected abstract long getMetricCounter(METRIC metric); + + protected Feign.Builder customizeBuilder(Feign.Builder builder) { + return builder; + } + + protected AsyncFeign.AsyncBuilder customizeBuilder(AsyncFeign.AsyncBuilder builder) { + return builder; + } } diff --git a/micrometer/src/test/java/feign/micrometer/FeignHeaderInstrumentationTest.java b/micrometer/src/test/java/feign/micrometer/FeignHeaderInstrumentationTest.java new file mode 100644 index 00000000..d19f88e1 --- /dev/null +++ b/micrometer/src/test/java/feign/micrometer/FeignHeaderInstrumentationTest.java @@ -0,0 +1,131 @@ +/* + * 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 static com.github.tomakehurst.wiremock.client.WireMock.anyUrl; +import static com.github.tomakehurst.wiremock.client.WireMock.equalTo; +import static com.github.tomakehurst.wiremock.client.WireMock.get; +import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor; +import static com.github.tomakehurst.wiremock.client.WireMock.ok; +import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; +import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; +import static com.github.tomakehurst.wiremock.client.WireMock.verify; +import static org.assertj.core.api.Assertions.assertThat; +import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo; +import com.github.tomakehurst.wiremock.junit5.WireMockTest; +import feign.AsyncFeign; +import feign.Feign; +import feign.Param; +import feign.Request; +import feign.RequestLine; +import feign.Response; +import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.Timer; +import io.micrometer.core.instrument.observation.DefaultMeterObservationHandler; +import io.micrometer.core.instrument.simple.SimpleMeterRegistry; +import io.micrometer.observation.Observation; +import io.micrometer.observation.ObservationHandler; +import io.micrometer.observation.ObservationRegistry; +import io.micrometer.observation.transport.RequestReplySenderContext; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import okhttp3.OkHttpClient; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +@WireMockTest +class FeignHeaderInstrumentationTest { + + MeterRegistry meterRegistry = new SimpleMeterRegistry(); + + ObservationRegistry observationRegistry = ObservationRegistry.create(); + + @BeforeEach + void setup() { + observationRegistry.observationConfig() + .observationHandler(new DefaultMeterObservationHandler(meterRegistry)); + observationRegistry.observationConfig().observationHandler(new HeaderMutatingHandler()); + } + + @Test + void getTemplatedPathForUri(WireMockRuntimeInfo wmRuntimeInfo) { + stubFor(get(anyUrl()).willReturn(ok())); + + TestClient testClient = clientInstrumentedWithObservations(wmRuntimeInfo.getHttpBaseUrl()); + testClient.templated("1", "2"); + + verify(getRequestedFor(urlEqualTo("/customers/1/carts/2")).withHeader("foo", equalTo("bar"))); + Timer timer = meterRegistry.get("http.client.requests").timer(); + assertThat(timer.count()).isEqualTo(1); + assertThat(timer.totalTime(TimeUnit.NANOSECONDS)).isPositive(); + } + + @Test + void getTemplatedPathForUriForAsync(WireMockRuntimeInfo wmRuntimeInfo) + throws ExecutionException, InterruptedException { + stubFor(get(anyUrl()).willReturn(ok())); + + AsyncTestClient testClient = + asyncClientInstrumentedWithObservations(wmRuntimeInfo.getHttpBaseUrl()); + testClient.templated("1", "2").get(); + + verify(getRequestedFor(urlEqualTo("/customers/1/carts/2")).withHeader("foo", equalTo("bar"))); + Timer timer = meterRegistry.get("http.client.requests").timer(); + assertThat(timer.count()).isEqualTo(1); + assertThat(timer.totalTime(TimeUnit.NANOSECONDS)).isPositive(); + } + + private TestClient clientInstrumentedWithObservations(String url) { + return Feign.builder() + .client(new feign.okhttp.OkHttpClient(new OkHttpClient())) + .addCapability(new MicrometerObservationCapability(this.observationRegistry)) + .target(TestClient.class, url); + } + + private AsyncTestClient asyncClientInstrumentedWithObservations(String url) { + return AsyncFeign.builder() + .client(new feign.okhttp.OkHttpClient(new OkHttpClient())) + .addCapability(new MicrometerObservationCapability(this.observationRegistry)) + .target(AsyncTestClient.class, url); + } + + public interface TestClient { + + @RequestLine("GET /customers/{customerId}/carts/{cartId}") + String templated(@Param("customerId") String customerId, @Param("cartId") String cartId); + } + + public interface AsyncTestClient { + + @RequestLine("GET /customers/{customerId}/carts/{cartId}") + CompletableFuture templated(@Param("customerId") String customerId, + @Param("cartId") String cartId); + } + + static class HeaderMutatingHandler + implements ObservationHandler> { + + @Override + public void onStart(RequestReplySenderContext context) { + Request carrier = context.getCarrier(); + carrier.header("foo", "bar"); + } + + @Override + public boolean supportsContext(Observation.Context context) { + return context instanceof FeignContext; + } + } +} diff --git a/micrometer/src/test/java/feign/micrometer/MicrometerObservationRegistryCapabilityTest.java b/micrometer/src/test/java/feign/micrometer/MicrometerObservationRegistryCapabilityTest.java new file mode 100644 index 00000000..7170f26b --- /dev/null +++ b/micrometer/src/test/java/feign/micrometer/MicrometerObservationRegistryCapabilityTest.java @@ -0,0 +1,43 @@ +/* + * 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.AsyncFeign; +import feign.Feign; +import io.micrometer.core.instrument.observation.DefaultMeterObservationHandler; +import io.micrometer.observation.ObservationRegistry; +import org.junit.Before; + +public class MicrometerObservationRegistryCapabilityTest extends MicrometerCapabilityTest { + + ObservationRegistry observationRegistry = ObservationRegistry.create(); + + @Before + public void setupRegistry() { + observationRegistry.observationConfig() + .observationHandler(new DefaultMeterObservationHandler(metricsRegistry)); + } + + @Override + protected Feign.Builder customizeBuilder(Feign.Builder builder) { + return super.customizeBuilder(builder) + .addCapability(new MicrometerObservationCapability(this.observationRegistry)); + } + + @Override + protected AsyncFeign.AsyncBuilder customizeBuilder(AsyncFeign.AsyncBuilder builder) { + return super.customizeBuilder(builder) + .addCapability(new MicrometerObservationCapability(this.observationRegistry)); + } +} diff --git a/pom.xml b/pom.xml index 2a339f9b..f8f12558 100644 --- a/pom.xml +++ b/pom.xml @@ -748,6 +748,26 @@ 15 + + org.apache.maven.plugins + maven-enforcer-plugin + 3.1.0 + + + enforce-no-repositories + + enforce + + + + + Feign should only depend on artifacts readly available on maven central + + + + + +