Skip to content

Commit 140b83b

Browse files
jonatan-ivanovjonatan-ivanov
authored
Harmonize @nullable annotations in okhttp instrumentation (#7375)
This is a just firefighting and not a proper fix. The proper fix would be non-null request and nullable response but fixing that seems to be a very deep rabbit hole and also involves breaking changes. Closes gh-7373
1 parent 8c1758b commit 140b83b

4 files changed

Lines changed: 33 additions & 30 deletions

File tree

‎micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/DefaultOkHttpObservationConvention.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,10 @@ public KeyValues getLowCardinalityKeyValues(OkHttpContext context) {
7676
Request request = state != null ? state.request : null;
7777
Response response = state != null ? state.response : null;
7878
IOException exception = state != null ? state.exception : null;
79-
Function<Request, String> urlMapper = context.getUrlMapper();
79+
Function<@Nullable Request, String> urlMapper = context.getUrlMapper();
8080
Iterable<KeyValue> extraTags = context.getExtraTags();
81-
Iterable<BiFunction<Request, Response, KeyValue>> contextSpecificTags = context.getContextSpecificTags();
81+
Iterable<BiFunction<@Nullable Request, @Nullable Response, KeyValue>> contextSpecificTags = context
82+
.getContextSpecificTags();
8283
Iterable<KeyValue> unknownRequestTags = context.getUnknownRequestTags();
8384
boolean includeHostTag = context.isIncludeHostTag();
8485
// TODO: Tags to key values and back - maybe we can improve this?
@@ -98,7 +99,7 @@ public KeyValues getLowCardinalityKeyValues(OkHttpContext context) {
9899
return keyValues;
99100
}
100101

101-
private String getUriTag(Function<Request, String> urlMapper, @Nullable Request request) {
102+
private String getUriTag(Function<@Nullable Request, String> urlMapper, @Nullable Request request) {
102103
if (request == null) {
103104
return TAG_VALUE_UNKNOWN;
104105
}

‎micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpContext.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,11 @@
3737
public class OkHttpContext extends RequestReplySenderContext<Request.Builder, Response>
3838
implements Supplier<OkHttpContext> {
3939

40-
private final Function<Request, String> urlMapper;
40+
private final Function<@Nullable Request, String> urlMapper;
4141

4242
private final Iterable<KeyValue> extraTags;
4343

44-
private final Iterable<BiFunction<Request, Response, KeyValue>> contextSpecificTags;
44+
private final Iterable<BiFunction<@Nullable Request, @Nullable Response, KeyValue>> contextSpecificTags;
4545

4646
private final Iterable<KeyValue> unknownRequestTags;
4747

@@ -51,8 +51,8 @@ public class OkHttpContext extends RequestReplySenderContext<Request.Builder, Re
5151

5252
private OkHttpObservationInterceptor.@Nullable CallState state;
5353

54-
public OkHttpContext(Function<Request, String> urlMapper, Iterable<KeyValue> extraTags,
55-
Iterable<BiFunction<Request, Response, KeyValue>> contextSpecificTags,
54+
public OkHttpContext(Function<@Nullable Request, String> urlMapper, Iterable<KeyValue> extraTags,
55+
Iterable<BiFunction<@Nullable Request, @Nullable Response, KeyValue>> contextSpecificTags,
5656
Iterable<KeyValue> unknownRequestTags, boolean includeHostTag, Request originalRequest) {
5757
super((carrier, key, value) -> {
5858
if (carrier != null) {
@@ -75,15 +75,15 @@ public void setState(OkHttpObservationInterceptor.CallState state) {
7575
return state;
7676
}
7777

78-
public Function<Request, String> getUrlMapper() {
78+
public Function<@Nullable Request, String> getUrlMapper() {
7979
return urlMapper;
8080
}
8181

8282
public Iterable<KeyValue> getExtraTags() {
8383
return extraTags;
8484
}
8585

86-
public Iterable<BiFunction<Request, Response, KeyValue>> getContextSpecificTags() {
86+
public Iterable<BiFunction<@Nullable Request, @Nullable Response, KeyValue>> getContextSpecificTags() {
8787
return contextSpecificTags;
8888
}
8989

‎micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpMetricsEventListener.java‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -89,11 +89,11 @@ public class OkHttpMetricsEventListener extends EventListener {
8989

9090
private final String requestsMetricName;
9191

92-
private final Function<Request, String> urlMapper;
92+
private final Function<@Nullable Request, String> urlMapper;
9393

9494
private final Iterable<Tag> extraTags;
9595

96-
private final Iterable<BiFunction<Request, Response, Tag>> contextSpecificTags;
96+
private final Iterable<BiFunction<@Nullable Request, @Nullable Response, Tag>> contextSpecificTags;
9797

9898
private final Iterable<Tag> unknownRequestTags;
9999

@@ -103,13 +103,14 @@ public class OkHttpMetricsEventListener extends EventListener {
103103
final ConcurrentMap<Call, CallState> callState = new ConcurrentHashMap<>();
104104

105105
protected OkHttpMetricsEventListener(MeterRegistry registry, String requestsMetricName,
106-
Function<Request, String> urlMapper, Iterable<Tag> extraTags,
107-
Iterable<BiFunction<Request, Response, Tag>> contextSpecificTags) {
106+
Function<@Nullable Request, String> urlMapper, Iterable<Tag> extraTags,
107+
Iterable<BiFunction<@Nullable Request, @Nullable Response, Tag>> contextSpecificTags) {
108108
this(registry, requestsMetricName, urlMapper, extraTags, contextSpecificTags, emptyList(), true);
109109
}
110110

111-
OkHttpMetricsEventListener(MeterRegistry registry, String requestsMetricName, Function<Request, String> urlMapper,
112-
Iterable<Tag> extraTags, Iterable<BiFunction<Request, Response, Tag>> contextSpecificTags,
111+
OkHttpMetricsEventListener(MeterRegistry registry, String requestsMetricName,
112+
Function<@Nullable Request, String> urlMapper, Iterable<Tag> extraTags,
113+
Iterable<BiFunction<@Nullable Request, @Nullable Response, Tag>> contextSpecificTags,
113114
Iterable<String> requestTagKeys, boolean includeHostTag) {
114115
this.registry = registry;
115116
this.requestsMetricName = requestsMetricName;
@@ -259,12 +260,12 @@ public static class Builder {
259260

260261
private final String name;
261262

262-
private Function<Request, String> uriMapper = (request) -> Optional.ofNullable(request.header(URI_PATTERN))
263-
.orElse(KeyValue.NONE_VALUE);
263+
private Function<@Nullable Request, String> uriMapper = (
264+
request) -> Optional.ofNullable(request).map(rq -> rq.header(URI_PATTERN)).orElse(KeyValue.NONE_VALUE);
264265

265266
private Tags tags = Tags.empty();
266267

267-
private Collection<BiFunction<Request, Response, Tag>> contextSpecificTags = new ArrayList<>();
268+
private Collection<BiFunction<@Nullable Request, @Nullable Response, Tag>> contextSpecificTags = new ArrayList<>();
268269

269270
private boolean includeHostTag = true;
270271

@@ -297,12 +298,12 @@ public Builder tag(Tag tag) {
297298
* @return this builder
298299
* @since 1.5.0
299300
*/
300-
public Builder tag(BiFunction<Request, Response, Tag> contextSpecificTag) {
301+
public Builder tag(BiFunction<@Nullable Request, @Nullable Response, Tag> contextSpecificTag) {
301302
this.contextSpecificTags.add(contextSpecificTag);
302303
return this;
303304
}
304305

305-
public Builder uriMapper(Function<Request, String> uriMapper) {
306+
public Builder uriMapper(Function<@Nullable Request, String> uriMapper) {
306307
this.uriMapper = uriMapper;
307308
return this;
308309
}

‎micrometer-core/src/main/java/io/micrometer/core/instrument/binder/okhttp3/OkHttpObservationInterceptor.java‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,21 +43,21 @@ public class OkHttpObservationInterceptor implements Interceptor {
4343

4444
private final String requestMetricName;
4545

46-
private final Function<Request, String> urlMapper;
46+
private final Function<@Nullable Request, String> urlMapper;
4747

4848
private final Iterable<KeyValue> extraTags;
4949

50-
private final Iterable<BiFunction<Request, Response, KeyValue>> contextSpecificTags;
50+
private final Iterable<BiFunction<@Nullable Request, @Nullable Response, KeyValue>> contextSpecificTags;
5151

5252
private final Iterable<KeyValue> unknownRequestTags;
5353

5454
private final boolean includeHostTag;
5555

5656
public OkHttpObservationInterceptor(ObservationRegistry registry,
5757
@Nullable OkHttpObservationConvention observationConvention, String requestsMetricName,
58-
Function<Request, String> urlMapper, Iterable<KeyValue> extraTags,
59-
Iterable<BiFunction<Request, Response, KeyValue>> contextSpecificTags, Iterable<String> requestTagKeys,
60-
boolean includeHostTag) {
58+
Function<@Nullable Request, String> urlMapper, Iterable<KeyValue> extraTags,
59+
Iterable<BiFunction<@Nullable Request, @Nullable Response, KeyValue>> contextSpecificTags,
60+
Iterable<String> requestTagKeys, boolean includeHostTag) {
6161
this.registry = registry;
6262
this.observationConvention = observationConvention;
6363
this.requestMetricName = requestsMetricName;
@@ -139,12 +139,12 @@ public static class Builder {
139139

140140
private final ObservationRegistry registry;
141141

142-
private Function<Request, String> uriMapper = (request) -> Optional.ofNullable(request.header(URI_PATTERN))
143-
.orElse(KeyValue.NONE_VALUE);
142+
private Function<@Nullable Request, String> uriMapper = (
143+
request) -> Optional.ofNullable(request).map(rq -> rq.header(URI_PATTERN)).orElse(KeyValue.NONE_VALUE);
144144

145145
private KeyValues tags = KeyValues.empty();
146146

147-
private Collection<BiFunction<Request, Response, KeyValue>> contextSpecificTags = new ArrayList<>();
147+
private final Collection<BiFunction<@Nullable Request, @Nullable Response, KeyValue>> contextSpecificTags = new ArrayList<>();
148148

149149
private boolean includeHostTag = true;
150150

@@ -183,12 +183,13 @@ public OkHttpObservationInterceptor.Builder tag(KeyValue tag) {
183183
* @param contextSpecificTag function to create a context-specific tag
184184
* @return this builder
185185
*/
186-
public OkHttpObservationInterceptor.Builder tag(BiFunction<Request, Response, KeyValue> contextSpecificTag) {
186+
public OkHttpObservationInterceptor.Builder tag(
187+
BiFunction<@Nullable Request, @Nullable Response, KeyValue> contextSpecificTag) {
187188
this.contextSpecificTags.add(contextSpecificTag);
188189
return this;
189190
}
190191

191-
public OkHttpObservationInterceptor.Builder uriMapper(Function<Request, String> uriMapper) {
192+
public OkHttpObservationInterceptor.Builder uriMapper(Function<@Nullable Request, String> uriMapper) {
192193
this.uriMapper = uriMapper;
193194
return this;
194195
}

0 commit comments

Comments
 (0)