Skip to content

Commit a3d82f8

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

14 files changed

+614
-45
lines changed

core/pom.xml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,6 @@
8686
<plugin>
8787
<groupId>org.apache.maven.plugins</groupId>
8888
<artifactId>maven-enforcer-plugin</artifactId>
89-
<version>3.1.0</version>
9089
<executions>
9190
<execution>
9291
<id>enforce-banned-dependencies</id>

core/src/main/java/feign/Request.java

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,17 @@
1313
*/
1414
package feign;
1515

16+
import static feign.Util.checkNotNull;
17+
import static feign.Util.valuesOrEmpty;
1618
import java.io.Serializable;
1719
import java.net.HttpURLConnection;
1820
import java.nio.charset.Charset;
21+
import java.util.Arrays;
1922
import java.util.Collection;
2023
import java.util.Collections;
2124
import java.util.Map;
2225
import java.util.Optional;
2326
import java.util.concurrent.TimeUnit;
24-
import static feign.Util.checkNotNull;
25-
import static feign.Util.valuesOrEmpty;
2627

2728
/**
2829
* An immutable request to an http server.
@@ -206,6 +207,26 @@ public Map<String, Collection<String>> headers() {
206207
return Collections.unmodifiableMap(headers);
207208
}
208209

210+
/**
211+
* Add new entries to request Headers. It overrides existing entries
212+
*
213+
* @param key
214+
* @param value
215+
*/
216+
public void header(String key, String value) {
217+
header(key, Arrays.asList(value));
218+
}
219+
220+
/**
221+
* Add new entries to request Headers. It overrides existing entries
222+
*
223+
* @param key
224+
* @param values
225+
*/
226+
public void header(String key, Collection<String> values) {
227+
headers.put(key, values);
228+
}
229+
209230
/**
210231
* Charset of the request.
211232
*

core/src/main/java/feign/RequestTemplate.java

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,32 @@
1313
*/
1414
package feign;
1515

16+
import static feign.Util.CONTENT_LENGTH;
17+
import static feign.Util.checkNotNull;
1618
import feign.Request.HttpMethod;
17-
import feign.template.*;
19+
import feign.template.BodyTemplate;
20+
import feign.template.HeaderTemplate;
21+
import feign.template.QueryTemplate;
22+
import feign.template.UriTemplate;
23+
import feign.template.UriUtils;
1824
import java.io.Serializable;
1925
import java.net.URI;
2026
import java.nio.charset.Charset;
2127
import java.util.AbstractMap.SimpleImmutableEntry;
22-
import java.util.*;
28+
import java.util.ArrayList;
29+
import java.util.Arrays;
30+
import java.util.Collection;
31+
import java.util.Collections;
32+
import java.util.Iterator;
33+
import java.util.LinkedHashMap;
34+
import java.util.LinkedHashSet;
35+
import java.util.List;
36+
import java.util.Map;
2337
import java.util.Map.Entry;
38+
import java.util.TreeMap;
2439
import java.util.regex.Matcher;
2540
import java.util.regex.Pattern;
2641
import java.util.stream.Collectors;
27-
import static feign.Util.*;
2842

2943
/**
3044
* Request Builder for an HTTP Target.
@@ -764,12 +778,10 @@ private RequestTemplate appendHeader(String name, Iterable<String> values, boole
764778
} else {
765779
return HeaderTemplate.create(headerName, values);
766780
}
781+
} else if (literal) {
782+
return HeaderTemplate.appendLiteral(headerTemplate, values);
767783
} else {
768-
if (literal) {
769-
return HeaderTemplate.appendLiteral(headerTemplate, values);
770-
} else {
771-
return HeaderTemplate.append(headerTemplate, values);
772-
}
784+
return HeaderTemplate.append(headerTemplate, values);
773785
}
774786
});
775787
return this;
@@ -791,7 +803,7 @@ public RequestTemplate headers(Map<String, Collection<String>> headers) {
791803
}
792804

793805
/**
794-
* Returns an immutable copy of the Headers for this request.
806+
* Returns an copy of the Headers for this request.
795807
*
796808
* @return the currently applied headers.
797809
*/
@@ -802,10 +814,10 @@ public Map<String, Collection<String>> headers() {
802814

803815
/* add the expanded collection, but only if it has values */
804816
if (!values.isEmpty()) {
805-
headerMap.put(key, Collections.unmodifiableList(values));
817+
headerMap.put(key, values);
806818
}
807819
});
808-
return Collections.unmodifiableMap(headerMap);
820+
return headerMap;
809821
}
810822

811823
/**

core/src/test/java/feign/RequestTemplateTest.java

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,13 @@
1515

1616
import static feign.assertj.FeignAssertions.assertThat;
1717
import static java.util.Arrays.asList;
18+
import static org.assertj.core.api.Assertions.assertThat;
1819
import static org.assertj.core.data.MapEntry.entry;
1920
import static org.junit.Assert.assertEquals;
2021
import static org.junit.Assert.assertNotNull;
2122
import static org.junit.Assert.assertTrue;
23+
import feign.Request.HttpMethod;
24+
import feign.template.UriUtils;
2225
import java.util.Arrays;
2326
import java.util.Collection;
2427
import java.util.Collections;
@@ -27,8 +30,6 @@
2730
import org.junit.Rule;
2831
import org.junit.Test;
2932
import org.junit.rules.ExpectedException;
30-
import feign.Request.HttpMethod;
31-
import feign.template.UriUtils;
3233

3334
public class RequestTemplateTest {
3435

@@ -472,16 +473,25 @@ public void shouldRetrieveHeadersWithoutNull() {
472473

473474
}
474475

475-
@SuppressWarnings("ConstantConditions")
476-
@Test(expected = UnsupportedOperationException.class)
477-
public void shouldNotInsertHeadersImmutableMap() {
476+
public void shouldNotMutateInternalHeadersMap() {
478477
RequestTemplate template = new RequestTemplate()
479478
.header("key1", "valid");
480479

481480
assertThat(template.headers()).hasSize(1);
482481
assertThat(template.headers().keySet()).containsExactly("key1");
482+
assertThat(template.headers().get("key1")).containsExactly("valid");
483483

484484
template.headers().put("key2", Collections.singletonList("other value"));
485+
// nothing should change
486+
assertThat(template.headers()).hasSize(1);
487+
assertThat(template.headers().keySet()).containsExactly("key1");
488+
assertThat(template.headers().get("key1")).containsExactly("valid");
489+
490+
template.headers().get("key1").add("value2");
491+
// nothing should change either
492+
assertThat(template.headers()).hasSize(1);
493+
assertThat(template.headers().keySet()).containsExactly("key1");
494+
assertThat(template.headers().get("key1")).containsExactly("valid");
485495
}
486496

487497
@Test

micrometer/pom.xml

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
<properties>
2929
<main.basedir>${project.basedir}/..</main.basedir>
30+
<micrometer.version>1.10.0</micrometer.version>
3031
</properties>
3132

3233
<dependencies>
@@ -42,19 +43,31 @@
4243
<dependency>
4344
<groupId>io.micrometer</groupId>
4445
<artifactId>micrometer-core</artifactId>
45-
<version>1.10.0</version>
46+
<version>${micrometer.version}</version>
4647
</dependency>
4748
<dependency>
4849
<groupId>org.mockito</groupId>
4950
<artifactId>mockito-core</artifactId>
5051
<version>${mockito.version}</version>
5152
<scope>test</scope>
5253
</dependency>
54+
<dependency>
55+
<groupId>io.micrometer</groupId>
56+
<artifactId>micrometer-test</artifactId>
57+
<version>${micrometer.version}</version>
58+
<scope>test</scope>
59+
</dependency>
5360
<dependency>
5461
<groupId>org.hamcrest</groupId>
5562
<artifactId>hamcrest</artifactId>
5663
<scope>test</scope>
5764
</dependency>
65+
<dependency>
66+
<groupId>${project.groupId}</groupId>
67+
<artifactId>feign-okhttp</artifactId>
68+
<version>${project.version}</version>
69+
<scope>test</scope>
70+
</dependency>
5871
</dependencies>
5972

6073
<build>
@@ -71,5 +84,4 @@
7184
</plugin>
7285
</plugins>
7386
</build>
74-
7587
</project>
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/*
2+
* Copyright 2012-2022 The Feign Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
5+
* in compliance with the License. You may obtain a copy of the License at
6+
*
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* Unless required by applicable law or agreed to in writing, software distributed under the License
10+
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
11+
* or implied. See the License for the specific language governing permissions and limitations under
12+
* the License.
13+
*/
14+
package feign.micrometer;
15+
16+
import feign.Request;
17+
import feign.Response;
18+
import io.micrometer.common.KeyValues;
19+
import io.micrometer.common.lang.Nullable;
20+
21+
/**
22+
* Default implementation of {@link FeignObservationConvention}.
23+
*
24+
* @since 12.1
25+
* @see FeignObservationConvention
26+
*/
27+
public class DefaultFeignObservationConvention implements FeignObservationConvention {
28+
29+
/**
30+
* Singleton instance of this convention.
31+
*/
32+
public static final DefaultFeignObservationConvention INSTANCE =
33+
new DefaultFeignObservationConvention();
34+
35+
// There is no need to instantiate this class multiple times, but it may be extended,
36+
// hence protected visibility.
37+
protected DefaultFeignObservationConvention() {}
38+
39+
@Override
40+
public String getName() {
41+
return "http.client.requests";
42+
}
43+
44+
@Override
45+
public String getContextualName(FeignContext context) {
46+
return "HTTP " + getMethodString(context.getCarrier());
47+
}
48+
49+
@Override
50+
public KeyValues getLowCardinalityKeyValues(FeignContext context) {
51+
String templatedUrl = context.getCarrier().requestTemplate().methodMetadata().template().url();
52+
return KeyValues.of(
53+
FeignObservationDocumentation.HttpClientTags.METHOD
54+
.withValue(getMethodString(context.getCarrier())),
55+
FeignObservationDocumentation.HttpClientTags.URI
56+
.withValue(templatedUrl),
57+
FeignObservationDocumentation.HttpClientTags.STATUS
58+
.withValue(getStatusValue(context.getResponse())));
59+
}
60+
61+
String getStatusValue(@Nullable Response response) {
62+
return response != null ? String.valueOf(response.status()) : "CLIENT_ERROR";
63+
}
64+
65+
String getMethodString(@Nullable Request request) {
66+
if (request == null) {
67+
return "UNKNOWN";
68+
}
69+
return request.httpMethod().name();
70+
}
71+
72+
}
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/*
2+
* Copyright 2012-2022 The Feign Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
5+
* in compliance with the License. You may obtain a copy of the License at
6+
*
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* Unless required by applicable law or agreed to in writing, software distributed under the License
10+
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
11+
* or implied. See the License for the specific language governing permissions and limitations under
12+
* the License.
13+
*/
14+
package feign.micrometer;
15+
16+
import feign.Request;
17+
import feign.Response;
18+
import io.micrometer.observation.transport.RequestReplySenderContext;
19+
import io.micrometer.observation.transport.SenderContext;
20+
21+
/**
22+
* A {@link SenderContext} for Feign.
23+
*
24+
* @author Marcin Grzejszczak
25+
* @since 12.1
26+
*/
27+
public class FeignContext extends RequestReplySenderContext<Request, Response> {
28+
29+
public FeignContext(Request request) {
30+
super(Request::header);
31+
setCarrier(request);
32+
}
33+
34+
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/*
2+
* Copyright 2012-2022 The Feign Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
5+
* in compliance with the License. You may obtain a copy of the License at
6+
*
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* Unless required by applicable law or agreed to in writing, software distributed under the License
10+
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
11+
* or implied. See the License for the specific language governing permissions and limitations under
12+
* the License.
13+
*/
14+
package feign.micrometer;
15+
16+
import io.micrometer.observation.Observation;
17+
import io.micrometer.observation.ObservationConvention;
18+
19+
/**
20+
* {@link ObservationConvention} for Feign.
21+
*
22+
* @since 12.1
23+
*/
24+
public interface FeignObservationConvention extends ObservationConvention<FeignContext> {
25+
26+
@Override
27+
default boolean supportsContext(Observation.Context context) {
28+
return context instanceof FeignContext;
29+
}
30+
31+
}

0 commit comments

Comments
 (0)