From 212fdf26f98d63b9b4269bf981897a028926fd6c Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Mon, 19 Sep 2022 15:10:09 +0200 Subject: [PATCH 01/22] WIP on Micrometer Observations --- core/src/main/java/feign/AsyncFeign.java | 10 +++ core/src/main/java/feign/BaseBuilder.java | 24 +++++ core/src/main/java/feign/Capability.java | 8 ++ .../main/java/feign/ClientInterceptor.java | 53 +++++++++++ .../java/feign/ClientInvocationContext.java | 43 +++++++++ core/src/main/java/feign/Feign.java | 11 +++ .../java/feign/SynchronousMethodHandler.java | 26 +++++- core/src/test/java/feign/BaseBuilderTest.java | 4 +- example-github-with-coroutine/pom.xml | 2 +- micrometer/pom.xml | 33 ++++++- .../DefaultFeignObservationConvention.java | 70 +++++++++++++++ .../java/feign/micrometer/FeignContext.java | 34 +++++++ .../FeignDocumentedObservation.java | 81 +++++++++++++++++ .../FeignObservationConvention.java | 33 +++++++ .../micrometer/MicrometerCapability.java | 20 ++++- .../micrometer/ObservedClientInterceptor.java | 68 ++++++++++++++ .../FeignHeaderInstrumentationTest.java | 88 +++++++++++++++++++ ...eterObservationRegistryCapabilityTest.java | 30 +++++++ 18 files changed, 630 insertions(+), 8 deletions(-) create mode 100644 core/src/main/java/feign/ClientInterceptor.java create mode 100644 core/src/main/java/feign/ClientInvocationContext.java create mode 100644 micrometer/src/main/java/feign/micrometer/DefaultFeignObservationConvention.java create mode 100644 micrometer/src/main/java/feign/micrometer/FeignContext.java create mode 100644 micrometer/src/main/java/feign/micrometer/FeignDocumentedObservation.java create mode 100644 micrometer/src/main/java/feign/micrometer/FeignObservationConvention.java create mode 100644 micrometer/src/main/java/feign/micrometer/ObservedClientInterceptor.java create mode 100644 micrometer/src/test/java/feign/micrometer/FeignHeaderInstrumentationTest.java create mode 100644 micrometer/src/test/java/feign/micrometer/MicrometerObservationRegistryCapabilityTest.java diff --git a/core/src/main/java/feign/AsyncFeign.java b/core/src/main/java/feign/AsyncFeign.java index ea10b060f..048401dce 100644 --- a/core/src/main/java/feign/AsyncFeign.java +++ b/core/src/main/java/feign/AsyncFeign.java @@ -184,6 +184,16 @@ public AsyncBuilder requestInterceptors(Iterable requestI return super.requestInterceptors(requestInterceptors); } + @Override + public AsyncBuilder clientInterceptor(ClientInterceptor clientInterceptor) { + return super.clientInterceptor(clientInterceptor); + } + + @Override + public AsyncBuilder clientInterceptors(Iterable clientInterceptors) { + return super.clientInterceptors(clientInterceptors); + } + @Override public AsyncBuilder invocationHandlerFactory(InvocationHandlerFactory invocationHandlerFactory) { return super.invocationHandlerFactory(invocationHandlerFactory); diff --git a/core/src/main/java/feign/BaseBuilder.java b/core/src/main/java/feign/BaseBuilder.java index ae12d4f9d..2e81295f0 100644 --- a/core/src/main/java/feign/BaseBuilder.java +++ b/core/src/main/java/feign/BaseBuilder.java @@ -35,6 +35,8 @@ public abstract class BaseBuilder> { protected final List requestInterceptors = new ArrayList<>(); + protected final List clientInterceptors = + new ArrayList<>(); protected ResponseInterceptor responseInterceptor = ResponseInterceptor.DEFAULT; protected Logger.Level logLevel = Logger.Level.NONE; protected Contract contract = new Contract.Default(); @@ -197,6 +199,26 @@ public B requestInterceptors(Iterable requestInterceptors) { return thisB; } + /** + * Adds a single client interceptor to the builder. + */ + public B clientInterceptor(ClientInterceptor clientInterceptor) { + this.clientInterceptors.add(clientInterceptor); + return thisB; + } + + /** + * Sets the full set of request interceptors for the builder, overwriting any previous + * interceptors. + */ + public B clientInterceptors(Iterable clientInterceptors) { + this.clientInterceptors.clear(); + for (ClientInterceptor clientInterceptor : clientInterceptors) { + this.clientInterceptors.add(clientInterceptor); + } + return thisB; + } + /** * Adds a single response interceptor to the builder. */ @@ -229,6 +251,8 @@ protected B enrich() { return thisB; } + capabilities.forEach(capability -> capability.enrich(thisB)); + getFieldsToEnrich().forEach(field -> { field.setAccessible(true); try { diff --git a/core/src/main/java/feign/Capability.java b/core/src/main/java/feign/Capability.java index 0b71a9f08..82df29acf 100644 --- a/core/src/main/java/feign/Capability.java +++ b/core/src/main/java/feign/Capability.java @@ -86,6 +86,10 @@ default RequestInterceptor enrich(RequestInterceptor requestInterceptor) { return requestInterceptor; } + default ClientInterceptor enrich(ClientInterceptor clientInterceptor) { + return clientInterceptor; + } + default ResponseInterceptor enrich(ResponseInterceptor responseInterceptor) { return responseInterceptor; } @@ -137,4 +141,8 @@ default AsyncContextSupplier enrich(AsyncContextSupplier asyncContextS default MethodInfoResolver enrich(MethodInfoResolver methodInfoResolver) { return methodInfoResolver; } + + default > BaseBuilder enrich(BaseBuilder baseBuilder) { + return baseBuilder; + } } diff --git a/core/src/main/java/feign/ClientInterceptor.java b/core/src/main/java/feign/ClientInterceptor.java new file mode 100644 index 000000000..6087ade00 --- /dev/null +++ b/core/src/main/java/feign/ClientInterceptor.java @@ -0,0 +1,53 @@ +/* + * 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; + +/** + * Zero or One {@code ClientInterceptor} may be configured for purposes such as tracing - mutate + * headers, execute the call, parse the response and close any previously created objects. + */ +public interface ClientInterceptor { + + ClientInterceptor DEFAULT = new ClientInterceptor() { + @Override + public void beforeExecute(ClientInvocationContext context) {} + + @Override + public void afterExecute(ClientInvocationContext context, + Response response, + Throwable exception) { + + } + }; + + /** + * Called before the request was made. Can be used to mutate the request and create objects that + * later will be passed to {@link #afterExecute(ClientInvocationContext, Response, Throwable)}. + * + * @param clientInvocationContext information surrounding the request + */ + void beforeExecute(ClientInvocationContext clientInvocationContext); + + /** + * + * @param clientInvocationContext information surrounding the request + * @param response received undecoded response (can be null when an exception occurred) + * @param exception exception that occurred while trying to send the request or receive the + * response (can be null when there was no exception) + */ + void afterExecute(ClientInvocationContext clientInvocationContext, + Response response, + Throwable exception); + +} diff --git a/core/src/main/java/feign/ClientInvocationContext.java b/core/src/main/java/feign/ClientInvocationContext.java new file mode 100644 index 000000000..850626693 --- /dev/null +++ b/core/src/main/java/feign/ClientInvocationContext.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; + +import java.util.HashMap; +import java.util.Map; + +public class ClientInvocationContext { + + private final RequestTemplate requestTemplate; + + private final Request.Options options; + + private final Map holder = new HashMap<>(); + + public ClientInvocationContext(RequestTemplate requestTemplate, Request.Options options) { + this.requestTemplate = requestTemplate; + this.options = options; + } + + public RequestTemplate getRequestTemplate() { + return requestTemplate; + } + + public Request.Options getOptions() { + return options; + } + + public Map getHolder() { + return holder; + } +} diff --git a/core/src/main/java/feign/Feign.java b/core/src/main/java/feign/Feign.java index 1ecc262cd..2a93d830e 100644 --- a/core/src/main/java/feign/Feign.java +++ b/core/src/main/java/feign/Feign.java @@ -169,6 +169,16 @@ public Builder requestInterceptors(Iterable requestIntercept return super.requestInterceptors(requestInterceptors); } + @Override + public Builder clientInterceptor(ClientInterceptor clientInterceptor) { + return super.clientInterceptor(clientInterceptor); + } + + @Override + public Builder clientInterceptors(Iterable clientInterceptors) { + return super.clientInterceptors(clientInterceptors); + } + @Override public Builder invocationHandlerFactory(InvocationHandlerFactory invocationHandlerFactory) { return super.invocationHandlerFactory(invocationHandlerFactory); @@ -210,6 +220,7 @@ public Feign build() { SynchronousMethodHandler.Factory synchronousMethodHandlerFactory = new SynchronousMethodHandler.Factory(client, retryer, requestInterceptors, + clientInterceptors, responseInterceptor, logger, logLevel, dismiss404, closeAfterDecode, propagationPolicy, forceDecoding); ParseHandlersByName handlersByName = diff --git a/core/src/main/java/feign/SynchronousMethodHandler.java b/core/src/main/java/feign/SynchronousMethodHandler.java index b15f2dda7..4208a3417 100644 --- a/core/src/main/java/feign/SynchronousMethodHandler.java +++ b/core/src/main/java/feign/SynchronousMethodHandler.java @@ -36,6 +36,8 @@ final class SynchronousMethodHandler implements MethodHandler { private final Client client; private final Retryer retryer; private final List requestInterceptors; + + private final List clientInterceptors; private final ResponseInterceptor responseInterceptor; private final Logger logger; private final Logger.Level logLevel; @@ -49,7 +51,8 @@ final class SynchronousMethodHandler implements MethodHandler { private SynchronousMethodHandler(Target target, Client client, Retryer retryer, - List requestInterceptors, ResponseInterceptor responseInterceptor, + List requestInterceptors, List clientInterceptors, + ResponseInterceptor responseInterceptor, Logger logger, Logger.Level logLevel, MethodMetadata metadata, RequestTemplate.Factory buildTemplateFromArgs, Options options, Decoder decoder, ErrorDecoder errorDecoder, boolean dismiss404, @@ -61,6 +64,8 @@ private SynchronousMethodHandler(Target target, Client client, Retryer retrye this.retryer = checkNotNull(retryer, "retryer for %s", target); this.requestInterceptors = checkNotNull(requestInterceptors, "requestInterceptors for %s", target); + this.clientInterceptors = + checkNotNull(clientInterceptors, "clientInterceptors for %s", target); this.logger = checkNotNull(logger, "logger for %s", target); this.logLevel = checkNotNull(logLevel, "logLevel for %s", target); this.metadata = checkNotNull(metadata, "metadata for %s", target); @@ -109,13 +114,19 @@ public Object invoke(Object[] argv) throws Throwable { } Object executeAndDecode(RequestTemplate template, Options options) throws Throwable { + ClientInvocationContext context = new ClientInvocationContext(template, options); + for (ClientInterceptor interceptor : this.clientInterceptors) { + interceptor.beforeExecute(context); + } + Request request = targetRequest(template); if (logLevel != Logger.Level.NONE) { logger.logRequest(metadata.configKey(), logLevel, request); } - Response response; + Response response = null; + Throwable throwable = null; long start = System.nanoTime(); try { response = client.execute(request, options); @@ -125,10 +136,15 @@ Object executeAndDecode(RequestTemplate template, Options options) throws Throwa .requestTemplate(template) .build(); } catch (IOException e) { + throwable = e; if (logLevel != Logger.Level.NONE) { logger.logIOException(metadata.configKey(), logLevel, e, elapsedTime(start)); } throw errorExecuting(request, e); + } finally { + for (ClientInterceptor interceptor : this.clientInterceptors) { + interceptor.afterExecute(context, response, throwable); + } } long elapsedTime = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start); @@ -180,6 +196,8 @@ static class Factory { private final Client client; private final Retryer retryer; private final List requestInterceptors; + + private final List clientInterceptors; private final ResponseInterceptor responseInterceptor; private final Logger logger; private final Logger.Level logLevel; @@ -189,12 +207,13 @@ static class Factory { private final boolean forceDecoding; Factory(Client client, Retryer retryer, List requestInterceptors, - ResponseInterceptor responseInterceptor, + List clientInterceptors, ResponseInterceptor responseInterceptor, Logger logger, Logger.Level logLevel, boolean dismiss404, boolean closeAfterDecode, ExceptionPropagationPolicy propagationPolicy, boolean forceDecoding) { this.client = checkNotNull(client, "client"); this.retryer = checkNotNull(retryer, "retryer"); this.requestInterceptors = checkNotNull(requestInterceptors, "requestInterceptors"); + this.clientInterceptors = checkNotNull(clientInterceptors, "clientInterceptors"); this.responseInterceptor = responseInterceptor; this.logger = checkNotNull(logger, "logger"); this.logLevel = checkNotNull(logLevel, "logLevel"); @@ -211,6 +230,7 @@ public MethodHandler create(Target target, Decoder decoder, ErrorDecoder errorDecoder) { return new SynchronousMethodHandler(target, client, retryer, requestInterceptors, + clientInterceptors, responseInterceptor, logger, logLevel, md, buildTemplateFromArgs, options, decoder, errorDecoder, dismiss404, closeAfterDecode, propagationPolicy, forceDecoding); } diff --git a/core/src/test/java/feign/BaseBuilderTest.java b/core/src/test/java/feign/BaseBuilderTest.java index 9599fd98f..d071b8a19 100644 --- a/core/src/test/java/feign/BaseBuilderTest.java +++ b/core/src/test/java/feign/BaseBuilderTest.java @@ -27,7 +27,7 @@ public class BaseBuilderTest { public void checkEnrichTouchesAllAsyncBuilderFields() throws IllegalArgumentException, IllegalAccessException { test(AsyncFeign.builder().requestInterceptor(template -> { - }), 14); + }).clientInterceptor(ClientInterceptor.DEFAULT), 14); } private void test(BaseBuilder builder, int expectedFieldsCount) @@ -54,7 +54,7 @@ private void test(BaseBuilder builder, int expectedFieldsCount) public void checkEnrichTouchesAllBuilderFields() throws IllegalArgumentException, IllegalAccessException { test(Feign.builder().requestInterceptor(template -> { - }), 12); + }).clientInterceptor(ClientInterceptor.DEFAULT), 13); } } diff --git a/example-github-with-coroutine/pom.xml b/example-github-with-coroutine/pom.xml index 5b8f057f4..38d38d0c3 100644 --- a/example-github-with-coroutine/pom.xml +++ b/example-github-with-coroutine/pom.xml @@ -135,8 +135,8 @@ - kotlin-maven-plugin org.jetbrains.kotlin + kotlin-maven-plugin ${kotlin.version} diff --git a/micrometer/pom.xml b/micrometer/pom.xml index 3611d01bd..bb02c5cbb 100644 --- a/micrometer/pom.xml +++ b/micrometer/pom.xml @@ -27,6 +27,7 @@ ${project.basedir}/.. + 1.10.0-SNAPSHOT @@ -42,7 +43,7 @@ io.micrometer micrometer-core - 1.9.4 + ${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 + @@ -72,4 +85,22 @@ + + + spring-snapshots + Spring Snapshots + https://repo.spring.io/snapshot + + true + + + + spring-milestones + Spring Milestones + https://repo.spring.io/milestone + + false + + + 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 000000000..447b4824f --- /dev/null +++ b/micrometer/src/main/java/feign/micrometer/DefaultFeignObservationConvention.java @@ -0,0 +1,70 @@ +/* + * 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.RequestTemplate; +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().methodMetadata().template().url(); + return KeyValues.of( + FeignDocumentedObservation.HttpClientTags.METHOD + .withValue(getMethodString(context.getCarrier())), + FeignDocumentedObservation.HttpClientTags.URI + .withValue(templatedUrl), + FeignDocumentedObservation.HttpClientTags.STATUS + .withValue(getStatusValue(context.getResponse()))); + } + + String getStatusValue(@Nullable Response response) { + return response != null ? response.reason() : "CLIENT_ERROR"; + } + + String getMethodString(@Nullable RequestTemplate request) { + return request != null && request.method() != null ? request.method() + : "UNKNOWN"; + } + +} 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 000000000..a2a212925 --- /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.RequestTemplate; +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 { + + public FeignContext(RequestTemplate requestTemplate) { + super((carrier, key, value) -> carrier.header(key, value)); + setCarrier(requestTemplate); + } + +} diff --git a/micrometer/src/main/java/feign/micrometer/FeignDocumentedObservation.java b/micrometer/src/main/java/feign/micrometer/FeignDocumentedObservation.java new file mode 100644 index 000000000..1f52c94f3 --- /dev/null +++ b/micrometer/src/main/java/feign/micrometer/FeignDocumentedObservation.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.DocumentedObservation; + +/** + * {@link DocumentedObservation} for Feign. + * + * @since 1.10.0 + */ +public enum FeignDocumentedObservation implements DocumentedObservation { + + 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 "status"; + } + }, + METHOD { + @Override + public String asString() { + return "method"; + } + }, + URI { + @Override + public String asString() { + return "uri"; + } + }, + TARGET_SCHEME { + @Override + public String asString() { + return "target.scheme"; + } + }, + TARGET_HOST { + @Override + public String asString() { + return "target.host"; + } + }, + TARGET_PORT { + @Override + public String asString() { + return "target.port"; + } + } + + } + +} 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 000000000..aaf9f99f7 --- /dev/null +++ b/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 { + + @Override + default boolean supportsContext(Observation.Context context) { + return context instanceof FeignContext; + } + +} diff --git a/micrometer/src/main/java/feign/micrometer/MicrometerCapability.java b/micrometer/src/main/java/feign/micrometer/MicrometerCapability.java index e4f3d76a5..71d31cc24 100644 --- a/micrometer/src/main/java/feign/micrometer/MicrometerCapability.java +++ b/micrometer/src/main/java/feign/micrometer/MicrometerCapability.java @@ -14,6 +14,7 @@ package feign.micrometer; import feign.AsyncClient; +import feign.BaseBuilder; import feign.Capability; import feign.Client; import feign.InvocationHandlerFactory; @@ -24,18 +25,35 @@ import io.micrometer.core.instrument.Metrics; import io.micrometer.core.instrument.simple.SimpleConfig; import io.micrometer.core.instrument.simple.SimpleMeterRegistry; +import io.micrometer.observation.ObservationRegistry; public class MicrometerCapability implements Capability { private final MeterRegistry meterRegistry; + private final ObservationRegistry observationRegistry; + public MicrometerCapability() { - this(new SimpleMeterRegistry(SimpleConfig.DEFAULT, Clock.SYSTEM)); + this(new SimpleMeterRegistry(SimpleConfig.DEFAULT, Clock.SYSTEM), ObservationRegistry.NOOP); Metrics.addRegistry(meterRegistry); } public MicrometerCapability(MeterRegistry meterRegistry) { + this(meterRegistry, ObservationRegistry.NOOP); + } + + public MicrometerCapability(MeterRegistry meterRegistry, + ObservationRegistry observationRegistry) { this.meterRegistry = meterRegistry; + this.observationRegistry = observationRegistry; + } + + @Override + public > BaseBuilder enrich(BaseBuilder baseBuilder) { + if (!observationRegistry.isNoop()) { + baseBuilder.clientInterceptor(new ObservedClientInterceptor(observationRegistry)); + } + return baseBuilder; } @Override diff --git a/micrometer/src/main/java/feign/micrometer/ObservedClientInterceptor.java b/micrometer/src/main/java/feign/micrometer/ObservedClientInterceptor.java new file mode 100644 index 000000000..1fbe6e714 --- /dev/null +++ b/micrometer/src/main/java/feign/micrometer/ObservedClientInterceptor.java @@ -0,0 +1,68 @@ +/* + * 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.Client; +import feign.ClientInterceptor; +import feign.ClientInvocationContext; +import feign.Response; +import io.micrometer.observation.Observation; +import io.micrometer.observation.ObservationRegistry; + +/** Warp feign {@link Client} with metrics. */ +public class ObservedClientInterceptor implements ClientInterceptor { + + private final ObservationRegistry observationRegistry; + + private final FeignObservationConvention customFeignObservationConvention; + + public ObservedClientInterceptor(ObservationRegistry observationRegistry, + FeignObservationConvention customFeignObservationConvention) { + this.observationRegistry = observationRegistry; + this.customFeignObservationConvention = customFeignObservationConvention; + } + + public ObservedClientInterceptor(ObservationRegistry observationRegistry) { + this(observationRegistry, null); + } + + @Override + public void beforeExecute(ClientInvocationContext clientInvocationContext) { + FeignContext feignContext = new FeignContext(clientInvocationContext.getRequestTemplate()); + Observation observation = FeignDocumentedObservation.DEFAULT + .observation(this.customFeignObservationConvention, + DefaultFeignObservationConvention.INSTANCE, feignContext, this.observationRegistry) + .start(); + clientInvocationContext.getHolder().put(Observation.class, observation); + clientInvocationContext.getHolder().put(FeignContext.class, feignContext); + } + + @Override + public void afterExecute(ClientInvocationContext clientInvocationContext, + Response response, + Throwable exception) { + FeignContext feignContext = + (FeignContext) clientInvocationContext.getHolder().get(FeignContext.class); + Observation observation = + (Observation) clientInvocationContext.getHolder().get(Observation.class); + if (feignContext == null) { + return; + } + feignContext.setResponse(response); + if (exception != null) { + observation.error(exception); + } + observation.stop(); + } +} 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 000000000..143bbd0ca --- /dev/null +++ b/micrometer/src/test/java/feign/micrometer/FeignHeaderInstrumentationTest.java @@ -0,0 +1,88 @@ +/* + * 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 com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo; +import com.github.tomakehurst.wiremock.junit5.WireMockTest; +import feign.Feign; +import feign.Param; +import feign.RequestLine; +import feign.RequestTemplate; +import feign.Response; +import io.micrometer.core.instrument.MeterRegistry; +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 okhttp3.OkHttpClient; +import org.junit.jupiter.api.Test; +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; + +@WireMockTest +class FeignHeaderInstrumentationTest { + + @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"))); + } + + private TestClient clientInstrumentedWithObservations(String url) { + MeterRegistry meterRegistry = new SimpleMeterRegistry(); + ObservationRegistry observationRegistry = ObservationRegistry.create(); + + observationRegistry.observationConfig() + .observationHandler(new DefaultMeterObservationHandler(meterRegistry)); + observationRegistry.observationConfig().observationHandler(new HeaderMutatingHandler()); + + return Feign.builder() + .client(new feign.okhttp.OkHttpClient(new OkHttpClient())) + .addCapability(new MicrometerCapability(meterRegistry, observationRegistry)) + .target(TestClient.class, url); + } + + public interface TestClient { + + @RequestLine("GET /customers/{customerId}/carts/{cartId}") + String templated(@Param("customerId") String customerId, @Param("cartId") String cartId); + } + + static class HeaderMutatingHandler + implements ObservationHandler> { + + @Override + public void onStart(RequestReplySenderContext context) { + RequestTemplate 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 000000000..ef6503bad --- /dev/null +++ b/micrometer/src/test/java/feign/micrometer/MicrometerObservationRegistryCapabilityTest.java @@ -0,0 +1,30 @@ +/* + * 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.Capability; +import io.micrometer.core.instrument.observation.DefaultMeterObservationHandler; +import io.micrometer.observation.ObservationRegistry; + +public class MicrometerObservationRegistryCapabilityTest extends MicrometerCapabilityTest { + + ObservationRegistry observationRegistry = ObservationRegistry.create(); + + @Override + protected Capability createMetricCapability() { + observationRegistry.observationConfig() + .observationHandler(new DefaultMeterObservationHandler(metricsRegistry)); + return new MicrometerCapability(metricsRegistry, observationRegistry); + } +} From 5e8192cbb0f51e5509d0347cec3f7f65d0f93f12 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Mon, 19 Sep 2022 15:24:29 +0200 Subject: [PATCH 02/22] Added verification that metrics are measured --- .../FeignHeaderInstrumentationTest.java | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/micrometer/src/test/java/feign/micrometer/FeignHeaderInstrumentationTest.java b/micrometer/src/test/java/feign/micrometer/FeignHeaderInstrumentationTest.java index 143bbd0ca..a5d0941fe 100644 --- a/micrometer/src/test/java/feign/micrometer/FeignHeaderInstrumentationTest.java +++ b/micrometer/src/test/java/feign/micrometer/FeignHeaderInstrumentationTest.java @@ -13,6 +13,8 @@ */ package feign.micrometer; +import java.util.concurrent.TimeUnit; + import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo; import com.github.tomakehurst.wiremock.junit5.WireMockTest; import feign.Feign; @@ -21,6 +23,7 @@ import feign.RequestTemplate; 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; @@ -28,6 +31,7 @@ import io.micrometer.observation.ObservationRegistry; import io.micrometer.observation.transport.RequestReplySenderContext; import okhttp3.OkHttpClient; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static com.github.tomakehurst.wiremock.client.WireMock.anyUrl; import static com.github.tomakehurst.wiremock.client.WireMock.equalTo; @@ -37,10 +41,22 @@ 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; @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())); @@ -49,16 +65,12 @@ void getTemplatedPathForUri(WireMockRuntimeInfo wmRuntimeInfo) { 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(); } private TestClient clientInstrumentedWithObservations(String url) { - MeterRegistry meterRegistry = new SimpleMeterRegistry(); - ObservationRegistry observationRegistry = ObservationRegistry.create(); - - observationRegistry.observationConfig() - .observationHandler(new DefaultMeterObservationHandler(meterRegistry)); - observationRegistry.observationConfig().observationHandler(new HeaderMutatingHandler()); - return Feign.builder() .client(new feign.okhttp.OkHttpClient(new OkHttpClient())) .addCapability(new MicrometerCapability(meterRegistry, observationRegistry)) From e23a805b5b9ddd01cc3406ae4e6cb824283646b6 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Mon, 19 Sep 2022 15:41:13 +0200 Subject: [PATCH 03/22] Fixed formatting --- .../java/feign/micrometer/FeignHeaderInstrumentationTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/micrometer/src/test/java/feign/micrometer/FeignHeaderInstrumentationTest.java b/micrometer/src/test/java/feign/micrometer/FeignHeaderInstrumentationTest.java index a5d0941fe..2eda54cbc 100644 --- a/micrometer/src/test/java/feign/micrometer/FeignHeaderInstrumentationTest.java +++ b/micrometer/src/test/java/feign/micrometer/FeignHeaderInstrumentationTest.java @@ -14,7 +14,6 @@ package feign.micrometer; import java.util.concurrent.TimeUnit; - import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo; import com.github.tomakehurst.wiremock.junit5.WireMockTest; import feign.Feign; @@ -53,7 +52,7 @@ class FeignHeaderInstrumentationTest { @BeforeEach void setup() { observationRegistry.observationConfig() - .observationHandler(new DefaultMeterObservationHandler(meterRegistry)); + .observationHandler(new DefaultMeterObservationHandler(meterRegistry)); observationRegistry.observationConfig().observationHandler(new HeaderMutatingHandler()); } From 97b23e96da3372a36e21f0fd2f6895dc49aa89c6 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Mon, 19 Sep 2022 17:05:22 +0200 Subject: [PATCH 04/22] Fixed wrong status code method call --- .../feign/micrometer/DefaultFeignObservationConvention.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/micrometer/src/main/java/feign/micrometer/DefaultFeignObservationConvention.java b/micrometer/src/main/java/feign/micrometer/DefaultFeignObservationConvention.java index 447b4824f..8e284991b 100644 --- a/micrometer/src/main/java/feign/micrometer/DefaultFeignObservationConvention.java +++ b/micrometer/src/main/java/feign/micrometer/DefaultFeignObservationConvention.java @@ -59,7 +59,7 @@ public KeyValues getLowCardinalityKeyValues(FeignContext context) { } String getStatusValue(@Nullable Response response) { - return response != null ? response.reason() : "CLIENT_ERROR"; + return response != null ? String.valueOf(response.status()) : "CLIENT_ERROR"; } String getMethodString(@Nullable RequestTemplate request) { From 9119a61304435f28213dece37a542f197f4b6eec Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Tue, 20 Sep 2022 16:09:21 +0200 Subject: [PATCH 05/22] Converted to using around --- .../main/java/feign/ClientInterceptor.java | 37 ++--- .../java/feign/ClientInvocationContext.java | 9 -- .../java/feign/SynchronousMethodHandler.java | 133 ++++++++++++------ .../micrometer/ObservedClientInterceptor.java | 41 +++--- 4 files changed, 123 insertions(+), 97 deletions(-) diff --git a/core/src/main/java/feign/ClientInterceptor.java b/core/src/main/java/feign/ClientInterceptor.java index 6087ade00..5d1218799 100644 --- a/core/src/main/java/feign/ClientInterceptor.java +++ b/core/src/main/java/feign/ClientInterceptor.java @@ -13,41 +13,26 @@ */ package feign; +import java.util.Iterator; + /** * Zero or One {@code ClientInterceptor} may be configured for purposes such as tracing - mutate * headers, execute the call, parse the response and close any previously created objects. */ public interface ClientInterceptor { - ClientInterceptor DEFAULT = new ClientInterceptor() { - @Override - public void beforeExecute(ClientInvocationContext context) {} - - @Override - public void afterExecute(ClientInvocationContext context, - Response response, - Throwable exception) { - - } - }; - - /** - * Called before the request was made. Can be used to mutate the request and create objects that - * later will be passed to {@link #afterExecute(ClientInvocationContext, Response, Throwable)}. - * - * @param clientInvocationContext information surrounding the request - */ - void beforeExecute(ClientInvocationContext clientInvocationContext); + ClientInterceptor DEFAULT = (context, iterator) -> null; /** + * Allows to make an around instrumentation. Remember to call the next interceptor by calling + * {@code ClientInterceptor interceptor = iterator.next(); return interceptor.around(context, iterator)}. * - * @param clientInvocationContext information surrounding the request - * @param response received undecoded response (can be null when an exception occurred) - * @param exception exception that occurred while trying to send the request or receive the - * response (can be null when there was no exception) + * @param context input context to send a request + * @param iterator iterator to the next {@link ClientInterceptor} + * @return response or an exception - remember to rethrow an exception if it occurrs + * @throws FeignException exception while trying to send a request */ - void afterExecute(ClientInvocationContext clientInvocationContext, - Response response, - Throwable exception); + Response around(ClientInvocationContext context, Iterator iterator) + throws FeignException; } diff --git a/core/src/main/java/feign/ClientInvocationContext.java b/core/src/main/java/feign/ClientInvocationContext.java index 850626693..b340d7ff0 100644 --- a/core/src/main/java/feign/ClientInvocationContext.java +++ b/core/src/main/java/feign/ClientInvocationContext.java @@ -13,17 +13,12 @@ */ package feign; -import java.util.HashMap; -import java.util.Map; - public class ClientInvocationContext { private final RequestTemplate requestTemplate; private final Request.Options options; - private final Map holder = new HashMap<>(); - public ClientInvocationContext(RequestTemplate requestTemplate, Request.Options options) { this.requestTemplate = requestTemplate; this.options = options; @@ -36,8 +31,4 @@ public RequestTemplate getRequestTemplate() { public Request.Options getOptions() { return options; } - - public Map getHolder() { - return holder; - } } diff --git a/core/src/main/java/feign/SynchronousMethodHandler.java b/core/src/main/java/feign/SynchronousMethodHandler.java index 4208a3417..155fcf9e8 100644 --- a/core/src/main/java/feign/SynchronousMethodHandler.java +++ b/core/src/main/java/feign/SynchronousMethodHandler.java @@ -21,6 +21,8 @@ import feign.codec.Decoder; import feign.codec.ErrorDecoder; import java.io.IOException; +import java.util.ArrayList; +import java.util.Iterator; import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; @@ -115,37 +117,11 @@ public Object invoke(Object[] argv) throws Throwable { Object executeAndDecode(RequestTemplate template, Options options) throws Throwable { ClientInvocationContext context = new ClientInvocationContext(template, options); - for (ClientInterceptor interceptor : this.clientInterceptors) { - interceptor.beforeExecute(context); - } - - Request request = targetRequest(template); - - if (logLevel != Logger.Level.NONE) { - logger.logRequest(metadata.configKey(), logLevel, request); - } - - Response response = null; - Throwable throwable = null; long start = System.nanoTime(); - try { - response = client.execute(request, options); - // ensure the request is set. TODO: remove in Feign 12 - response = response.toBuilder() - .request(request) - .requestTemplate(template) - .build(); - } catch (IOException e) { - throwable = e; - if (logLevel != Logger.Level.NONE) { - logger.logIOException(metadata.configKey(), logLevel, e, elapsedTime(start)); - } - throw errorExecuting(request, e); - } finally { - for (ClientInterceptor interceptor : this.clientInterceptors) { - interceptor.afterExecute(context, response, throwable); - } - } + InterceptorChain interceptorChain = + new InterceptorChain(this.clientInterceptors, new HttpCall(this.metadata, this.target, + this.requestInterceptors, this.logger, this.logLevel, this.client, start)); + Response response = interceptorChain.call(context); long elapsedTime = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start); if (decoder != null) { @@ -169,17 +145,6 @@ Object executeAndDecode(RequestTemplate template, Options options) throws Throwa } } - long elapsedTime(long start) { - return TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start); - } - - Request targetRequest(RequestTemplate template) { - for (RequestInterceptor interceptor : requestInterceptors) { - interceptor.apply(template); - } - return target.apply(template); - } - Options findOptions(Object[] argv) { if (argv == null || argv.length == 0) { return this.options; @@ -191,6 +156,92 @@ Options findOptions(Object[] argv) { .orElse(this.options); } + static class HttpCall { + private final MethodMetadata metadata; + private final Target target; + private final List requestInterceptors; + private final Logger logger; + private final Logger.Level logLevel; + private final Client client; + + private final long start; + + HttpCall(MethodMetadata metadata, Target target, + List requestInterceptors, Logger logger, Logger.Level logLevel, + Client client, long start) { + this.metadata = metadata; + this.target = target; + this.requestInterceptors = requestInterceptors; + this.logger = logger; + this.logLevel = logLevel; + this.client = client; + this.start = start; + } + + Response call(RequestTemplate template, Request.Options options) { + Request request = targetRequest(template); + + if (logLevel != Logger.Level.NONE) { + logger.logRequest(metadata.configKey(), logLevel, request); + } + + Response response; + try { + response = client.execute(request, options); + // ensure the request is set. TODO: remove in Feign 12 + return response.toBuilder() + .request(request) + .requestTemplate(template) + .build(); + } catch (IOException e) { + if (logLevel != Logger.Level.NONE) { + logger.logIOException(metadata.configKey(), logLevel, e, elapsedTime(start)); + } + throw errorExecuting(request, e); + } + } + + Request targetRequest(RequestTemplate template) { + for (RequestInterceptor interceptor : requestInterceptors) { + interceptor.apply(template); + } + return target.apply(template); + } + + long elapsedTime(long start) { + return TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start); + } + } + + private static class HttpCallingInterceptor implements ClientInterceptor { + + private final HttpCall httpCall; + + HttpCallingInterceptor(HttpCall httpCall) { + this.httpCall = httpCall; + } + + @Override + public Response around(ClientInvocationContext context, Iterator iterator) { + return httpCall.call(context.getRequestTemplate(), context.getOptions()); + } + } + + private static class InterceptorChain { + private final List interceptors; + + InterceptorChain(List interceptors, HttpCall httpCall) { + this.interceptors = new ArrayList<>(interceptors); + this.interceptors.add(new HttpCallingInterceptor(httpCall)); + } + + Response call(ClientInvocationContext context) { + Iterator iterator = this.interceptors.iterator(); + ClientInterceptor next = iterator.next(); + return next.around(context, iterator); + } + } + static class Factory { private final Client client; diff --git a/micrometer/src/main/java/feign/micrometer/ObservedClientInterceptor.java b/micrometer/src/main/java/feign/micrometer/ObservedClientInterceptor.java index 1fbe6e714..a84b0b1cf 100644 --- a/micrometer/src/main/java/feign/micrometer/ObservedClientInterceptor.java +++ b/micrometer/src/main/java/feign/micrometer/ObservedClientInterceptor.java @@ -13,9 +13,11 @@ */ package feign.micrometer; +import java.util.Iterator; import feign.Client; import feign.ClientInterceptor; import feign.ClientInvocationContext; +import feign.FeignException; import feign.Response; import io.micrometer.observation.Observation; import io.micrometer.observation.ObservationRegistry; @@ -38,31 +40,28 @@ public ObservedClientInterceptor(ObservationRegistry observationRegistry) { } @Override - public void beforeExecute(ClientInvocationContext clientInvocationContext) { - FeignContext feignContext = new FeignContext(clientInvocationContext.getRequestTemplate()); + public Response around(ClientInvocationContext context, Iterator iterator) + throws FeignException { + FeignContext feignContext = new FeignContext(context.getRequestTemplate()); Observation observation = FeignDocumentedObservation.DEFAULT .observation(this.customFeignObservationConvention, DefaultFeignObservationConvention.INSTANCE, feignContext, this.observationRegistry) .start(); - clientInvocationContext.getHolder().put(Observation.class, observation); - clientInvocationContext.getHolder().put(FeignContext.class, feignContext); - } - - @Override - public void afterExecute(ClientInvocationContext clientInvocationContext, - Response response, - Throwable exception) { - FeignContext feignContext = - (FeignContext) clientInvocationContext.getHolder().get(FeignContext.class); - Observation observation = - (Observation) clientInvocationContext.getHolder().get(Observation.class); - if (feignContext == null) { - return; - } - feignContext.setResponse(response); - if (exception != null) { - observation.error(exception); + Exception ex = null; + Response response = null; + try { + ClientInterceptor interceptor = iterator.next(); + response = interceptor.around(context, iterator); + return response; + } catch (FeignException exception) { + ex = exception; + throw exception; + } finally { + feignContext.setResponse(response); + if (ex != null) { + observation.error(ex); + } + observation.stop(); } - observation.stop(); } } From 8832affd89d4dd214fb39611514e41b6153d7cc1 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Tue, 20 Sep 2022 16:26:35 +0200 Subject: [PATCH 06/22] Fixed compilation issues --- core/src/main/java/feign/AsyncFeign.java | 2 +- core/src/test/java/feign/BaseBuilderTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/feign/AsyncFeign.java b/core/src/main/java/feign/AsyncFeign.java index 048401dce..ce2be5238 100644 --- a/core/src/main/java/feign/AsyncFeign.java +++ b/core/src/main/java/feign/AsyncFeign.java @@ -217,7 +217,7 @@ public AsyncFeign build() { final SynchronousMethodHandler.Factory synchronousMethodHandlerFactory = new SynchronousMethodHandler.Factory(stageExecution(activeContextHolder, client), retryer, - requestInterceptors, + requestInterceptors, this.clientInterceptors, responseInterceptor, logger, logLevel, dismiss404, closeAfterDecode, propagationPolicy, true); final ParseHandlersByName handlersByName = diff --git a/core/src/test/java/feign/BaseBuilderTest.java b/core/src/test/java/feign/BaseBuilderTest.java index d071b8a19..d1c8434d9 100644 --- a/core/src/test/java/feign/BaseBuilderTest.java +++ b/core/src/test/java/feign/BaseBuilderTest.java @@ -27,7 +27,7 @@ public class BaseBuilderTest { public void checkEnrichTouchesAllAsyncBuilderFields() throws IllegalArgumentException, IllegalAccessException { test(AsyncFeign.builder().requestInterceptor(template -> { - }).clientInterceptor(ClientInterceptor.DEFAULT), 14); + }).clientInterceptor(ClientInterceptor.DEFAULT), 15); } private void test(BaseBuilder builder, int expectedFieldsCount) From 75f5a976945eecb69bd81984ca6da050ca8e6928 Mon Sep 17 00:00:00 2001 From: Marvin Froeder Date: Thu, 22 Sep 2022 09:06:03 +1200 Subject: [PATCH 07/22] prepare release 11.10 --- annotation-error-decoder/pom.xml | 2 +- apt-test-generator/pom.xml | 2 +- benchmark/pom.xml | 2 +- core/pom.xml | 2 +- dropwizard-metrics4/pom.xml | 2 +- dropwizard-metrics5/pom.xml | 2 +- example-github/pom.xml | 2 +- example-wikipedia/pom.xml | 2 +- googlehttpclient/pom.xml | 2 +- gson/pom.xml | 2 +- hc5/pom.xml | 2 +- httpclient/pom.xml | 2 +- hystrix/pom.xml | 2 +- jackson-jaxb/pom.xml | 2 +- jackson-jr/pom.xml | 2 +- jackson/pom.xml | 2 +- jakarta/pom.xml | 2 +- java11/pom.xml | 2 +- jaxb/pom.xml | 2 +- jaxrs/pom.xml | 2 +- jaxrs2/pom.xml | 2 +- json/pom.xml | 2 +- micrometer/pom.xml | 2 +- mock/pom.xml | 2 +- okhttp/pom.xml | 2 +- pom.xml | 2 +- reactive/pom.xml | 2 +- ribbon/pom.xml | 2 +- sax/pom.xml | 2 +- slf4j/pom.xml | 2 +- soap/pom.xml | 2 +- spring4/pom.xml | 2 +- 32 files changed, 32 insertions(+), 32 deletions(-) diff --git a/annotation-error-decoder/pom.xml b/annotation-error-decoder/pom.xml index d47ebe77d..7907f10bc 100644 --- a/annotation-error-decoder/pom.xml +++ b/annotation-error-decoder/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-annotation-error-decoder diff --git a/apt-test-generator/pom.xml b/apt-test-generator/pom.xml index 36a3fcb10..77961a639 100644 --- a/apt-test-generator/pom.xml +++ b/apt-test-generator/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 io.github.openfeign.experimental diff --git a/benchmark/pom.xml b/benchmark/pom.xml index b7b630d01..5c10259ea 100644 --- a/benchmark/pom.xml +++ b/benchmark/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-benchmark diff --git a/core/pom.xml b/core/pom.xml index 780122ce5..f9aa642fd 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-core diff --git a/dropwizard-metrics4/pom.xml b/dropwizard-metrics4/pom.xml index 874088623..3b7c3b1b2 100644 --- a/dropwizard-metrics4/pom.xml +++ b/dropwizard-metrics4/pom.xml @@ -19,7 +19,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-dropwizard-metrics4 Feign Dropwizard Metrics4 diff --git a/dropwizard-metrics5/pom.xml b/dropwizard-metrics5/pom.xml index 10958d16c..0060cb3c7 100644 --- a/dropwizard-metrics5/pom.xml +++ b/dropwizard-metrics5/pom.xml @@ -19,7 +19,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-dropwizard-metrics5 Feign Dropwizard Metrics5 diff --git a/example-github/pom.xml b/example-github/pom.xml index 77ed79ea4..d7bae587c 100644 --- a/example-github/pom.xml +++ b/example-github/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-example-github diff --git a/example-wikipedia/pom.xml b/example-wikipedia/pom.xml index 3cee876d7..123fc2d21 100644 --- a/example-wikipedia/pom.xml +++ b/example-wikipedia/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 io.github.openfeign diff --git a/googlehttpclient/pom.xml b/googlehttpclient/pom.xml index 2417be990..80f29be4b 100644 --- a/googlehttpclient/pom.xml +++ b/googlehttpclient/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-googlehttpclient diff --git a/gson/pom.xml b/gson/pom.xml index d64594ec2..724bc2a48 100644 --- a/gson/pom.xml +++ b/gson/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-gson diff --git a/hc5/pom.xml b/hc5/pom.xml index 631076bd2..9ab9123a5 100644 --- a/hc5/pom.xml +++ b/hc5/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-hc5 diff --git a/httpclient/pom.xml b/httpclient/pom.xml index d952378b5..856f20e2a 100644 --- a/httpclient/pom.xml +++ b/httpclient/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-httpclient diff --git a/hystrix/pom.xml b/hystrix/pom.xml index 1be137e9b..bce141c1d 100644 --- a/hystrix/pom.xml +++ b/hystrix/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-hystrix diff --git a/jackson-jaxb/pom.xml b/jackson-jaxb/pom.xml index 9f4a3157e..1e6f533e7 100644 --- a/jackson-jaxb/pom.xml +++ b/jackson-jaxb/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-jackson-jaxb diff --git a/jackson-jr/pom.xml b/jackson-jr/pom.xml index 7fe255dad..2f10f0e22 100644 --- a/jackson-jr/pom.xml +++ b/jackson-jr/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-jackson-jr diff --git a/jackson/pom.xml b/jackson/pom.xml index cb1a8b5ca..8d2163bdf 100644 --- a/jackson/pom.xml +++ b/jackson/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-jackson diff --git a/jakarta/pom.xml b/jakarta/pom.xml index 7d2a0e67e..aa50f33cf 100644 --- a/jakarta/pom.xml +++ b/jakarta/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-jakarta diff --git a/java11/pom.xml b/java11/pom.xml index ef0fac29c..8768557fe 100644 --- a/java11/pom.xml +++ b/java11/pom.xml @@ -19,7 +19,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-java11 diff --git a/jaxb/pom.xml b/jaxb/pom.xml index ec7cbc7f8..f1dfead9a 100644 --- a/jaxb/pom.xml +++ b/jaxb/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-jaxb diff --git a/jaxrs/pom.xml b/jaxrs/pom.xml index 0eb7ef33b..4598bc08a 100644 --- a/jaxrs/pom.xml +++ b/jaxrs/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-jaxrs diff --git a/jaxrs2/pom.xml b/jaxrs2/pom.xml index 508e16693..4ffc126ab 100644 --- a/jaxrs2/pom.xml +++ b/jaxrs2/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-jaxrs2 diff --git a/json/pom.xml b/json/pom.xml index e529ff63a..58b523827 100644 --- a/json/pom.xml +++ b/json/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-json diff --git a/micrometer/pom.xml b/micrometer/pom.xml index bb02c5cbb..5297ff243 100644 --- a/micrometer/pom.xml +++ b/micrometer/pom.xml @@ -19,7 +19,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-micrometer Feign Micrometer diff --git a/mock/pom.xml b/mock/pom.xml index 6e1e15737..b41a9b639 100644 --- a/mock/pom.xml +++ b/mock/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-mock diff --git a/okhttp/pom.xml b/okhttp/pom.xml index b3aef9dc3..5d2ed7d17 100644 --- a/okhttp/pom.xml +++ b/okhttp/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-okhttp diff --git a/pom.xml b/pom.xml index 9a185ccab..09c0be060 100644 --- a/pom.xml +++ b/pom.xml @@ -19,7 +19,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 pom Feign (Parent) diff --git a/reactive/pom.xml b/reactive/pom.xml index 0bb68b63f..d94e33754 100644 --- a/reactive/pom.xml +++ b/reactive/pom.xml @@ -19,7 +19,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-reactive-wrappers diff --git a/ribbon/pom.xml b/ribbon/pom.xml index cfa97a500..09c9376fb 100644 --- a/ribbon/pom.xml +++ b/ribbon/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-ribbon diff --git a/sax/pom.xml b/sax/pom.xml index 843b4dcf3..089c20ad6 100644 --- a/sax/pom.xml +++ b/sax/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-sax diff --git a/slf4j/pom.xml b/slf4j/pom.xml index 5e17cf3f8..5f07415bc 100644 --- a/slf4j/pom.xml +++ b/slf4j/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-slf4j diff --git a/soap/pom.xml b/soap/pom.xml index 3d9e65c6c..78ef560ff 100644 --- a/soap/pom.xml +++ b/soap/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-soap diff --git a/spring4/pom.xml b/spring4/pom.xml index c473cbe69..2f11a9fbd 100644 --- a/spring4/pom.xml +++ b/spring4/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 11.10 feign-spring4 From 6a213cfd7a2da74ab13c77eae43c23a6ee35beba Mon Sep 17 00:00:00 2001 From: Marvin Froeder Date: Thu, 22 Sep 2022 09:06:13 +1200 Subject: [PATCH 08/22] [ci skip] updating versions to next development iteration 11.11-SNAPSHOT --- annotation-error-decoder/pom.xml | 2 +- apt-test-generator/pom.xml | 2 +- benchmark/pom.xml | 2 +- core/pom.xml | 2 +- dropwizard-metrics4/pom.xml | 2 +- dropwizard-metrics5/pom.xml | 2 +- example-github/pom.xml | 2 +- example-wikipedia/pom.xml | 2 +- googlehttpclient/pom.xml | 2 +- gson/pom.xml | 2 +- hc5/pom.xml | 2 +- httpclient/pom.xml | 2 +- hystrix/pom.xml | 2 +- jackson-jaxb/pom.xml | 2 +- jackson-jr/pom.xml | 2 +- jackson/pom.xml | 2 +- jakarta/pom.xml | 2 +- java11/pom.xml | 2 +- jaxb/pom.xml | 2 +- jaxrs/pom.xml | 2 +- jaxrs2/pom.xml | 2 +- json/pom.xml | 2 +- micrometer/pom.xml | 2 +- mock/pom.xml | 2 +- okhttp/pom.xml | 2 +- pom.xml | 2 +- reactive/pom.xml | 2 +- ribbon/pom.xml | 2 +- sax/pom.xml | 2 +- slf4j/pom.xml | 2 +- soap/pom.xml | 2 +- spring4/pom.xml | 2 +- 32 files changed, 32 insertions(+), 32 deletions(-) diff --git a/annotation-error-decoder/pom.xml b/annotation-error-decoder/pom.xml index 7907f10bc..13637f359 100644 --- a/annotation-error-decoder/pom.xml +++ b/annotation-error-decoder/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-annotation-error-decoder diff --git a/apt-test-generator/pom.xml b/apt-test-generator/pom.xml index 77961a639..085044d1a 100644 --- a/apt-test-generator/pom.xml +++ b/apt-test-generator/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT io.github.openfeign.experimental diff --git a/benchmark/pom.xml b/benchmark/pom.xml index 5c10259ea..b7dff8c15 100644 --- a/benchmark/pom.xml +++ b/benchmark/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-benchmark diff --git a/core/pom.xml b/core/pom.xml index f9aa642fd..0467aec7e 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-core diff --git a/dropwizard-metrics4/pom.xml b/dropwizard-metrics4/pom.xml index 3b7c3b1b2..30c61c790 100644 --- a/dropwizard-metrics4/pom.xml +++ b/dropwizard-metrics4/pom.xml @@ -19,7 +19,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-dropwizard-metrics4 Feign Dropwizard Metrics4 diff --git a/dropwizard-metrics5/pom.xml b/dropwizard-metrics5/pom.xml index 0060cb3c7..35a5a11c1 100644 --- a/dropwizard-metrics5/pom.xml +++ b/dropwizard-metrics5/pom.xml @@ -19,7 +19,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-dropwizard-metrics5 Feign Dropwizard Metrics5 diff --git a/example-github/pom.xml b/example-github/pom.xml index d7bae587c..ad4a20e5a 100644 --- a/example-github/pom.xml +++ b/example-github/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-example-github diff --git a/example-wikipedia/pom.xml b/example-wikipedia/pom.xml index 123fc2d21..f660a3ea1 100644 --- a/example-wikipedia/pom.xml +++ b/example-wikipedia/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT io.github.openfeign diff --git a/googlehttpclient/pom.xml b/googlehttpclient/pom.xml index 80f29be4b..b789194b4 100644 --- a/googlehttpclient/pom.xml +++ b/googlehttpclient/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-googlehttpclient diff --git a/gson/pom.xml b/gson/pom.xml index 724bc2a48..ca427fea3 100644 --- a/gson/pom.xml +++ b/gson/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-gson diff --git a/hc5/pom.xml b/hc5/pom.xml index 9ab9123a5..aa4e235c6 100644 --- a/hc5/pom.xml +++ b/hc5/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-hc5 diff --git a/httpclient/pom.xml b/httpclient/pom.xml index 856f20e2a..aa77d3499 100644 --- a/httpclient/pom.xml +++ b/httpclient/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-httpclient diff --git a/hystrix/pom.xml b/hystrix/pom.xml index bce141c1d..7e6cf82c3 100644 --- a/hystrix/pom.xml +++ b/hystrix/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-hystrix diff --git a/jackson-jaxb/pom.xml b/jackson-jaxb/pom.xml index 1e6f533e7..11beda7cf 100644 --- a/jackson-jaxb/pom.xml +++ b/jackson-jaxb/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-jackson-jaxb diff --git a/jackson-jr/pom.xml b/jackson-jr/pom.xml index 2f10f0e22..9f80ef931 100644 --- a/jackson-jr/pom.xml +++ b/jackson-jr/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-jackson-jr diff --git a/jackson/pom.xml b/jackson/pom.xml index 8d2163bdf..fd25138f3 100644 --- a/jackson/pom.xml +++ b/jackson/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-jackson diff --git a/jakarta/pom.xml b/jakarta/pom.xml index aa50f33cf..eac1e40b8 100644 --- a/jakarta/pom.xml +++ b/jakarta/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-jakarta diff --git a/java11/pom.xml b/java11/pom.xml index 8768557fe..6d397162b 100644 --- a/java11/pom.xml +++ b/java11/pom.xml @@ -19,7 +19,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-java11 diff --git a/jaxb/pom.xml b/jaxb/pom.xml index f1dfead9a..48cd97a24 100644 --- a/jaxb/pom.xml +++ b/jaxb/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-jaxb diff --git a/jaxrs/pom.xml b/jaxrs/pom.xml index 4598bc08a..7c48d7164 100644 --- a/jaxrs/pom.xml +++ b/jaxrs/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-jaxrs diff --git a/jaxrs2/pom.xml b/jaxrs2/pom.xml index 4ffc126ab..b59270a42 100644 --- a/jaxrs2/pom.xml +++ b/jaxrs2/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-jaxrs2 diff --git a/json/pom.xml b/json/pom.xml index 58b523827..e68bfcd3c 100644 --- a/json/pom.xml +++ b/json/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-json diff --git a/micrometer/pom.xml b/micrometer/pom.xml index 5297ff243..908be7dc8 100644 --- a/micrometer/pom.xml +++ b/micrometer/pom.xml @@ -19,7 +19,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-micrometer Feign Micrometer diff --git a/mock/pom.xml b/mock/pom.xml index b41a9b639..27adffb1f 100644 --- a/mock/pom.xml +++ b/mock/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-mock diff --git a/okhttp/pom.xml b/okhttp/pom.xml index 5d2ed7d17..31bb7ff49 100644 --- a/okhttp/pom.xml +++ b/okhttp/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-okhttp diff --git a/pom.xml b/pom.xml index 09c0be060..6be2a571a 100644 --- a/pom.xml +++ b/pom.xml @@ -19,7 +19,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT pom Feign (Parent) diff --git a/reactive/pom.xml b/reactive/pom.xml index d94e33754..87ae94eea 100644 --- a/reactive/pom.xml +++ b/reactive/pom.xml @@ -19,7 +19,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-reactive-wrappers diff --git a/ribbon/pom.xml b/ribbon/pom.xml index 09c9376fb..42bfa6bce 100644 --- a/ribbon/pom.xml +++ b/ribbon/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-ribbon diff --git a/sax/pom.xml b/sax/pom.xml index 089c20ad6..933033136 100644 --- a/sax/pom.xml +++ b/sax/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-sax diff --git a/slf4j/pom.xml b/slf4j/pom.xml index 5f07415bc..e1a5c0c1d 100644 --- a/slf4j/pom.xml +++ b/slf4j/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-slf4j diff --git a/soap/pom.xml b/soap/pom.xml index 78ef560ff..d63afa376 100644 --- a/soap/pom.xml +++ b/soap/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-soap diff --git a/spring4/pom.xml b/spring4/pom.xml index 2f11a9fbd..f809ad41b 100644 --- a/spring4/pom.xml +++ b/spring4/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10 + 11.11-SNAPSHOT feign-spring4 From 4a9d1fb52531cfe90da2b20e09b2f8aa6b92a9e6 Mon Sep 17 00:00:00 2001 From: Marvin Froeder Date: Thu, 22 Sep 2022 09:16:26 +1200 Subject: [PATCH 09/22] Preparing for next development version --- annotation-error-decoder/pom.xml | 2 +- apt-test-generator/pom.xml | 2 +- benchmark/pom.xml | 2 +- core/pom.xml | 2 +- dropwizard-metrics4/pom.xml | 2 +- dropwizard-metrics5/pom.xml | 2 +- example-github-with-coroutine/pom.xml | 2 +- example-github/pom.xml | 2 +- example-wikipedia/pom.xml | 2 +- googlehttpclient/pom.xml | 2 +- gson/pom.xml | 2 +- hc5/pom.xml | 2 +- httpclient/pom.xml | 2 +- hystrix/pom.xml | 2 +- jackson-jaxb/pom.xml | 2 +- jackson-jr/pom.xml | 2 +- jackson/pom.xml | 2 +- jakarta/pom.xml | 2 +- java11/pom.xml | 2 +- jaxb/pom.xml | 2 +- jaxrs/pom.xml | 2 +- jaxrs2/pom.xml | 2 +- json/pom.xml | 2 +- kotlin/pom.xml | 2 +- micrometer/pom.xml | 2 +- mock/pom.xml | 2 +- okhttp/pom.xml | 2 +- pom.xml | 2 +- reactive/pom.xml | 2 +- ribbon/pom.xml | 2 +- sax/pom.xml | 2 +- slf4j/pom.xml | 2 +- soap/pom.xml | 2 +- spring4/pom.xml | 2 +- 34 files changed, 34 insertions(+), 34 deletions(-) diff --git a/annotation-error-decoder/pom.xml b/annotation-error-decoder/pom.xml index 13637f359..83f5b5041 100644 --- a/annotation-error-decoder/pom.xml +++ b/annotation-error-decoder/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-annotation-error-decoder diff --git a/apt-test-generator/pom.xml b/apt-test-generator/pom.xml index 085044d1a..0fbf28fe2 100644 --- a/apt-test-generator/pom.xml +++ b/apt-test-generator/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT io.github.openfeign.experimental diff --git a/benchmark/pom.xml b/benchmark/pom.xml index b7dff8c15..3af013c79 100644 --- a/benchmark/pom.xml +++ b/benchmark/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-benchmark diff --git a/core/pom.xml b/core/pom.xml index 0467aec7e..e2bbf747a 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-core diff --git a/dropwizard-metrics4/pom.xml b/dropwizard-metrics4/pom.xml index 30c61c790..3c92f9b4f 100644 --- a/dropwizard-metrics4/pom.xml +++ b/dropwizard-metrics4/pom.xml @@ -19,7 +19,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-dropwizard-metrics4 Feign Dropwizard Metrics4 diff --git a/dropwizard-metrics5/pom.xml b/dropwizard-metrics5/pom.xml index 35a5a11c1..7daa543ce 100644 --- a/dropwizard-metrics5/pom.xml +++ b/dropwizard-metrics5/pom.xml @@ -19,7 +19,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-dropwizard-metrics5 Feign Dropwizard Metrics5 diff --git a/example-github-with-coroutine/pom.xml b/example-github-with-coroutine/pom.xml index 38d38d0c3..2239910b6 100644 --- a/example-github-with-coroutine/pom.xml +++ b/example-github-with-coroutine/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 12.0-SNAPSHOT feign-example-github-with-coroutine diff --git a/example-github/pom.xml b/example-github/pom.xml index ad4a20e5a..ba1d7845c 100644 --- a/example-github/pom.xml +++ b/example-github/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-example-github diff --git a/example-wikipedia/pom.xml b/example-wikipedia/pom.xml index f660a3ea1..4b3998dca 100644 --- a/example-wikipedia/pom.xml +++ b/example-wikipedia/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT io.github.openfeign diff --git a/googlehttpclient/pom.xml b/googlehttpclient/pom.xml index b789194b4..d02087c5f 100644 --- a/googlehttpclient/pom.xml +++ b/googlehttpclient/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-googlehttpclient diff --git a/gson/pom.xml b/gson/pom.xml index ca427fea3..cd7ddfa52 100644 --- a/gson/pom.xml +++ b/gson/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-gson diff --git a/hc5/pom.xml b/hc5/pom.xml index aa4e235c6..e821480d9 100644 --- a/hc5/pom.xml +++ b/hc5/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-hc5 diff --git a/httpclient/pom.xml b/httpclient/pom.xml index aa77d3499..8ad438b15 100644 --- a/httpclient/pom.xml +++ b/httpclient/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-httpclient diff --git a/hystrix/pom.xml b/hystrix/pom.xml index 7e6cf82c3..0dcdbaddf 100644 --- a/hystrix/pom.xml +++ b/hystrix/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-hystrix diff --git a/jackson-jaxb/pom.xml b/jackson-jaxb/pom.xml index 11beda7cf..6e1d3931d 100644 --- a/jackson-jaxb/pom.xml +++ b/jackson-jaxb/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-jackson-jaxb diff --git a/jackson-jr/pom.xml b/jackson-jr/pom.xml index 9f80ef931..f2e1b7ab0 100644 --- a/jackson-jr/pom.xml +++ b/jackson-jr/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-jackson-jr diff --git a/jackson/pom.xml b/jackson/pom.xml index fd25138f3..f3e778ef5 100644 --- a/jackson/pom.xml +++ b/jackson/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-jackson diff --git a/jakarta/pom.xml b/jakarta/pom.xml index eac1e40b8..0b54ab6a7 100644 --- a/jakarta/pom.xml +++ b/jakarta/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-jakarta diff --git a/java11/pom.xml b/java11/pom.xml index 6d397162b..09a09e8da 100644 --- a/java11/pom.xml +++ b/java11/pom.xml @@ -19,7 +19,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-java11 diff --git a/jaxb/pom.xml b/jaxb/pom.xml index 48cd97a24..b99bc7ccf 100644 --- a/jaxb/pom.xml +++ b/jaxb/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-jaxb diff --git a/jaxrs/pom.xml b/jaxrs/pom.xml index 7c48d7164..4986f4fe5 100644 --- a/jaxrs/pom.xml +++ b/jaxrs/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-jaxrs diff --git a/jaxrs2/pom.xml b/jaxrs2/pom.xml index b59270a42..c9c1c2565 100644 --- a/jaxrs2/pom.xml +++ b/jaxrs2/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-jaxrs2 diff --git a/json/pom.xml b/json/pom.xml index e68bfcd3c..5935fee9b 100644 --- a/json/pom.xml +++ b/json/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-json diff --git a/kotlin/pom.xml b/kotlin/pom.xml index 8db570853..7307a9550 100644 --- a/kotlin/pom.xml +++ b/kotlin/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.10-SNAPSHOT + 12.0-SNAPSHOT feign-kotlin diff --git a/micrometer/pom.xml b/micrometer/pom.xml index 908be7dc8..4bf16759f 100644 --- a/micrometer/pom.xml +++ b/micrometer/pom.xml @@ -19,7 +19,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-micrometer Feign Micrometer diff --git a/mock/pom.xml b/mock/pom.xml index 27adffb1f..e1db71119 100644 --- a/mock/pom.xml +++ b/mock/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-mock diff --git a/okhttp/pom.xml b/okhttp/pom.xml index 31bb7ff49..9eb142765 100644 --- a/okhttp/pom.xml +++ b/okhttp/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-okhttp diff --git a/pom.xml b/pom.xml index 6be2a571a..7b754fdc0 100644 --- a/pom.xml +++ b/pom.xml @@ -19,7 +19,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT pom Feign (Parent) diff --git a/reactive/pom.xml b/reactive/pom.xml index 87ae94eea..e6ef73e0f 100644 --- a/reactive/pom.xml +++ b/reactive/pom.xml @@ -19,7 +19,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-reactive-wrappers diff --git a/ribbon/pom.xml b/ribbon/pom.xml index 42bfa6bce..51613077b 100644 --- a/ribbon/pom.xml +++ b/ribbon/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-ribbon diff --git a/sax/pom.xml b/sax/pom.xml index 933033136..3a9950530 100644 --- a/sax/pom.xml +++ b/sax/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-sax diff --git a/slf4j/pom.xml b/slf4j/pom.xml index e1a5c0c1d..4bd01d520 100644 --- a/slf4j/pom.xml +++ b/slf4j/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-slf4j diff --git a/soap/pom.xml b/soap/pom.xml index d63afa376..9390b7065 100644 --- a/soap/pom.xml +++ b/soap/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-soap diff --git a/spring4/pom.xml b/spring4/pom.xml index f809ad41b..c83c69d09 100644 --- a/spring4/pom.xml +++ b/spring4/pom.xml @@ -20,7 +20,7 @@ io.github.openfeign parent - 11.11-SNAPSHOT + 12.0-SNAPSHOT feign-spring4 From 5afb23473486a565a35ef0b13950f9ec6f957502 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 26 Sep 2022 22:44:37 +1300 Subject: [PATCH 10/22] 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] Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 7b754fdc0..7e8fefbf6 100644 --- a/pom.xml +++ b/pom.xml @@ -82,7 +82,7 @@ 2.9.1 2.0.2 1.70 - 20220320 + 20220924 4.13.2 2.13.4 From f02d3ca9ffeff61fec317c7475447d98218ac57b Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Tue, 27 Sep 2022 14:33:11 +0200 Subject: [PATCH 11/22] Updated to latest micrometer changes --- .../feign/micrometer/DefaultFeignObservationConvention.java | 6 +++--- ...dObservation.java => FeignObservationDocumentation.java} | 6 +++--- .../java/feign/micrometer/ObservedClientInterceptor.java | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) rename micrometer/src/main/java/feign/micrometer/{FeignDocumentedObservation.java => FeignObservationDocumentation.java} (90%) diff --git a/micrometer/src/main/java/feign/micrometer/DefaultFeignObservationConvention.java b/micrometer/src/main/java/feign/micrometer/DefaultFeignObservationConvention.java index 8e284991b..3ec33c6b6 100644 --- a/micrometer/src/main/java/feign/micrometer/DefaultFeignObservationConvention.java +++ b/micrometer/src/main/java/feign/micrometer/DefaultFeignObservationConvention.java @@ -50,11 +50,11 @@ public String getContextualName(FeignContext context) { public KeyValues getLowCardinalityKeyValues(FeignContext context) { String templatedUrl = context.getCarrier().methodMetadata().template().url(); return KeyValues.of( - FeignDocumentedObservation.HttpClientTags.METHOD + FeignObservationDocumentation.HttpClientTags.METHOD .withValue(getMethodString(context.getCarrier())), - FeignDocumentedObservation.HttpClientTags.URI + FeignObservationDocumentation.HttpClientTags.URI .withValue(templatedUrl), - FeignDocumentedObservation.HttpClientTags.STATUS + FeignObservationDocumentation.HttpClientTags.STATUS .withValue(getStatusValue(context.getResponse()))); } diff --git a/micrometer/src/main/java/feign/micrometer/FeignDocumentedObservation.java b/micrometer/src/main/java/feign/micrometer/FeignObservationDocumentation.java similarity index 90% rename from micrometer/src/main/java/feign/micrometer/FeignDocumentedObservation.java rename to micrometer/src/main/java/feign/micrometer/FeignObservationDocumentation.java index 1f52c94f3..dc64ddfd4 100644 --- a/micrometer/src/main/java/feign/micrometer/FeignDocumentedObservation.java +++ b/micrometer/src/main/java/feign/micrometer/FeignObservationDocumentation.java @@ -16,14 +16,14 @@ import io.micrometer.common.docs.KeyName; import io.micrometer.observation.Observation; import io.micrometer.observation.ObservationConvention; -import io.micrometer.observation.docs.DocumentedObservation; +import io.micrometer.observation.docs.ObservationDocumentation; /** - * {@link DocumentedObservation} for Feign. + * {@link ObservationDocumentation} for Feign. * * @since 1.10.0 */ -public enum FeignDocumentedObservation implements DocumentedObservation { +public enum FeignObservationDocumentation implements ObservationDocumentation { DEFAULT { @Override diff --git a/micrometer/src/main/java/feign/micrometer/ObservedClientInterceptor.java b/micrometer/src/main/java/feign/micrometer/ObservedClientInterceptor.java index a84b0b1cf..4a7abd3e0 100644 --- a/micrometer/src/main/java/feign/micrometer/ObservedClientInterceptor.java +++ b/micrometer/src/main/java/feign/micrometer/ObservedClientInterceptor.java @@ -43,7 +43,7 @@ public ObservedClientInterceptor(ObservationRegistry observationRegistry) { public Response around(ClientInvocationContext context, Iterator iterator) throws FeignException { FeignContext feignContext = new FeignContext(context.getRequestTemplate()); - Observation observation = FeignDocumentedObservation.DEFAULT + Observation observation = FeignObservationDocumentation.DEFAULT .observation(this.customFeignObservationConvention, DefaultFeignObservationConvention.INSTANCE, feignContext, this.observationRegistry) .start(); From 2551952854d7f47c2a3b72b9ced938aa4d4daf3d Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Wed, 28 Sep 2022 13:59:14 +0200 Subject: [PATCH 12/22] Enriches via clientInterceptors --- core/src/main/java/feign/AsyncFeign.java | 5 ----- core/src/main/java/feign/BaseBuilder.java | 2 -- core/src/main/java/feign/Capability.java | 4 ---- .../feign/micrometer/MicrometerCapability.java | 15 ++++++++------- .../FeignHeaderInstrumentationTest.java | 2 ++ 5 files changed, 10 insertions(+), 18 deletions(-) diff --git a/core/src/main/java/feign/AsyncFeign.java b/core/src/main/java/feign/AsyncFeign.java index ce2be5238..7dc6ac398 100644 --- a/core/src/main/java/feign/AsyncFeign.java +++ b/core/src/main/java/feign/AsyncFeign.java @@ -184,11 +184,6 @@ public AsyncBuilder requestInterceptors(Iterable requestI return super.requestInterceptors(requestInterceptors); } - @Override - public AsyncBuilder clientInterceptor(ClientInterceptor clientInterceptor) { - return super.clientInterceptor(clientInterceptor); - } - @Override public AsyncBuilder clientInterceptors(Iterable clientInterceptors) { return super.clientInterceptors(clientInterceptors); diff --git a/core/src/main/java/feign/BaseBuilder.java b/core/src/main/java/feign/BaseBuilder.java index 2e81295f0..01784fccd 100644 --- a/core/src/main/java/feign/BaseBuilder.java +++ b/core/src/main/java/feign/BaseBuilder.java @@ -251,8 +251,6 @@ protected B enrich() { return thisB; } - capabilities.forEach(capability -> capability.enrich(thisB)); - getFieldsToEnrich().forEach(field -> { field.setAccessible(true); try { diff --git a/core/src/main/java/feign/Capability.java b/core/src/main/java/feign/Capability.java index 82df29acf..d14497622 100644 --- a/core/src/main/java/feign/Capability.java +++ b/core/src/main/java/feign/Capability.java @@ -141,8 +141,4 @@ default AsyncContextSupplier enrich(AsyncContextSupplier asyncContextS default MethodInfoResolver enrich(MethodInfoResolver methodInfoResolver) { return methodInfoResolver; } - - default > BaseBuilder enrich(BaseBuilder baseBuilder) { - return baseBuilder; - } } diff --git a/micrometer/src/main/java/feign/micrometer/MicrometerCapability.java b/micrometer/src/main/java/feign/micrometer/MicrometerCapability.java index 71d31cc24..d2cc61cbf 100644 --- a/micrometer/src/main/java/feign/micrometer/MicrometerCapability.java +++ b/micrometer/src/main/java/feign/micrometer/MicrometerCapability.java @@ -17,6 +17,7 @@ import feign.BaseBuilder; import feign.Capability; import feign.Client; +import feign.ClientInterceptor; import feign.InvocationHandlerFactory; import feign.codec.Decoder; import feign.codec.Encoder; @@ -49,16 +50,16 @@ public MicrometerCapability(MeterRegistry meterRegistry, } @Override - public > BaseBuilder enrich(BaseBuilder baseBuilder) { - if (!observationRegistry.isNoop()) { - baseBuilder.clientInterceptor(new ObservedClientInterceptor(observationRegistry)); - } - return baseBuilder; + public Client enrich(Client client) { + return new MeteredClient(client, meterRegistry); } @Override - public Client enrich(Client client) { - return new MeteredClient(client, meterRegistry); + public ClientInterceptor enrich(ClientInterceptor clientInterceptor) { + if (!this.observationRegistry.isNoop()) { + return new ObservedClientInterceptor(observationRegistry); + } + return clientInterceptor; } @Override diff --git a/micrometer/src/test/java/feign/micrometer/FeignHeaderInstrumentationTest.java b/micrometer/src/test/java/feign/micrometer/FeignHeaderInstrumentationTest.java index 2eda54cbc..df94785dc 100644 --- a/micrometer/src/test/java/feign/micrometer/FeignHeaderInstrumentationTest.java +++ b/micrometer/src/test/java/feign/micrometer/FeignHeaderInstrumentationTest.java @@ -16,6 +16,7 @@ import java.util.concurrent.TimeUnit; import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo; import com.github.tomakehurst.wiremock.junit5.WireMockTest; +import feign.ClientInterceptor; import feign.Feign; import feign.Param; import feign.RequestLine; @@ -73,6 +74,7 @@ private TestClient clientInstrumentedWithObservations(String url) { return Feign.builder() .client(new feign.okhttp.OkHttpClient(new OkHttpClient())) .addCapability(new MicrometerCapability(meterRegistry, observationRegistry)) + .clientInterceptor(ClientInterceptor.DEFAULT) .target(TestClient.class, url); } From 2392312082e8f53f4cc05598d5385561b82859a8 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Wed, 28 Sep 2022 14:20:08 +0200 Subject: [PATCH 13/22] Fixed the error in the DEFAULT instance --- core/src/main/java/feign/ClientInterceptor.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/feign/ClientInterceptor.java b/core/src/main/java/feign/ClientInterceptor.java index 5d1218799..855bde694 100644 --- a/core/src/main/java/feign/ClientInterceptor.java +++ b/core/src/main/java/feign/ClientInterceptor.java @@ -21,7 +21,15 @@ */ public interface ClientInterceptor { - ClientInterceptor DEFAULT = (context, iterator) -> null; + /** + * Returns the default instance of {@link ClientInterceptor}. We don't need to check if the + * iterator has a next entry because the {@link SynchronousMethodHandler} will add 1 entry. That + * means that in the default scenario an iterator to that entry will be passed here. + */ + ClientInterceptor DEFAULT = (context, iterator) -> { + ClientInterceptor next = iterator.next(); + return next.around(context, iterator); + }; /** * Allows to make an around instrumentation. Remember to call the next interceptor by calling From 24ae435117653fa4dddc7f757eadf9ede94fe76d Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Wed, 28 Sep 2022 14:42:20 +0200 Subject: [PATCH 14/22] Reverts enriching of CLientInterceptor to achieve observability --- .../micrometer/MicrometerCapability.java | 21 +------ .../micrometer/AbstractMetricsTestBase.java | 56 +++++++++++-------- .../FeignHeaderInstrumentationTest.java | 3 +- ...eterObservationRegistryCapabilityTest.java | 20 ++++++- 4 files changed, 51 insertions(+), 49 deletions(-) diff --git a/micrometer/src/main/java/feign/micrometer/MicrometerCapability.java b/micrometer/src/main/java/feign/micrometer/MicrometerCapability.java index d2cc61cbf..e4f3d76a5 100644 --- a/micrometer/src/main/java/feign/micrometer/MicrometerCapability.java +++ b/micrometer/src/main/java/feign/micrometer/MicrometerCapability.java @@ -14,10 +14,8 @@ package feign.micrometer; import feign.AsyncClient; -import feign.BaseBuilder; import feign.Capability; import feign.Client; -import feign.ClientInterceptor; import feign.InvocationHandlerFactory; import feign.codec.Decoder; import feign.codec.Encoder; @@ -26,27 +24,18 @@ import io.micrometer.core.instrument.Metrics; import io.micrometer.core.instrument.simple.SimpleConfig; import io.micrometer.core.instrument.simple.SimpleMeterRegistry; -import io.micrometer.observation.ObservationRegistry; public class MicrometerCapability implements Capability { private final MeterRegistry meterRegistry; - private final ObservationRegistry observationRegistry; - public MicrometerCapability() { - this(new SimpleMeterRegistry(SimpleConfig.DEFAULT, Clock.SYSTEM), ObservationRegistry.NOOP); + this(new SimpleMeterRegistry(SimpleConfig.DEFAULT, Clock.SYSTEM)); Metrics.addRegistry(meterRegistry); } public MicrometerCapability(MeterRegistry meterRegistry) { - this(meterRegistry, ObservationRegistry.NOOP); - } - - public MicrometerCapability(MeterRegistry meterRegistry, - ObservationRegistry observationRegistry) { this.meterRegistry = meterRegistry; - this.observationRegistry = observationRegistry; } @Override @@ -54,14 +43,6 @@ public Client enrich(Client client) { return new MeteredClient(client, meterRegistry); } - @Override - public ClientInterceptor enrich(ClientInterceptor clientInterceptor) { - if (!this.observationRegistry.isNoop()) { - return new ObservedClientInterceptor(observationRegistry); - } - return clientInterceptor; - } - @Override public AsyncClient enrich(AsyncClient client) { return new MeteredAsyncClient(client, meterRegistry); diff --git a/micrometer/src/test/java/feign/micrometer/AbstractMetricsTestBase.java b/micrometer/src/test/java/feign/micrometer/AbstractMetricsTestBase.java index a109c829c..c515ddcab 100644 --- a/micrometer/src/test/java/feign/micrometer/AbstractMetricsTestBase.java +++ b/micrometer/src/test/java/feign/micrometer/AbstractMetricsTestBase.java @@ -63,10 +63,10 @@ public final void initializeMetricRegistry() { @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 final void addMetricsCapability() { @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 void clientPropagatesUncheckedException() { 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 void decoderPropagatesUncheckedException() { 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 void decoderPropagatesUncheckedException() { @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 void shouldMetricCollectionWithCustomException() { @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 interface SourceWithPathExpressions { @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 void decoderExceptionCounterHasUriLabelWithPathExpression() { 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 void decoderExceptionCounterHasUriLabelWithPathExpression() { 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 index df94785dc..ad8586faa 100644 --- a/micrometer/src/test/java/feign/micrometer/FeignHeaderInstrumentationTest.java +++ b/micrometer/src/test/java/feign/micrometer/FeignHeaderInstrumentationTest.java @@ -73,8 +73,7 @@ void getTemplatedPathForUri(WireMockRuntimeInfo wmRuntimeInfo) { private TestClient clientInstrumentedWithObservations(String url) { return Feign.builder() .client(new feign.okhttp.OkHttpClient(new OkHttpClient())) - .addCapability(new MicrometerCapability(meterRegistry, observationRegistry)) - .clientInterceptor(ClientInterceptor.DEFAULT) + .clientInterceptor(new ObservedClientInterceptor(this.observationRegistry)) .target(TestClient.class, url); } diff --git a/micrometer/src/test/java/feign/micrometer/MicrometerObservationRegistryCapabilityTest.java b/micrometer/src/test/java/feign/micrometer/MicrometerObservationRegistryCapabilityTest.java index ef6503bad..3b0e0aa19 100644 --- a/micrometer/src/test/java/feign/micrometer/MicrometerObservationRegistryCapabilityTest.java +++ b/micrometer/src/test/java/feign/micrometer/MicrometerObservationRegistryCapabilityTest.java @@ -13,18 +13,32 @@ */ package feign.micrometer; +import feign.AsyncFeign; import feign.Capability; +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(); - @Override - protected Capability createMetricCapability() { + @Before + public void setupRegistry() { observationRegistry.observationConfig() .observationHandler(new DefaultMeterObservationHandler(metricsRegistry)); - return new MicrometerCapability(metricsRegistry, observationRegistry); + } + + @Override + protected Feign.Builder customizeBuilder(Feign.Builder builder) { + return super.customizeBuilder(builder) + .clientInterceptor(new ObservedClientInterceptor(this.observationRegistry)); + } + + @Override + protected AsyncFeign.AsyncBuilder customizeBuilder(AsyncFeign.AsyncBuilder builder) { + return super.customizeBuilder(builder) + .clientInterceptor(new ObservedClientInterceptor(this.observationRegistry)); } } From a2c4323db8280dc3ae6a0e0ac96122116ab66ad1 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 29 Sep 2022 20:33:18 +0000 Subject: [PATCH 15/22] 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](https://github.com/qos-ch/slf4j/compare/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](https://github.com/qos-ch/slf4j/compare/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](https://github.com/qos-ch/slf4j/compare/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](https://github.com/qos-ch/slf4j/compare/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] Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 7e8fefbf6..6d7388637 100644 --- a/pom.xml +++ b/pom.xml @@ -80,7 +80,7 @@ 31.1-jre 1.42.2 2.9.1 - 2.0.2 + 2.0.3 1.70 20220924 From 4b675d7c9f7535050590f05a3c3d75272db45adb Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sat, 1 Oct 2022 02:30:56 +1300 Subject: [PATCH 16/22] 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](https://github.com/JetBrains/kotlin/compare/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](https://github.com/JetBrains/kotlin/compare/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] Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- kotlin/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kotlin/pom.xml b/kotlin/pom.xml index 7307a9550..794d594dd 100644 --- a/kotlin/pom.xml +++ b/kotlin/pom.xml @@ -29,7 +29,7 @@ ${project.basedir}/.. - 1.7.10 + 1.7.20 1.6.4 From 258c464baf493a935026667bdebe88f24c7f253c Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 4 Oct 2022 08:51:05 +1300 Subject: [PATCH 17/22] 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] Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 6d7388637..7a6ed49e0 100644 --- a/pom.xml +++ b/pom.xml @@ -444,7 +444,7 @@ org.ow2.asm asm - 9.3 + 9.4 @@ -512,7 +512,7 @@ org.ow2.asm asm - 9.3 + 9.4 From ba413e8d2047d94400f43281b3abf5b3c97d087f Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Tue, 4 Oct 2022 13:15:42 +0200 Subject: [PATCH 18/22] Applied latest changes of Micrometer --- .../main/java/feign/micrometer/ObservedClientInterceptor.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/micrometer/src/main/java/feign/micrometer/ObservedClientInterceptor.java b/micrometer/src/main/java/feign/micrometer/ObservedClientInterceptor.java index 4a7abd3e0..65edf5369 100644 --- a/micrometer/src/main/java/feign/micrometer/ObservedClientInterceptor.java +++ b/micrometer/src/main/java/feign/micrometer/ObservedClientInterceptor.java @@ -45,7 +45,8 @@ public Response around(ClientInvocationContext context, Iterator feignContext, + this.observationRegistry) .start(); Exception ex = null; Response response = null; From 0421e724d1b94b67431b50e7e2b24d83e108e84a Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Fri, 4 Nov 2022 17:18:50 +0100 Subject: [PATCH 19/22] Polish --- core/src/main/java/feign/ClientInterceptor.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/core/src/main/java/feign/ClientInterceptor.java b/core/src/main/java/feign/ClientInterceptor.java index fc7e3036d..65f67c99d 100644 --- a/core/src/main/java/feign/ClientInterceptor.java +++ b/core/src/main/java/feign/ClientInterceptor.java @@ -82,6 +82,9 @@ default boolean isSync() { } + /** + * {@link WrappedResponse} for async communication. + */ class AsyncResponse implements ClientInterceptor.WrappedResponse { private final CompletableFuture response; @@ -105,6 +108,9 @@ public boolean isAsync() { } } + /** + * {@link WrappedResponse} for sync communication. + */ class SyncResponse implements ClientInterceptor.WrappedResponse { private final Response response; From 91ec1114cb18ca31e2165e2cd312fc79bf232ca3 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Mon, 7 Nov 2022 09:53:00 +0100 Subject: [PATCH 20/22] Upgraded Micrometer to 1.10.0' --- micrometer/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/micrometer/pom.xml b/micrometer/pom.xml index d269b4152..5a6a647f4 100644 --- a/micrometer/pom.xml +++ b/micrometer/pom.xml @@ -27,7 +27,7 @@ ${project.basedir}/.. - 1.10.0-SNAPSHOT + 1.10.0 From 0784382ae1643a9715b0d8c5a9e60bca3a06773b Mon Sep 17 00:00:00 2001 From: Marvin Froeder Date: Thu, 10 Nov 2022 09:36:42 +1300 Subject: [PATCH 21/22] Alternative micrometer observation using capability --- core/src/main/java/feign/AsyncFeign.java | 7 +- .../java/feign/AsynchronousMethodHandler.java | 170 +++++------------- core/src/main/java/feign/BaseBuilder.java | 23 --- core/src/main/java/feign/Capability.java | 4 - .../main/java/feign/ClientInterceptor.java | 136 -------------- .../java/feign/ClientInvocationContext.java | 34 ---- core/src/main/java/feign/Feign.java | 11 -- core/src/main/java/feign/Request.java | 25 ++- core/src/main/java/feign/RequestTemplate.java | 34 ++-- .../java/feign/SynchronousMethodHandler.java | 142 ++++----------- core/src/test/java/feign/BaseBuilderTest.java | 4 +- .../test/java/feign/RequestTemplateTest.java | 20 ++- .../DefaultFeignObservationConvention.java | 9 +- .../java/feign/micrometer/FeignContext.java | 10 +- .../MicrometerObservationCapability.java | 95 ++++++++++ .../micrometer/ObservedClientInterceptor.java | 86 --------- .../FeignHeaderInstrumentationTest.java | 37 ++-- ...eterObservationRegistryCapabilityTest.java | 5 +- 18 files changed, 262 insertions(+), 590 deletions(-) delete mode 100644 core/src/main/java/feign/ClientInterceptor.java delete mode 100644 core/src/main/java/feign/ClientInvocationContext.java create mode 100644 micrometer/src/main/java/feign/micrometer/MicrometerObservationCapability.java delete mode 100644 micrometer/src/main/java/feign/micrometer/ObservedClientInterceptor.java diff --git a/core/src/main/java/feign/AsyncFeign.java b/core/src/main/java/feign/AsyncFeign.java index cf39ee78a..0b0d5b1bf 100644 --- a/core/src/main/java/feign/AsyncFeign.java +++ b/core/src/main/java/feign/AsyncFeign.java @@ -181,11 +181,6 @@ public AsyncBuilder requestInterceptors(Iterable requestI return super.requestInterceptors(requestInterceptors); } - @Override - public AsyncBuilder clientInterceptors(Iterable clientInterceptors) { - return super.clientInterceptors(clientInterceptors); - } - @Override public AsyncBuilder invocationHandlerFactory(InvocationHandlerFactory invocationHandlerFactory) { return super.invocationHandlerFactory(invocationHandlerFactory); @@ -209,7 +204,7 @@ public AsyncFeign build() { final MethodHandler.Factory methodHandlerFactory = new AsynchronousMethodHandler.Factory<>( client, retryer, requestInterceptors, - clientInterceptors, responseHandler, logger, logLevel, + responseHandler, logger, logLevel, propagationPolicy, methodInfoResolver); final ParseHandlersByName handlersByName = new ParseHandlersByName<>(contract, options, encoder, diff --git a/core/src/main/java/feign/AsynchronousMethodHandler.java b/core/src/main/java/feign/AsynchronousMethodHandler.java index b8c78abfb..82466fc15 100644 --- a/core/src/main/java/feign/AsynchronousMethodHandler.java +++ b/core/src/main/java/feign/AsynchronousMethodHandler.java @@ -18,8 +18,6 @@ import feign.codec.Decoder; import feign.codec.ErrorDecoder; import java.io.IOException; -import java.util.ArrayList; -import java.util.Iterator; import java.util.List; import java.util.Optional; import java.util.concurrent.CompletableFuture; @@ -38,7 +36,6 @@ final class AsynchronousMethodHandler implements MethodHandler { private final AsyncClient client; private final Retryer retryer; private final List requestInterceptors; - private final List clientInterceptors; private final Logger logger; private final Logger.Level logLevel; private final RequestTemplate.Factory buildTemplateFromArgs; @@ -50,7 +47,7 @@ final class AsynchronousMethodHandler implements MethodHandler { private AsynchronousMethodHandler(Target target, AsyncClient client, Retryer retryer, - List requestInterceptors, List clientInterceptors, + List requestInterceptors, Logger logger, Logger.Level logLevel, MethodMetadata metadata, RequestTemplate.Factory buildTemplateFromArgs, Options options, AsyncResponseHandler asyncResponseHandler, ExceptionPropagationPolicy propagationPolicy, @@ -61,8 +58,6 @@ private AsynchronousMethodHandler(Target target, AsyncClient client, Retry this.retryer = checkNotNull(retryer, "retryer for %s", target); this.requestInterceptors = checkNotNull(requestInterceptors, "requestInterceptors for %s", target); - this.clientInterceptors = - checkNotNull(clientInterceptors, "clientInterceptors for %s", target); this.logger = checkNotNull(logger, "logger for %s", target); this.logLevel = checkNotNull(logLevel, "logLevel for %s", target); this.metadata = checkNotNull(metadata, "metadata for %s", target); @@ -114,109 +109,6 @@ private CompletableFuture executeAndDecode(RequestTemplate template, return resultFuture; } - static class HttpCall { - private final C requestContext; - private final AsyncClient client; - - private final Target target; - - private final List requestInterceptors; - - private final Logger logger; - - private final Logger.Level logLevel; - - private final MethodMetadata metadata; - - private final long start; - - HttpCall(MethodMetadata metadata, Target target, - List requestInterceptors, Logger logger, Logger.Level logLevel, - AsyncClient client, long start, C requestContext) { - this.requestContext = requestContext; - this.client = client; - this.target = target; - this.requestInterceptors = requestInterceptors; - this.logger = logger; - this.logLevel = logLevel; - this.metadata = metadata; - this.start = start; - } - - ClientInterceptor.WrappedResponse call(RequestTemplate template, Request.Options options) { - Request request = targetRequest(template); - - if (logLevel != Logger.Level.NONE) { - logger.logRequest(metadata.configKey(), logLevel, request); - } - - return new ClientInterceptor.AsyncResponse( - client.execute(request, options, Optional.ofNullable(requestContext)) - .thenApply(response -> { - // TODO: remove in Feign 12 - return ensureRequestIsSet(response, template, request); - }) - .exceptionally(throwable -> { - CompletionException completionException = throwable instanceof CompletionException - ? (CompletionException) throwable - : new CompletionException(throwable); - if (completionException.getCause() instanceof IOException) { - IOException ioException = (IOException) completionException.getCause(); - if (logLevel != Logger.Level.NONE) { - logger.logIOException(metadata.configKey(), logLevel, ioException, - elapsedTime(start)); - } - - throw errorExecuting(request, ioException); - } else { - throw completionException; - } - })); - } - - private Request targetRequest(RequestTemplate template) { - for (RequestInterceptor interceptor : requestInterceptors) { - interceptor.apply(template); - } - return target.apply(template); - } - - private long elapsedTime(long start) { - return TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start); - } - - } - - private static class HttpCallingInterceptor implements ClientInterceptor { - - private final HttpCall httpCall; - - HttpCallingInterceptor(HttpCall httpCall) { - this.httpCall = httpCall; - } - - @Override - public WrappedResponse around(ClientInvocationContext context, - Iterator iterator) { - return httpCall.call(context.getRequestTemplate(), context.getOptions()); - } - } - - private static class InterceptorChain { - private final List interceptors; - - InterceptorChain(List interceptors, HttpCall httpCall) { - this.interceptors = new ArrayList<>(interceptors); - this.interceptors.add(new HttpCallingInterceptor(httpCall)); - } - - ClientInterceptor.WrappedResponse call(ClientInvocationContext context) { - Iterator iterator = this.interceptors.iterator(); - ClientInterceptor next = iterator.next(); - return next.around(context, iterator); - } - } - private static class CancellableFuture extends CompletableFuture { private CompletableFuture inner = null; @@ -276,25 +168,36 @@ private boolean shouldRetry(Retryer retryer, } } - @SuppressWarnings("unchecked") private CompletableFuture executeAndDecode(RequestTemplate template, Options options) { - long start = System.nanoTime(); - ClientInvocationContext context = new ClientInvocationContext(template, options); - InterceptorChain interceptorChain = - new InterceptorChain(this.clientInterceptors, new HttpCall<>(this.metadata, this.target, - this.requestInterceptors, this.logger, this.logLevel, this.client, start, - requestContext)); - ClientInterceptor.WrappedResponse interceptedResponse = interceptorChain.call(context); - if (!interceptedResponse.isAsync()) { - throw new IllegalStateException("You're trying to use the sync version in an async context"); + Request request = targetRequest(template); + + if (logLevel != Logger.Level.NONE) { + logger.logRequest(metadata.configKey(), logLevel, request); } - CompletableFuture completableFuture = interceptedResponse.unwrapAsync(); - return completableFuture - .thenCompose(response -> handleResponse(response, elapsedTime(start))); - } - private long elapsedTime(long start) { - return TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start); + long start = System.nanoTime(); + return client.execute(request, options, Optional.ofNullable(requestContext)) + .thenApply(response -> { + // TODO: remove in Feign 12 + return ensureRequestIsSet(response, template, request); + }) + .exceptionally(throwable -> { + CompletionException completionException = throwable instanceof CompletionException + ? (CompletionException) throwable + : new CompletionException(throwable); + if (completionException.getCause() instanceof IOException) { + IOException ioException = (IOException) completionException.getCause(); + if (logLevel != Logger.Level.NONE) { + logger.logIOException(metadata.configKey(), logLevel, ioException, + elapsedTime(start)); + } + + throw errorExecuting(request, ioException); + } else { + throw completionException; + } + }) + .thenCompose(response -> handleResponse(response, elapsedTime(start))); } private static Response ensureRequestIsSet(Response response, @@ -311,6 +214,17 @@ private CompletableFuture handleResponse(Response response, long elapsed metadata.configKey(), response, methodInfo.underlyingReturnType(), elapsedTime); } + private long elapsedTime(long start) { + return TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start); + } + + private Request targetRequest(RequestTemplate template) { + for (RequestInterceptor interceptor : requestInterceptors) { + interceptor.apply(template); + } + return target.apply(template); + } + private Options findOptions(Object[] argv) { if (argv == null || argv.length == 0) { return this.options; @@ -327,8 +241,6 @@ static class Factory implements MethodHandler.Factory { private final AsyncClient client; private final Retryer retryer; private final List requestInterceptors; - - private final List clientInterceptors; private final AsyncResponseHandler responseHandler; private final Logger logger; private final Logger.Level logLevel; @@ -336,14 +248,13 @@ static class Factory implements MethodHandler.Factory { private final MethodInfoResolver methodInfoResolver; Factory(AsyncClient client, Retryer retryer, List requestInterceptors, - List clientInterceptors, AsyncResponseHandler responseHandler, + AsyncResponseHandler responseHandler, Logger logger, Logger.Level logLevel, ExceptionPropagationPolicy propagationPolicy, MethodInfoResolver methodInfoResolver) { this.client = checkNotNull(client, "client"); this.retryer = checkNotNull(retryer, "retryer"); this.requestInterceptors = checkNotNull(requestInterceptors, "requestInterceptors"); - this.clientInterceptors = clientInterceptors; this.responseHandler = responseHandler; this.logger = checkNotNull(logger, "logger"); this.logLevel = checkNotNull(logLevel, "logLevel"); @@ -359,7 +270,6 @@ public MethodHandler create(Target target, ErrorDecoder errorDecoder, C requestContext) { return new AsynchronousMethodHandler(target, client, retryer, requestInterceptors, - clientInterceptors, logger, logLevel, md, buildTemplateFromArgs, options, responseHandler, propagationPolicy, requestContext, methodInfoResolver.resolve(target.type(), md.method())); diff --git a/core/src/main/java/feign/BaseBuilder.java b/core/src/main/java/feign/BaseBuilder.java index d06712b7b..ae12d4f9d 100644 --- a/core/src/main/java/feign/BaseBuilder.java +++ b/core/src/main/java/feign/BaseBuilder.java @@ -27,7 +27,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Objects; -import java.util.concurrent.CompletableFuture; import java.util.stream.Collectors; public abstract class BaseBuilder> { @@ -36,8 +35,6 @@ public abstract class BaseBuilder> { protected final List requestInterceptors = new ArrayList<>(); - protected final List clientInterceptors = - new ArrayList<>(); protected ResponseInterceptor responseInterceptor = ResponseInterceptor.DEFAULT; protected Logger.Level logLevel = Logger.Level.NONE; protected Contract contract = new Contract.Default(); @@ -200,26 +197,6 @@ public B requestInterceptors(Iterable requestInterceptors) { return thisB; } - /** - * Adds a single client interceptor to the builder. - */ - public B clientInterceptor(ClientInterceptor clientInterceptor) { - this.clientInterceptors.add(clientInterceptor); - return thisB; - } - - /** - * Sets the full set of request interceptors for the builder, overwriting any previous - * interceptors. - */ - public B clientInterceptors(Iterable clientInterceptors) { - this.clientInterceptors.clear(); - for (ClientInterceptor clientInterceptor : clientInterceptors) { - this.clientInterceptors.add(clientInterceptor); - } - return thisB; - } - /** * Adds a single response interceptor to the builder. */ diff --git a/core/src/main/java/feign/Capability.java b/core/src/main/java/feign/Capability.java index d14497622..0b71a9f08 100644 --- a/core/src/main/java/feign/Capability.java +++ b/core/src/main/java/feign/Capability.java @@ -86,10 +86,6 @@ default RequestInterceptor enrich(RequestInterceptor requestInterceptor) { return requestInterceptor; } - default ClientInterceptor enrich(ClientInterceptor clientInterceptor) { - return clientInterceptor; - } - default ResponseInterceptor enrich(ResponseInterceptor responseInterceptor) { return responseInterceptor; } diff --git a/core/src/main/java/feign/ClientInterceptor.java b/core/src/main/java/feign/ClientInterceptor.java deleted file mode 100644 index 65f67c99d..000000000 --- a/core/src/main/java/feign/ClientInterceptor.java +++ /dev/null @@ -1,136 +0,0 @@ -/* - * 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; - -import java.util.Iterator; -import java.util.concurrent.CompletableFuture; - -/** - * Zero or One {@code ClientInterceptor} may be configured for purposes such as tracing - mutate - * headers, execute the call, parse the response and close any previously created objects. - */ -public interface ClientInterceptor { - - /** - * Returns the default instance of {@link ClientInterceptor}. We don't need to check if the - * iterator has a next entry because the {@link SynchronousMethodHandler} will add 1 entry. That - * means that in the default scenario an iterator to that entry will be passed here. - */ - ClientInterceptor DEFAULT = (context, iterator) -> { - ClientInterceptor next = iterator.next(); - return next.around(context, iterator); - }; - - /** - * Allows to make an around instrumentation. Remember to call the next interceptor by calling - * {@code ClientInterceptor interceptor = iterator.next(); return interceptor.around(context, iterator)}. - * - * @param context input context to send a request - * @param iterator iterator to the next {@link ClientInterceptor} - * @return response or an exception - remember to rethrow an exception if it occurrs - * @throws FeignException exception while trying to send a request - */ - WrappedResponse around(ClientInvocationContext context, Iterator iterator) - throws FeignException; - - /** - * Wrapper around a response. Used in both synchronous and asynchronous communication. For - * synchronous the unwrapped object must be of type {@link Response}. For asynchronous the - * unwrapped object must be of type {@code CompletableFuture} - */ - interface WrappedResponse { - /** - * Unwraps the actual response type for sync communication. - * - * @return the actual response type for sync - */ - Response unwrapSync(); - - /** - * Unwraps the actual response type for async communications. - * - * @return the actual response type for async - */ - CompletableFuture unwrapAsync(); - - /** - * Returns {@code true} for async communication. - * - * @return {@code true} for async communication - */ - boolean isAsync(); - - /** - * Returns {@code true} for sync communication. - * - * @return {@code true} for sync communication - */ - default boolean isSync() { - return !isAsync(); - } - - } - - /** - * {@link WrappedResponse} for async communication. - */ - class AsyncResponse implements ClientInterceptor.WrappedResponse { - private final CompletableFuture response; - - public AsyncResponse(CompletableFuture response) { - this.response = response; - } - - @Override - public Response unwrapSync() { - throw new UnsupportedOperationException("AsyncResponse must be used only in async scope"); - } - - @Override - public CompletableFuture unwrapAsync() { - return this.response; - } - - @Override - public boolean isAsync() { - return true; - } - } - - /** - * {@link WrappedResponse} for sync communication. - */ - class SyncResponse implements ClientInterceptor.WrappedResponse { - private final Response response; - - public SyncResponse(Response response) { - this.response = response; - } - - @Override - public Response unwrapSync() { - return this.response; - } - - @Override - public CompletableFuture unwrapAsync() { - throw new UnsupportedOperationException("SyncResponse must be used only in sync scope"); - } - - @Override - public boolean isAsync() { - return false; - } - } -} diff --git a/core/src/main/java/feign/ClientInvocationContext.java b/core/src/main/java/feign/ClientInvocationContext.java deleted file mode 100644 index b340d7ff0..000000000 --- a/core/src/main/java/feign/ClientInvocationContext.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - * 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; - -public class ClientInvocationContext { - - private final RequestTemplate requestTemplate; - - private final Request.Options options; - - public ClientInvocationContext(RequestTemplate requestTemplate, Request.Options options) { - this.requestTemplate = requestTemplate; - this.options = options; - } - - public RequestTemplate getRequestTemplate() { - return requestTemplate; - } - - public Request.Options getOptions() { - return options; - } -} diff --git a/core/src/main/java/feign/Feign.java b/core/src/main/java/feign/Feign.java index f3fb318a1..bb28fef8a 100644 --- a/core/src/main/java/feign/Feign.java +++ b/core/src/main/java/feign/Feign.java @@ -169,16 +169,6 @@ public Builder requestInterceptors(Iterable requestIntercept return super.requestInterceptors(requestInterceptors); } - @Override - public Builder clientInterceptor(ClientInterceptor clientInterceptor) { - return super.clientInterceptor(clientInterceptor); - } - - @Override - public Builder clientInterceptors(Iterable clientInterceptors) { - return super.clientInterceptors(clientInterceptors); - } - @Override public Builder invocationHandlerFactory(InvocationHandlerFactory invocationHandlerFactory) { return super.invocationHandlerFactory(invocationHandlerFactory); @@ -212,7 +202,6 @@ public Feign build() { MethodHandler.Factory synchronousMethodHandlerFactory = new SynchronousMethodHandler.Factory(client, retryer, requestInterceptors, - clientInterceptors, responseInterceptor, logger, logLevel, dismiss404, closeAfterDecode, propagationPolicy); ParseHandlersByName handlersByName = diff --git a/core/src/main/java/feign/Request.java b/core/src/main/java/feign/Request.java index a974e467c..31ae5b8d4 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 Map> 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 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 9014430fd..20bbb8b66 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 @@ private RequestTemplate appendHeader(String name, Iterable 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> 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> 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/main/java/feign/SynchronousMethodHandler.java b/core/src/main/java/feign/SynchronousMethodHandler.java index 13b9351f0..2d074f5b6 100644 --- a/core/src/main/java/feign/SynchronousMethodHandler.java +++ b/core/src/main/java/feign/SynchronousMethodHandler.java @@ -21,10 +21,7 @@ import feign.codec.Decoder; import feign.codec.ErrorDecoder; import java.io.IOException; -import java.util.ArrayList; -import java.util.Iterator; import java.util.List; -import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; import java.util.stream.Stream; @@ -35,7 +32,6 @@ final class SynchronousMethodHandler implements MethodHandler { private final Client client; private final Retryer retryer; private final List requestInterceptors; - private final List clientInterceptors; private final Logger logger; private final Logger.Level logLevel; private final RequestTemplate.Factory buildTemplateFromArgs; @@ -45,8 +41,7 @@ final class SynchronousMethodHandler implements MethodHandler { private SynchronousMethodHandler(Target target, Client client, Retryer retryer, - List requestInterceptors, List clientInterceptors, - ResponseInterceptor responseInterceptor, + List requestInterceptors, ResponseInterceptor responseInterceptor, Logger logger, Logger.Level logLevel, MethodMetadata metadata, RequestTemplate.Factory buildTemplateFromArgs, Options options, Decoder decoder, ErrorDecoder errorDecoder, boolean dismiss404, @@ -57,8 +52,6 @@ private SynchronousMethodHandler(Target target, Client client, Retryer retrye this.retryer = checkNotNull(retryer, "retryer for %s", target); this.requestInterceptors = checkNotNull(requestInterceptors, "requestInterceptors for %s", target); - this.clientInterceptors = - checkNotNull(clientInterceptors, "clientInterceptors for %s", target); this.logger = checkNotNull(logger, "logger for %s", target); this.logLevel = checkNotNull(logLevel, "logLevel for %s", target); this.metadata = checkNotNull(metadata, "metadata for %s", target); @@ -97,18 +90,42 @@ public Object invoke(Object[] argv) throws Throwable { } Object executeAndDecode(RequestTemplate template, Options options) throws Throwable { - ClientInvocationContext context = new ClientInvocationContext(template, options); + Request request = targetRequest(template); + + if (logLevel != Logger.Level.NONE) { + logger.logRequest(metadata.configKey(), logLevel, request); + } + + Response response; long start = System.nanoTime(); - InterceptorChain interceptorChain = - new InterceptorChain(this.clientInterceptors, new HttpCall(this.metadata, this.target, - this.requestInterceptors, this.logger, this.logLevel, this.client, start)); - ClientInterceptor.WrappedResponse wrappedResponse = interceptorChain.call(context); - if (wrappedResponse.isAsync()) { - throw new IllegalStateException("You're trying to use the async version in a sync context"); + try { + response = client.execute(request, options); + // ensure the request is set. TODO: remove in Feign 12 + response = response.toBuilder() + .request(request) + .requestTemplate(template) + .build(); + } catch (IOException e) { + if (logLevel != Logger.Level.NONE) { + logger.logIOException(metadata.configKey(), logLevel, e, elapsedTime(start)); + } + throw errorExecuting(request, e); } + long elapsedTime = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start); return responseHandler.handleResponse( - metadata.configKey(), wrappedResponse.unwrapSync(), metadata.returnType(), elapsedTime); + metadata.configKey(), response, metadata.returnType(), elapsedTime); + } + + long elapsedTime(long start) { + return TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start); + } + + Request targetRequest(RequestTemplate template) { + for (RequestInterceptor interceptor : requestInterceptors) { + interceptor.apply(template); + } + return target.apply(template); } Options findOptions(Object[] argv) { @@ -122,100 +139,11 @@ Options findOptions(Object[] argv) { .orElse(this.options); } - - static class HttpCall { - private final MethodMetadata metadata; - private final Target target; - private final List requestInterceptors; - private final Logger logger; - private final Logger.Level logLevel; - private final Client client; - - private final long start; - - HttpCall(MethodMetadata metadata, Target target, - List requestInterceptors, Logger logger, Logger.Level logLevel, - Client client, long start) { - this.metadata = metadata; - this.target = target; - this.requestInterceptors = requestInterceptors; - this.logger = logger; - this.logLevel = logLevel; - this.client = client; - this.start = start; - } - - Response call(RequestTemplate template, Request.Options options) { - Request request = targetRequest(template); - - if (logLevel != Logger.Level.NONE) { - logger.logRequest(metadata.configKey(), logLevel, request); - } - - Response response; - try { - response = client.execute(request, options); - // ensure the request is set. TODO: remove in Feign 12 - return response.toBuilder() - .request(request) - .requestTemplate(template) - .build(); - } catch (IOException e) { - if (logLevel != Logger.Level.NONE) { - logger.logIOException(metadata.configKey(), logLevel, e, elapsedTime(start)); - } - throw errorExecuting(request, e); - } - } - - Request targetRequest(RequestTemplate template) { - for (RequestInterceptor interceptor : requestInterceptors) { - interceptor.apply(template); - } - return target.apply(template); - } - - long elapsedTime(long start) { - return TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start); - } - } - - private static class HttpCallingInterceptor implements ClientInterceptor { - - private final HttpCall httpCall; - - HttpCallingInterceptor(HttpCall httpCall) { - this.httpCall = httpCall; - } - - @Override - public WrappedResponse around(ClientInvocationContext context, - Iterator iterator) { - return new SyncResponse(httpCall.call(context.getRequestTemplate(), context.getOptions())); - } - } - - private static class InterceptorChain { - private final List interceptors; - - InterceptorChain(List interceptors, HttpCall httpCall) { - this.interceptors = new ArrayList<>(interceptors); - this.interceptors.add(new HttpCallingInterceptor(httpCall)); - } - - ClientInterceptor.WrappedResponse call(ClientInvocationContext context) { - Iterator iterator = this.interceptors.iterator(); - ClientInterceptor next = iterator.next(); - return next.around(context, iterator); - } - } - static class Factory implements MethodHandler.Factory { private final Client client; private final Retryer retryer; private final List requestInterceptors; - private final List clientInterceptors; private final ResponseInterceptor responseInterceptor; private final Logger logger; private final Logger.Level logLevel; @@ -224,13 +152,12 @@ static class Factory implements MethodHandler.Factory { private final ExceptionPropagationPolicy propagationPolicy; Factory(Client client, Retryer retryer, List requestInterceptors, - List clientInterceptors, ResponseInterceptor responseInterceptor, + ResponseInterceptor responseInterceptor, Logger logger, Logger.Level logLevel, boolean dismiss404, boolean closeAfterDecode, ExceptionPropagationPolicy propagationPolicy) { this.client = checkNotNull(client, "client"); this.retryer = checkNotNull(retryer, "retryer"); this.requestInterceptors = checkNotNull(requestInterceptors, "requestInterceptors"); - this.clientInterceptors = checkNotNull(clientInterceptors, "clientInterceptors"); this.responseInterceptor = responseInterceptor; this.logger = checkNotNull(logger, "logger"); this.logLevel = checkNotNull(logLevel, "logLevel"); @@ -247,7 +174,6 @@ public MethodHandler create(Target target, ErrorDecoder errorDecoder, Object requestContext) { return new SynchronousMethodHandler(target, client, retryer, requestInterceptors, - clientInterceptors, responseInterceptor, logger, logLevel, md, buildTemplateFromArgs, options, decoder, errorDecoder, dismiss404, closeAfterDecode, propagationPolicy); } diff --git a/core/src/test/java/feign/BaseBuilderTest.java b/core/src/test/java/feign/BaseBuilderTest.java index d1c8434d9..9599fd98f 100644 --- a/core/src/test/java/feign/BaseBuilderTest.java +++ b/core/src/test/java/feign/BaseBuilderTest.java @@ -27,7 +27,7 @@ public class BaseBuilderTest { public void checkEnrichTouchesAllAsyncBuilderFields() throws IllegalArgumentException, IllegalAccessException { test(AsyncFeign.builder().requestInterceptor(template -> { - }).clientInterceptor(ClientInterceptor.DEFAULT), 15); + }), 14); } private void test(BaseBuilder builder, int expectedFieldsCount) @@ -54,7 +54,7 @@ private void test(BaseBuilder builder, int expectedFieldsCount) public void checkEnrichTouchesAllBuilderFields() throws IllegalArgumentException, IllegalAccessException { test(Feign.builder().requestInterceptor(template -> { - }).clientInterceptor(ClientInterceptor.DEFAULT), 13); + }), 12); } } diff --git a/core/src/test/java/feign/RequestTemplateTest.java b/core/src/test/java/feign/RequestTemplateTest.java index dbfccdd3a..1becf347b 100644 --- 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/src/main/java/feign/micrometer/DefaultFeignObservationConvention.java b/micrometer/src/main/java/feign/micrometer/DefaultFeignObservationConvention.java index 3ec33c6b6..9a29f50aa 100644 --- a/micrometer/src/main/java/feign/micrometer/DefaultFeignObservationConvention.java +++ b/micrometer/src/main/java/feign/micrometer/DefaultFeignObservationConvention.java @@ -13,7 +13,7 @@ */ package feign.micrometer; -import feign.RequestTemplate; +import feign.Request; import feign.Response; import io.micrometer.common.KeyValues; import io.micrometer.common.lang.Nullable; @@ -48,7 +48,7 @@ public String getContextualName(FeignContext context) { @Override public KeyValues getLowCardinalityKeyValues(FeignContext context) { - String templatedUrl = context.getCarrier().methodMetadata().template().url(); + String templatedUrl = context.getCarrier().requestTemplate().methodMetadata().template().url(); return KeyValues.of( FeignObservationDocumentation.HttpClientTags.METHOD .withValue(getMethodString(context.getCarrier())), @@ -62,9 +62,8 @@ String getStatusValue(@Nullable Response response) { return response != null ? String.valueOf(response.status()) : "CLIENT_ERROR"; } - String getMethodString(@Nullable RequestTemplate request) { - return request != null && request.method() != null ? request.method() - : "UNKNOWN"; + String getMethodString(@Nullable Request request) { + return request.httpMethod().name(); } } diff --git a/micrometer/src/main/java/feign/micrometer/FeignContext.java b/micrometer/src/main/java/feign/micrometer/FeignContext.java index a2a212925..88aa50fdb 100644 --- a/micrometer/src/main/java/feign/micrometer/FeignContext.java +++ b/micrometer/src/main/java/feign/micrometer/FeignContext.java @@ -13,7 +13,7 @@ */ package feign.micrometer; -import feign.RequestTemplate; +import feign.Request; import feign.Response; import io.micrometer.observation.transport.RequestReplySenderContext; import io.micrometer.observation.transport.SenderContext; @@ -24,11 +24,11 @@ * @author Marcin Grzejszczak * @since 1.10.0 */ -public class FeignContext extends RequestReplySenderContext { +public class FeignContext extends RequestReplySenderContext { - public FeignContext(RequestTemplate requestTemplate) { - super((carrier, key, value) -> carrier.header(key, value)); - setCarrier(requestTemplate); + public FeignContext(Request request) { + super(Request::header); + setCarrier(request); } } 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 000000000..927691597 --- /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/main/java/feign/micrometer/ObservedClientInterceptor.java b/micrometer/src/main/java/feign/micrometer/ObservedClientInterceptor.java deleted file mode 100644 index 6622b35ac..000000000 --- a/micrometer/src/main/java/feign/micrometer/ObservedClientInterceptor.java +++ /dev/null @@ -1,86 +0,0 @@ -/* - * 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 java.util.Iterator; -import java.util.concurrent.CompletableFuture; -import feign.Client; -import feign.ClientInterceptor; -import feign.ClientInvocationContext; -import feign.FeignException; -import feign.Response; -import io.micrometer.observation.Observation; -import io.micrometer.observation.ObservationRegistry; - -/** Warp feign {@link Client} with metrics. */ -public class ObservedClientInterceptor implements ClientInterceptor { - - private final ObservationRegistry observationRegistry; - - private final FeignObservationConvention customFeignObservationConvention; - - public ObservedClientInterceptor(ObservationRegistry observationRegistry, - FeignObservationConvention customFeignObservationConvention) { - this.observationRegistry = observationRegistry; - this.customFeignObservationConvention = customFeignObservationConvention; - } - - public ObservedClientInterceptor(ObservationRegistry observationRegistry) { - this(observationRegistry, null); - } - - @Override - public WrappedResponse around(ClientInvocationContext context, - Iterator iterator) - throws FeignException { - FeignContext feignContext = new FeignContext(context.getRequestTemplate()); - Observation observation = FeignObservationDocumentation.DEFAULT - .observation(this.customFeignObservationConvention, - DefaultFeignObservationConvention.INSTANCE, () -> feignContext, - this.observationRegistry) - .start(); - Exception ex = null; - WrappedResponse response = null; - try { - ClientInterceptor interceptor = iterator.next(); - response = interceptor.around(context, iterator); - if (response.isSync()) { - return response; - } - CompletableFuture asyncResponse = response.unwrapAsync(); - return new AsyncResponse(asyncResponse.handle((res, throwable) -> { - finalizeObservation(feignContext, observation, throwable, res); - return res; - })); - } catch (FeignException exception) { - ex = exception; - throw exception; - } finally { - if (response != null && response.isSync()) { - finalizeObservation(feignContext, observation, ex, response.unwrapSync()); - } - } - } - - 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/FeignHeaderInstrumentationTest.java b/micrometer/src/test/java/feign/micrometer/FeignHeaderInstrumentationTest.java index 459ac5f01..d19f88e1c 100644 --- a/micrometer/src/test/java/feign/micrometer/FeignHeaderInstrumentationTest.java +++ b/micrometer/src/test/java/feign/micrometer/FeignHeaderInstrumentationTest.java @@ -13,17 +13,22 @@ */ package feign.micrometer; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeUnit; +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.ClientInterceptor; import feign.Feign; import feign.Param; +import feign.Request; import feign.RequestLine; -import feign.RequestTemplate; import feign.Response; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Timer; @@ -33,18 +38,12 @@ 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; -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; @WireMockTest class FeignHeaderInstrumentationTest { @@ -91,14 +90,14 @@ void getTemplatedPathForUriForAsync(WireMockRuntimeInfo wmRuntimeInfo) private TestClient clientInstrumentedWithObservations(String url) { return Feign.builder() .client(new feign.okhttp.OkHttpClient(new OkHttpClient())) - .clientInterceptor(new ObservedClientInterceptor(this.observationRegistry)) + .addCapability(new MicrometerObservationCapability(this.observationRegistry)) .target(TestClient.class, url); } private AsyncTestClient asyncClientInstrumentedWithObservations(String url) { return AsyncFeign.builder() .client(new feign.okhttp.OkHttpClient(new OkHttpClient())) - .clientInterceptor(new ObservedClientInterceptor(this.observationRegistry)) + .addCapability(new MicrometerObservationCapability(this.observationRegistry)) .target(AsyncTestClient.class, url); } @@ -116,11 +115,11 @@ CompletableFuture templated(@Param("customerId") String customerId, } static class HeaderMutatingHandler - implements ObservationHandler> { + implements ObservationHandler> { @Override - public void onStart(RequestReplySenderContext context) { - RequestTemplate carrier = context.getCarrier(); + public void onStart(RequestReplySenderContext context) { + Request carrier = context.getCarrier(); carrier.header("foo", "bar"); } diff --git a/micrometer/src/test/java/feign/micrometer/MicrometerObservationRegistryCapabilityTest.java b/micrometer/src/test/java/feign/micrometer/MicrometerObservationRegistryCapabilityTest.java index 3b0e0aa19..7170f26b4 100644 --- a/micrometer/src/test/java/feign/micrometer/MicrometerObservationRegistryCapabilityTest.java +++ b/micrometer/src/test/java/feign/micrometer/MicrometerObservationRegistryCapabilityTest.java @@ -14,7 +14,6 @@ package feign.micrometer; import feign.AsyncFeign; -import feign.Capability; import feign.Feign; import io.micrometer.core.instrument.observation.DefaultMeterObservationHandler; import io.micrometer.observation.ObservationRegistry; @@ -33,12 +32,12 @@ public void setupRegistry() { @Override protected Feign.Builder customizeBuilder(Feign.Builder builder) { return super.customizeBuilder(builder) - .clientInterceptor(new ObservedClientInterceptor(this.observationRegistry)); + .addCapability(new MicrometerObservationCapability(this.observationRegistry)); } @Override protected AsyncFeign.AsyncBuilder customizeBuilder(AsyncFeign.AsyncBuilder builder) { return super.customizeBuilder(builder) - .clientInterceptor(new ObservedClientInterceptor(this.observationRegistry)); + .addCapability(new MicrometerObservationCapability(this.observationRegistry)); } } From 80f2e77e065adedb9ec538465f91081a588821e5 Mon Sep 17 00:00:00 2001 From: Marvin Froeder Date: Thu, 10 Nov 2022 09:52:56 +1300 Subject: [PATCH 22/22] Ban 'repositories' --- core/pom.xml | 1 - micrometer/pom.xml | 19 ------------------- pom.xml | 20 ++++++++++++++++++++ 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/core/pom.xml b/core/pom.xml index f32245e5f..de3bb11cc 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/micrometer/pom.xml b/micrometer/pom.xml index 5a6a647f4..66aceda74 100644 --- a/micrometer/pom.xml +++ b/micrometer/pom.xml @@ -84,23 +84,4 @@ - - - - spring-snapshots - Spring Snapshots - https://repo.spring.io/snapshot - - true - - - - spring-milestones - Spring Milestones - https://repo.spring.io/milestone - - false - - - diff --git a/pom.xml b/pom.xml index 6b951ee04..9df22d8af 100644 --- a/pom.xml +++ b/pom.xml @@ -761,6 +761,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 + + + + + +