Skip to content

Commit 7e45f60

Browse files
shakuzenbclozel
shakuzen
authored and
bclozel
committed
Add test for OpenTelemetryServerRequestObservationConvention
See gh-35358
1 parent 3269751 commit 7e45f60

File tree

3 files changed

+164
-5
lines changed

3 files changed

+164
-5
lines changed

‎spring-web/src/main/java/org/springframework/http/server/observation/OpenTelemetryServerHttpObservationDocumentation.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ public String asString() {
9797
},
9898

9999
/**
100-
* Name of the exception thrown during the exchange, or
100+
* Fully qualified name of the exception thrown during the exchange, or
101101
* {@value KeyValue#NONE_VALUE} if no exception was thrown.
102102
*/
103103
EXCEPTION {

‎spring-web/src/main/java/org/springframework/http/server/observation/OpenTelemetryServerRequestObservationConvention.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import org.springframework.http.HttpStatusCode;
3030
import org.springframework.http.server.observation.OpenTelemetryServerHttpObservationDocumentation.HighCardinalityKeyNames;
3131
import org.springframework.http.server.observation.OpenTelemetryServerHttpObservationDocumentation.LowCardinalityKeyNames;
32-
import org.springframework.util.StringUtils;
3332

3433
/**
3534
* A {@link ServerRequestObservationConvention} based on the stable OpenTelemetry semantic conventions.
@@ -180,9 +179,7 @@ protected KeyValue pathTemplate(ServerRequestObservationContext context) {
180179
protected KeyValue exception(ServerRequestObservationContext context) {
181180
Throwable error = context.getError();
182181
if (error != null) {
183-
String simpleName = error.getClass().getSimpleName();
184-
return KeyValue.of(LowCardinalityKeyNames.EXCEPTION,
185-
StringUtils.hasText(simpleName) ? simpleName : error.getClass().getName());
182+
return KeyValue.of(LowCardinalityKeyNames.EXCEPTION, error.getClass().getName());
186183
}
187184
return EXCEPTION_NONE;
188185
}

‎spring-web/src/test/java/org/springframework/http/server/observation/OpenTelemetryServerRequestObservationConventionTests.java‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
/*
2+
* Copyright 2002-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.http.server.observation;
18+
19+
import io.micrometer.common.KeyValue;
20+
import io.micrometer.observation.Observation;
21+
import org.junit.jupiter.api.Test;
22+
23+
import org.springframework.web.testfixture.servlet.MockHttpServletRequest;
24+
import org.springframework.web.testfixture.servlet.MockHttpServletResponse;
25+
26+
import static org.assertj.core.api.Assertions.assertThat;
27+
28+
/**
29+
* Tests for {@link OpenTelemetryServerRequestObservationConvention}.
30+
* @author Brian Clozel
31+
* @author Tommy Ludwig
32+
*/
33+
class OpenTelemetryServerRequestObservationConventionTests {
34+
35+
private final OpenTelemetryServerRequestObservationConvention convention = new OpenTelemetryServerRequestObservationConvention();
36+
37+
private final MockHttpServletRequest request = new MockHttpServletRequest("GET", "/test/resource");
38+
39+
private final MockHttpServletResponse response = new MockHttpServletResponse();
40+
41+
private final ServerRequestObservationContext context = new ServerRequestObservationContext(this.request, this.response);
42+
43+
44+
@Test
45+
void shouldHaveName() {
46+
assertThat(convention.getName()).isEqualTo("http.server.request.duration");
47+
}
48+
49+
@Test
50+
void shouldHaveContextualName() {
51+
assertThat(convention.getContextualName(this.context)).isEqualTo("GET");
52+
}
53+
54+
@Test
55+
void contextualNameShouldUsePathPatternWhenAvailable() {
56+
this.context.setPathPattern("/test/{name}");
57+
assertThat(convention.getContextualName(this.context)).isEqualTo("GET /test/{name}");
58+
}
59+
60+
@Test
61+
void supportsOnlyHttpRequestsObservationContext() {
62+
assertThat(this.convention.supportsContext(this.context)).isTrue();
63+
assertThat(this.convention.supportsContext(new Observation.Context())).isFalse();
64+
}
65+
66+
@Test
67+
void addsKeyValuesForExchange() {
68+
this.request.setMethod("POST");
69+
this.request.setRequestURI("/test/resource");
70+
71+
assertThat(this.convention.getLowCardinalityKeyValues(this.context)).hasSize(6)
72+
.contains(KeyValue.of("http.request.method", "POST"), KeyValue.of("http.route", "UNKNOWN"), KeyValue.of("http.response.status_code", "200"),
73+
KeyValue.of("error.type", "none"), KeyValue.of("outcome", "SUCCESS"), KeyValue.of("url.scheme", "http"));
74+
assertThat(this.convention.getHighCardinalityKeyValues(this.context)).hasSize(2)
75+
.contains(KeyValue.of("url.path", "/test/resource"), KeyValue.of("http.request.method_original", "POST"));
76+
}
77+
78+
@Test
79+
void addsKeyValuesForExchangeWithPathPattern() {
80+
this.request.setRequestURI("/test/resource");
81+
this.context.setPathPattern("/test/{name}");
82+
83+
assertThat(this.convention.getLowCardinalityKeyValues(this.context)).hasSize(6)
84+
.contains(KeyValue.of("http.request.method", "GET"), KeyValue.of("http.route", "/test/{name}"), KeyValue.of("http.response.status_code", "200"),
85+
KeyValue.of("error.type", "none"), KeyValue.of("outcome", "SUCCESS"), KeyValue.of("url.scheme", "http"));
86+
assertThat(this.convention.getHighCardinalityKeyValues(this.context)).hasSize(2)
87+
.contains(KeyValue.of("url.path", "/test/resource"), KeyValue.of("http.request.method_original", "GET"));
88+
}
89+
90+
@Test
91+
void addsKeyValuesForErrorExchange() {
92+
this.request.setRequestURI("/test/resource");
93+
this.context.setError(new IllegalArgumentException("custom error"));
94+
this.response.setStatus(500);
95+
96+
assertThat(this.convention.getLowCardinalityKeyValues(this.context)).hasSize(6)
97+
.contains(KeyValue.of("http.request.method", "GET"), KeyValue.of("http.route", "UNKNOWN"), KeyValue.of("http.response.status_code", "500"),
98+
KeyValue.of("error.type", "java.lang.IllegalArgumentException"), KeyValue.of("outcome", "SERVER_ERROR"), KeyValue.of("url.scheme", "http"));
99+
assertThat(this.convention.getHighCardinalityKeyValues(this.context)).hasSize(2)
100+
.contains(KeyValue.of("url.path", "/test/resource"), KeyValue.of("http.request.method_original", "GET"));
101+
}
102+
103+
@Test
104+
void addsKeyValuesForRedirectExchange() {
105+
this.request.setRequestURI("/test/redirect");
106+
this.response.setStatus(302);
107+
this.response.addHeader("Location", "https://example.org/other");
108+
109+
assertThat(this.convention.getLowCardinalityKeyValues(this.context)).hasSize(6)
110+
.contains(KeyValue.of("http.request.method", "GET"), KeyValue.of("http.route", "REDIRECTION"), KeyValue.of("http.response.status_code", "302"),
111+
KeyValue.of("error.type", "none"), KeyValue.of("outcome", "REDIRECTION"), KeyValue.of("url.scheme", "http"));
112+
assertThat(this.convention.getHighCardinalityKeyValues(this.context)).hasSize(2)
113+
.contains(KeyValue.of("url.path", "/test/redirect"), KeyValue.of("http.request.method_original", "GET"));
114+
}
115+
116+
@Test
117+
void addsKeyValuesForNotFoundExchange() {
118+
this.request.setRequestURI("/test/notFound");
119+
this.response.setStatus(404);
120+
121+
assertThat(this.convention.getLowCardinalityKeyValues(this.context)).hasSize(6)
122+
.contains(KeyValue.of("http.request.method", "GET"), KeyValue.of("http.route", "NOT_FOUND"), KeyValue.of("http.response.status_code", "404"),
123+
KeyValue.of("error.type", "none"), KeyValue.of("outcome", "CLIENT_ERROR"), KeyValue.of("url.scheme", "http"));
124+
assertThat(this.convention.getHighCardinalityKeyValues(this.context)).hasSize(2)
125+
.contains(KeyValue.of("url.path", "/test/notFound"), KeyValue.of("http.request.method_original", "GET"));
126+
}
127+
128+
@Test
129+
void addsKeyValuesForUnknownHttpMethodExchange() {
130+
this.request.setMethod("SPRING");
131+
this.request.setRequestURI("/test");
132+
this.response.setStatus(404);
133+
134+
assertThat(this.convention.getContextualName(this.context)).isEqualTo("HTTP");
135+
assertThat(this.convention.getLowCardinalityKeyValues(this.context)).hasSize(6)
136+
.contains(KeyValue.of("http.request.method", "_OTHER"), KeyValue.of("http.route", "NOT_FOUND"), KeyValue.of("http.response.status_code", "404"),
137+
KeyValue.of("error.type", "none"), KeyValue.of("outcome", "CLIENT_ERROR"), KeyValue.of("url.scheme", "http"));
138+
assertThat(this.convention.getHighCardinalityKeyValues(this.context)).hasSize(2)
139+
.contains(KeyValue.of("url.path", "/test"), KeyValue.of("http.request.method_original", "SPRING"));
140+
}
141+
142+
@Test
143+
void setsContextualNameWithPathPatternButInvalidMethod() {
144+
this.request.setMethod("CUSTOM");
145+
this.context.setPathPattern("/test/{name}");
146+
147+
assertThat(this.convention.getContextualName(this.context)).isEqualTo("HTTP /test/{name}");
148+
}
149+
150+
@Test
151+
void addsKeyValuesForInvalidStatusExchange() {
152+
this.request.setRequestURI("/test/invalidStatus");
153+
this.response.setStatus(0);
154+
155+
assertThat(this.convention.getLowCardinalityKeyValues(this.context)).hasSize(6)
156+
.contains(KeyValue.of("http.request.method", "GET"), KeyValue.of("http.route", "UNKNOWN"), KeyValue.of("http.response.status_code", "0"),
157+
KeyValue.of("error.type", "none"), KeyValue.of("outcome", "UNKNOWN"), KeyValue.of("url.scheme", "http"));
158+
assertThat(this.convention.getHighCardinalityKeyValues(this.context)).hasSize(2)
159+
.contains(KeyValue.of("url.path", "/test/invalidStatus"), KeyValue.of("http.request.method_original", "GET"));
160+
}
161+
162+
}

0 commit comments

Comments
 (0)