Skip to content

[pull] master from spring-projects:master #77

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pull merged 2 commits into from
Aug 23, 2019
Merged

[pull] master from spring-projects:master #77

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
608228d
Improve handling of non-standard status codes in WebClient metrics
wilkinsona Aug 23, 2019
b54ff7c
Merge branch '2.1.x'
wilkinsona Aug 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 22 additions & 18 deletions ...a/org/springframework/boot/actuate/metrics/web/reactive/client/WebClientExchangeTags.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,15 @@
package org.springframework.boot.actuate.metrics.web.reactive.client;

import java.io.IOException;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.regex.Pattern;

import io.micrometer.core.instrument.Tag;

import org.springframework.http.HttpStatus;
import org.springframework.http.HttpStatus.Series;
import org.springframework.http.client.reactive.ClientHttpRequest;
import org.springframework.web.reactive.function.client.ClientRequest;
import org.springframework.web.reactive.function.client.ClientResponse;
Expand Down Expand Up @@ -59,6 +63,18 @@ public final class WebClientExchangeTags {

private static final Tag OUTCOME_SERVER_ERROR = Tag.of("outcome", "SERVER_ERROR");

private static final Map<Series, Tag> SERIES_OUTCOMES;

static {
Map<Series, Tag> seriesOutcomes = new HashMap<>();
seriesOutcomes.put(Series.INFORMATIONAL, OUTCOME_INFORMATIONAL);
seriesOutcomes.put(Series.SUCCESSFUL, OUTCOME_SUCCESS);
seriesOutcomes.put(Series.REDIRECTION, OUTCOME_REDIRECTION);
seriesOutcomes.put(Series.CLIENT_ERROR, OUTCOME_CLIENT_ERROR);
seriesOutcomes.put(Series.SERVER_ERROR, OUTCOME_SERVER_ERROR);
SERIES_OUTCOMES = Collections.unmodifiableMap(seriesOutcomes);
}

private WebClientExchangeTags() {
}

Expand Down Expand Up @@ -94,7 +110,7 @@ private static String extractPath(String url) {
* @return the status tag
*/
public static Tag status(ClientResponse response) {
return Tag.of("status", String.valueOf(response.statusCode().value()));
return Tag.of("status", String.valueOf(response.rawStatusCode()));
}

/**
Expand Down Expand Up @@ -132,28 +148,16 @@ public static Tag clientName(ClientRequest request) {
public static Tag outcome(ClientResponse response) {
try {
if (response != null) {
HttpStatus status = response.statusCode();
if (status.is1xxInformational()) {
return OUTCOME_INFORMATIONAL;
}
if (status.is2xxSuccessful()) {
return OUTCOME_SUCCESS;
}
if (status.is3xxRedirection()) {
return OUTCOME_REDIRECTION;
}
if (status.is4xxClientError()) {
return OUTCOME_CLIENT_ERROR;
}
if (status.is5xxServerError()) {
return OUTCOME_SERVER_ERROR;
Series series = HttpStatus.Series.resolve(response.rawStatusCode());
if (series != null) {
return SERIES_OUTCOMES.getOrDefault(series, OUTCOME_UNKNOWN);
}
}
return OUTCOME_UNKNOWN;
}
catch (IllegalArgumentException exc) {
return OUTCOME_UNKNOWN;
// Continue
}
return OUTCOME_UNKNOWN;
}

}
2 changes: 1 addition & 1 deletion ...k/boot/actuate/metrics/web/reactive/client/DefaultWebClientExchangeTagsProviderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ void setup() {
this.request = ClientRequest.create(HttpMethod.GET, URI.create("https://example.org/projects/spring-boot"))
.attribute(URI_TEMPLATE_ATTRIBUTE, "https://example.org/projects/{project}").build();
this.response = mock(ClientResponse.class);
given(this.response.statusCode()).willReturn(HttpStatus.OK);
given(this.response.rawStatusCode()).willReturn(HttpStatus.OK.value());
}

@Test
Expand Down
4 changes: 2 additions & 2 deletions ...amework/boot/actuate/metrics/web/reactive/client/MetricsWebClientFilterFunctionTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ void setup() {
void filterShouldRecordTimer() {
ClientRequest request = ClientRequest
.create(HttpMethod.GET, URI.create("https://example.com/projects/spring-boot")).build();
given(this.response.statusCode()).willReturn(HttpStatus.OK);
given(this.response.rawStatusCode()).willReturn(HttpStatus.OK.value());
this.filterFunction.filter(request, this.exchange).block(Duration.ofSeconds(30));
assertThat(this.registry.get("http.client.requests")
.tags("method", "GET", "uri", "/projects/spring-boot", "status", "200").timer().count()).isEqualTo(1);
Expand All @@ -83,7 +83,7 @@ void filterWhenUriTemplatePresentShouldRecordTimer() {
ClientRequest request = ClientRequest
.create(HttpMethod.GET, URI.create("https://example.com/projects/spring-boot"))
.attribute(URI_TEMPLATE_ATTRIBUTE, "/projects/{project}").build();
given(this.response.statusCode()).willReturn(HttpStatus.OK);
given(this.response.rawStatusCode()).willReturn(HttpStatus.OK.value());
this.filterFunction.filter(request, this.exchange).block(Duration.ofSeconds(30));
assertThat(this.registry.get("http.client.requests")
.tags("method", "GET", "uri", "/projects/{project}", "status", "200").timer().count()).isEqualTo(1);
Expand Down
29 changes: 21 additions & 8 deletions .../springframework/boot/actuate/metrics/web/reactive/client/WebClientExchangeTagsTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ void setup() {
this.request = ClientRequest.create(HttpMethod.GET, URI.create("https://example.org/projects/spring-boot"))
.attribute(URI_TEMPLATE_ATTRIBUTE, "https://example.org/projects/{project}").build();
this.response = mock(ClientResponse.class);
given(this.response.statusCode()).willReturn(HttpStatus.OK);
}

@Test
Expand Down Expand Up @@ -86,6 +85,7 @@ void clientName() {

@Test
void status() {
given(this.response.rawStatusCode()).willReturn(HttpStatus.OK.value());
assertThat(WebClientExchangeTags.status(this.response)).isEqualTo(Tag.of("status", "200"));
}

Expand All @@ -100,6 +100,12 @@ void statusWhenClientException() {
.isEqualTo(Tag.of("status", "CLIENT_ERROR"));
}

@Test
void statusWhenNonStandard() {
given(this.response.rawStatusCode()).willReturn(490);
assertThat(WebClientExchangeTags.status(this.response)).isEqualTo(Tag.of("status", "490"));
}

@Test
void outcomeTagIsUnknownWhenResponseIsNull() {
Tag tag = WebClientExchangeTags.outcome(null);
Expand All @@ -108,42 +114,49 @@ void outcomeTagIsUnknownWhenResponseIsNull() {

@Test
void outcomeTagIsInformationalWhenResponseIs1xx() {
given(this.response.statusCode()).willReturn(HttpStatus.CONTINUE);
given(this.response.rawStatusCode()).willReturn(HttpStatus.CONTINUE.value());
Tag tag = WebClientExchangeTags.outcome(this.response);
assertThat(tag.getValue()).isEqualTo("INFORMATIONAL");
}

@Test
void outcomeTagIsSuccessWhenResponseIs2xx() {
given(this.response.statusCode()).willReturn(HttpStatus.OK);
given(this.response.rawStatusCode()).willReturn(HttpStatus.OK.value());
Tag tag = WebClientExchangeTags.outcome(this.response);
assertThat(tag.getValue()).isEqualTo("SUCCESS");
}

@Test
void outcomeTagIsRedirectionWhenResponseIs3xx() {
given(this.response.statusCode()).willReturn(HttpStatus.MOVED_PERMANENTLY);
given(this.response.rawStatusCode()).willReturn(HttpStatus.MOVED_PERMANENTLY.value());
Tag tag = WebClientExchangeTags.outcome(this.response);
assertThat(tag.getValue()).isEqualTo("REDIRECTION");
}

@Test
void outcomeTagIsClientErrorWhenResponseIs4xx() {
given(this.response.statusCode()).willReturn(HttpStatus.BAD_REQUEST);
given(this.response.rawStatusCode()).willReturn(HttpStatus.BAD_REQUEST.value());
Tag tag = WebClientExchangeTags.outcome(this.response);
assertThat(tag.getValue()).isEqualTo("CLIENT_ERROR");
}

@Test
void outcomeTagIsServerErrorWhenResponseIs5xx() {
given(this.response.statusCode()).willReturn(HttpStatus.BAD_GATEWAY);
given(this.response.rawStatusCode()).willReturn(HttpStatus.BAD_GATEWAY.value());
Tag tag = WebClientExchangeTags.outcome(this.response);
assertThat(tag.getValue()).isEqualTo("SERVER_ERROR");
}

@Test
void outcomeTagIsUnknownWhenResponseStatusIsUnknown() {
given(this.response.statusCode()).willThrow(IllegalArgumentException.class);
void outcomeTagIsServerErrorWhenResponseIsNonStandardInKnownSeries() {
given(this.response.rawStatusCode()).willReturn(490);
Tag tag = WebClientExchangeTags.outcome(this.response);
assertThat(tag.getValue()).isEqualTo("CLIENT_ERROR");
}

@Test
void outcomeTagIsUnknownWhenResponseStatusIsInUnknownSeries() {
given(this.response.rawStatusCode()).willReturn(701);
Tag tag = WebClientExchangeTags.outcome(this.response);
assertThat(tag.getValue()).isEqualTo("UNKNOWN");
}
Expand Down