Skip to content

Commit 5b4eb8c

Browse files
bclozelbclozel
committed
Align observation convention with Micrometer best practices
This commit ensures that the contextual names of the observations all start with `"graphql"` (lowercase) and that all key values are prefixed with `"graphql.*"` for a cleaner namespace. Fixes gh-558
1 parent 5fe8123 commit 5b4eb8c

File tree

6 files changed

+55
-42
lines changed

6 files changed

+55
-42
lines changed

‎spring-graphql/src/main/java/org/springframework/graphql/observation/DefaultDataFetcherObservationConvention.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public String getName() {
5555

5656
@Override
5757
public String getContextualName(DataFetcherObservationContext context) {
58-
return "graphQL field " + context.getEnvironment().getField().getName();
58+
return "graphql field " + context.getEnvironment().getField().getName();
5959
}
6060

6161
@Override

‎spring-graphql/src/main/java/org/springframework/graphql/observation/DefaultExecutionRequestObservationConvention.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public class DefaultExecutionRequestObservationConvention implements ExecutionRe
3333

3434
private static final String DEFAULT_NAME = "graphql.request";
3535

36-
private static final String BASE_CONTEXTUAL_NAME = "graphQL ";
36+
private static final String BASE_CONTEXTUAL_NAME = "graphql ";
3737

3838
private static final KeyValue OUTCOME_SUCCESS = KeyValue.of(ExecutionRequestLowCardinalityKeyNames.OUTCOME, "SUCCESS");
3939

@@ -60,7 +60,8 @@ public String getName() {
6060

6161
@Override
6262
public String getContextualName(ExecutionRequestObservationContext context) {
63-
return BASE_CONTEXTUAL_NAME + context.getCarrier().getOperationName();
63+
String operationName = (context.getCarrier().getOperationName() != null) ? context.getCarrier().getOperationName() : "query";
64+
return BASE_CONTEXTUAL_NAME + operationName;
6465
}
6566

6667
@Override

‎spring-graphql/src/main/java/org/springframework/graphql/observation/GraphQlObservationDocumentation.java

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,12 @@ public enum GraphQlObservationDocumentation implements ObservationDocumentation
3636
* Observation created for GraphQL execution requests.
3737
*/
3838
EXECUTION_REQUEST {
39+
40+
@Override
41+
public String getPrefix() {
42+
return "graphql";
43+
}
44+
3945
@Override
4046
public Class<? extends ObservationConvention<? extends Observation.Context>> getDefaultConvention() {
4147
return DefaultExecutionRequestObservationConvention.class;
@@ -58,6 +64,12 @@ public KeyName[] getHighCardinalityKeyNames() {
5864
* data fetching operations.
5965
*/
6066
DATA_FETCHER {
67+
68+
@Override
69+
public String getPrefix() {
70+
return "graphql";
71+
}
72+
6173
@Override
6274
public Class<? extends ObservationConvention<? extends Observation.Context>> getDefaultConvention() {
6375
return DefaultDataFetcherObservationConvention.class;
@@ -77,7 +89,7 @@ public enum ExecutionRequestLowCardinalityKeyNames implements KeyName {
7789
OUTCOME {
7890
@Override
7991
public String asString() {
80-
return "outcome";
92+
return "graphql.outcome";
8193
}
8294
},
8395

@@ -87,7 +99,7 @@ public String asString() {
8799
OPERATION {
88100
@Override
89101
public String asString() {
90-
return "operation";
102+
return "graphql.operation";
91103
}
92104
}
93105
}
@@ -100,7 +112,7 @@ public enum ExecutionRequestHighCardinalityKeyNames implements KeyName {
100112
EXECUTION_ID {
101113
@Override
102114
public String asString() {
103-
return "execution.id";
115+
return "graphql.execution.id";
104116
}
105117
}
106118
}
@@ -113,7 +125,7 @@ public enum DataFetcherLowCardinalityKeyNames implements KeyName {
113125
OUTCOME {
114126
@Override
115127
public String asString() {
116-
return "outcome";
128+
return "graphql.outcome";
117129
}
118130
},
119131

@@ -123,7 +135,7 @@ public String asString() {
123135
FIELD_NAME {
124136
@Override
125137
public String asString() {
126-
return "field.name";
138+
return "graphql.field.name";
127139
}
128140
},
129141

@@ -133,7 +145,7 @@ public String asString() {
133145
ERROR_TYPE {
134146
@Override
135147
public String asString() {
136-
return "error.type";
148+
return "graphql.error.type";
137149
}
138150
}
139151

@@ -147,7 +159,7 @@ public enum DataFetcherHighCardinalityKeyNames implements KeyName {
147159
FIELD_PATH {
148160
@Override
149161
public String asString() {
150-
return "field.path";
162+
return "graphql.field.path";
151163
}
152164
}
153165

‎spring-graphql/src/test/java/org/springframework/graphql/observation/DefaultDataFetcherObservationConventionTests.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ void contextualNameContainsFieldName() {
5959
builder.mergedField(MergedField.newMergedField(Field.newField("project").build()).build());
6060
});
6161
DataFetcherObservationContext context = new DataFetcherObservationContext(environment);
62-
assertThat(this.convention.getContextualName(context)).isEqualTo("graphQL field project");
62+
assertThat(this.convention.getContextualName(context)).isEqualTo("graphql field project");
6363
}
6464

6565
@Test
@@ -68,7 +68,7 @@ void fieldNameKeyValueIsPresent() {
6868
builder.mergedField(MergedField.newMergedField(Field.newField("project").build()).build());
6969
});
7070
DataFetcherObservationContext context = new DataFetcherObservationContext(environment);
71-
assertThat(this.convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("field.name", "project"));
71+
assertThat(this.convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("graphql.field.name", "project"));
7272
}
7373

7474
@Test
@@ -78,7 +78,7 @@ void errorTypeKeyValueIsPresent() {
7878
});
7979
DataFetcherObservationContext context = new DataFetcherObservationContext(environment);
8080
context.setError(new IllegalStateException("custom data fetching failure"));
81-
assertThat(this.convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("error.type", "IllegalStateException"));
81+
assertThat(this.convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("graphql.error.type", "IllegalStateException"));
8282
}
8383

8484
@Test
@@ -90,7 +90,7 @@ void fieldPathKeyValueIsPresent() {
9090
});
9191
DataFetcherObservationContext context = new DataFetcherObservationContext(environment);
9292
context.setError(new IllegalStateException("custom data fetching failure"));
93-
assertThat(this.convention.getHighCardinalityKeyValues(context)).contains(KeyValue.of("field.path", "/projectBySlug/releases"));
93+
assertThat(this.convention.getHighCardinalityKeyValues(context)).contains(KeyValue.of("graphql.field.path", "/projectBySlug/releases"));
9494
}
9595

9696
private DataFetchingEnvironment createDataFetchingEnvironment(Consumer<DataFetchingEnvironmentImpl.Builder> consumer) {

‎spring-graphql/src/test/java/org/springframework/graphql/observation/DefaultExecutionRequestObservationConventionTests.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,43 +58,43 @@ void hasContextualName() {
5858
ExecutionInput input = ExecutionInput.newExecutionInput().query("{ greeting }")
5959
.operationName("mutation").build();
6060
ExecutionRequestObservationContext context = createObservationContext(input, builder -> {});
61-
assertThat(this.convention.getContextualName(context)).isEqualTo("graphQL mutation");
61+
assertThat(this.convention.getContextualName(context)).isEqualTo("graphql mutation");
6262
}
6363

6464
@Test
6565
void hasOperationKeyValueWhenSuccessfulOutput() {
6666
ExecutionRequestObservationContext context = createObservationContext(this.input, builder -> {
6767
});
68-
assertThat(this.convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("operation", "query"));
68+
assertThat(this.convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("graphql.operation", "query"));
6969
}
7070

7171
@Test
7272
void hasOutcomeKeyValueWhenSuccessfulOutput() {
7373
ExecutionRequestObservationContext context = createObservationContext(this.input, builder -> {
7474
});
75-
assertThat(this.convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("outcome", "SUCCESS"));
75+
assertThat(this.convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("graphql.outcome", "SUCCESS"));
7676
}
7777

7878
@Test
7979
void hasOutcomeKeyValueWhenErrorOutput() {
8080
ExecutionRequestObservationContext context = createObservationContext(this.input,
8181
builder -> builder.addError(new QueryOperationMissingError()));
82-
assertThat(this.convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("outcome", "REQUEST_ERROR"));
82+
assertThat(this.convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("graphql.outcome", "REQUEST_ERROR"));
8383
}
8484

8585
@Test
8686
void hasOutcomeKeyValueWhenInternalError() {
8787
ExecutionRequestObservationContext context = createObservationContext(this.input, builder -> {
8888
});
8989
context.setError(new IllegalStateException("custom internal error"));
90-
assertThat(this.convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("outcome", "INTERNAL_ERROR"));
90+
assertThat(this.convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("graphql.outcome", "INTERNAL_ERROR"));
9191
}
9292

9393
@Test
9494
void hasExecutionIdKeyValue() {
9595
ExecutionRequestObservationContext context = createObservationContext(this.input, builder -> {
9696
});
97-
assertThat(this.convention.getHighCardinalityKeyValues(context)).contains(KeyValue.of("execution.id", "42"));
97+
assertThat(this.convention.getHighCardinalityKeyValues(context)).contains(KeyValue.of("graphql.execution.id", "42"));
9898
}
9999

100100

‎spring-graphql/src/test/java/org/springframework/graphql/observation/GraphQlObservationInstrumentationTests.java

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -61,16 +61,16 @@ void instrumentGraphQlRequestWhenSuccess() {
6161
.execute(TestExecutionRequest.forDocument(document));
6262
ResponseHelper response = ResponseHelper.forResponse(responseMono);
6363
TestObservationRegistryAssert.assertThat(this.observationRegistry).hasObservationWithNameEqualTo("graphql.request")
64-
.that().hasLowCardinalityKeyValue("outcome", "SUCCESS")
65-
.hasHighCardinalityKeyValueWithKey("execution.id");
64+
.that().hasLowCardinalityKeyValue("graphql.outcome", "SUCCESS")
65+
.hasHighCardinalityKeyValueWithKey("graphql.execution.id");
6666

6767
TestObservationRegistryAssert.assertThat(this.observationRegistry)
6868
.hasNumberOfObservationsWithNameEqualTo("graphql.datafetcher", 1)
6969
.hasObservationWithNameEqualTo("graphql.datafetcher")
7070
.that()
71-
.hasLowCardinalityKeyValue("outcome", "SUCCESS")
72-
.hasLowCardinalityKeyValue("field.name", "bookById")
73-
.hasHighCardinalityKeyValue("field.path", "/bookById");
71+
.hasLowCardinalityKeyValue("graphql.outcome", "SUCCESS")
72+
.hasLowCardinalityKeyValue("graphql.field.name", "bookById")
73+
.hasHighCardinalityKeyValue("graphql.field.path", "/bookById");
7474
}
7575

7676
@Test
@@ -82,8 +82,8 @@ void instrumentGraphQlRequestWhenInvalidRequest() {
8282
.execute(TestExecutionRequest.forDocument(document));
8383
ResponseHelper response = ResponseHelper.forResponse(responseMono);
8484
TestObservationRegistryAssert.assertThat(this.observationRegistry).hasObservationWithNameEqualTo("graphql.request")
85-
.that().hasLowCardinalityKeyValue("outcome", "REQUEST_ERROR")
86-
.hasHighCardinalityKeyValueWithKey("execution.id");
85+
.that().hasLowCardinalityKeyValue("graphql.outcome", "REQUEST_ERROR")
86+
.hasHighCardinalityKeyValueWithKey("graphql.execution.id");
8787

8888
TestObservationRegistryAssert.assertThat(this.observationRegistry)
8989
.hasNumberOfObservationsWithNameEqualTo("graphql.datafetcher", 0);
@@ -108,22 +108,22 @@ void instrumentMultipleDataFetcherOperations() {
108108
.execute(TestExecutionRequest.forDocument(document));
109109
ResponseHelper response = ResponseHelper.forResponse(responseMono);
110110
TestObservationRegistryAssert.assertThat(this.observationRegistry).hasObservationWithNameEqualTo("graphql.request")
111-
.that().hasLowCardinalityKeyValue("outcome", "SUCCESS")
112-
.hasHighCardinalityKeyValueWithKey("execution.id");
111+
.that().hasLowCardinalityKeyValue("graphql.outcome", "SUCCESS")
112+
.hasHighCardinalityKeyValueWithKey("graphql.execution.id");
113113

114114
TestObservationRegistryAssert.assertThat(this.observationRegistry)
115115
.hasNumberOfObservationsWithNameEqualTo("graphql.datafetcher", 2);
116116

117117
TestObservationRegistryAssert.assertThat(this.observationRegistry)
118118
.hasObservationWithNameEqualTo("graphql.datafetcher")
119119
.that()
120-
.hasLowCardinalityKeyValue("outcome", "SUCCESS")
121-
.hasLowCardinalityKeyValue("field.name", "bookById")
122-
.hasHighCardinalityKeyValue("field.path", "/bookById");
120+
.hasLowCardinalityKeyValue("graphql.outcome", "SUCCESS")
121+
.hasLowCardinalityKeyValue("graphql.field.name", "bookById")
122+
.hasHighCardinalityKeyValue("graphql.field.path", "/bookById");
123123

124124
TestObservationRegistryAssert.assertThat(this.observationRegistry)
125-
.hasAnObservationWithAKeyValue("field.name", "author")
126-
.hasAnObservationWithAKeyValue("field.path", "/bookById/author");
125+
.hasAnObservationWithAKeyValue("graphql.field.name", "author")
126+
.hasAnObservationWithAKeyValue("graphql.field.path", "/bookById/author");
127127
}
128128

129129
@Test
@@ -148,17 +148,17 @@ void instrumentGraphQlRequestWhenDataFetchingFailure() {
148148
.execute(TestExecutionRequest.forDocument(document));
149149
ResponseHelper response = ResponseHelper.forResponse(responseMono);
150150
TestObservationRegistryAssert.assertThat(this.observationRegistry).hasObservationWithNameEqualTo("graphql.request")
151-
.that().hasLowCardinalityKeyValue("outcome", "REQUEST_ERROR")
152-
.hasHighCardinalityKeyValueWithKey("execution.id");
151+
.that().hasLowCardinalityKeyValue("graphql.outcome", "REQUEST_ERROR")
152+
.hasHighCardinalityKeyValueWithKey("graphql.execution.id");
153153

154154
TestObservationRegistryAssert.assertThat(this.observationRegistry)
155155
.hasNumberOfObservationsWithNameEqualTo("graphql.datafetcher", 1)
156156
.hasObservationWithNameEqualTo("graphql.datafetcher")
157157
.that()
158-
.hasLowCardinalityKeyValue("outcome", "ERROR")
159-
.hasLowCardinalityKeyValue("error.type", "IllegalStateException")
160-
.hasLowCardinalityKeyValue("field.name", "bookById")
161-
.hasHighCardinalityKeyValue("field.path", "/bookById");
158+
.hasLowCardinalityKeyValue("graphql.outcome", "ERROR")
159+
.hasLowCardinalityKeyValue("graphql.error.type", "IllegalStateException")
160+
.hasLowCardinalityKeyValue("graphql.field.name", "bookById")
161+
.hasHighCardinalityKeyValue("graphql.field.path", "/bookById");
162162
}
163163

164164
@Test
@@ -177,8 +177,8 @@ void propagatesContextBetweenObservations() {
177177
ResponseHelper response = ResponseHelper.forResponse(responseMono);
178178

179179
TestObservationRegistryAssert.assertThat(this.observationRegistry).hasObservationWithNameEqualTo("graphql.request")
180-
.that().hasLowCardinalityKeyValue("outcome", "SUCCESS")
181-
.hasHighCardinalityKeyValueWithKey("execution.id");
180+
.that().hasLowCardinalityKeyValue("graphql.outcome", "SUCCESS")
181+
.hasHighCardinalityKeyValueWithKey("graphql.execution.id");
182182

183183
TestObservationRegistryAssert.assertThat(this.observationRegistry)
184184
.hasObservationWithNameEqualTo("graphql.datafetcher")

0 commit comments

Comments
 (0)