Spring/spring-graphql
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Align observation convention with Micrometer best practices

Browse Source 
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
This commit is contained in:
Brian Clozel
2022-12-02 09:04:10 +01:00
parent 5fe8123126
commit 5b4eb8cb73
6 changed files with 55 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
spring-graphql/src/main/java/org/springframework/graphql/observation/DefaultDataFetcherObservationConvention.java
View File

@@ -55,7 +55,7 @@ public class DefaultDataFetcherObservationConvention implements DataFetcherObser
@Override
public String getContextualName(DataFetcherObservationContext context) {
return "graphQL field " + context.getEnvironment().getField().getName();
return "graphql field " + context.getEnvironment().getField().getName();
}
@Override

5
spring-graphql/src/main/java/org/springframework/graphql/observation/DefaultExecutionRequestObservationConvention.java
View File

@@ -33,7 +33,7 @@ public class DefaultExecutionRequestObservationConvention implements ExecutionRe
private static final String DEFAULT_NAME = "graphql.request";
private static final String BASE_CONTEXTUAL_NAME = "graphQL ";
private static final String BASE_CONTEXTUAL_NAME = "graphql ";
private static final KeyValue OUTCOME_SUCCESS = KeyValue.of(ExecutionRequestLowCardinalityKeyNames.OUTCOME, "SUCCESS");
@@ -60,7 +60,8 @@ public class DefaultExecutionRequestObservationConvention implements ExecutionRe
@Override
public String getContextualName(ExecutionRequestObservationContext context) {
return BASE_CONTEXTUAL_NAME + context.getCarrier().getOperationName();
String operationName = (context.getCarrier().getOperationName() != null) ? context.getCarrier().getOperationName() : "query";
return BASE_CONTEXTUAL_NAME + operationName;
}
@Override

26
spring-graphql/src/main/java/org/springframework/graphql/observation/GraphQlObservationDocumentation.java
View File

@@ -36,6 +36,12 @@ public enum GraphQlObservationDocumentation implements ObservationDocumentation
* Observation created for GraphQL execution requests.
*/
EXECUTION_REQUEST {
@Override
public String getPrefix() {
return "graphql";
}
@Override
public Class<? extends ObservationConvention<? extends Observation.Context>> getDefaultConvention() {
return DefaultExecutionRequestObservationConvention.class;
@@ -58,6 +64,12 @@ public enum GraphQlObservationDocumentation implements ObservationDocumentation
* data fetching operations.
*/
DATA_FETCHER {
@Override
public String getPrefix() {
return "graphql";
}
@Override
public Class<? extends ObservationConvention<? extends Observation.Context>> getDefaultConvention() {
return DefaultDataFetcherObservationConvention.class;
@@ -77,7 +89,7 @@ public enum GraphQlObservationDocumentation implements ObservationDocumentation
OUTCOME {
@Override
public String asString() {
return "outcome";
return "graphql.outcome";
}
},
@@ -87,7 +99,7 @@ public enum GraphQlObservationDocumentation implements ObservationDocumentation
OPERATION {
@Override
public String asString() {
return "operation";
return "graphql.operation";
}
}
}
@@ -100,7 +112,7 @@ public enum GraphQlObservationDocumentation implements ObservationDocumentation
EXECUTION_ID {
@Override
public String asString() {
return "execution.id";
return "graphql.execution.id";
}
}
}
@@ -113,7 +125,7 @@ public enum GraphQlObservationDocumentation implements ObservationDocumentation
OUTCOME {
@Override
public String asString() {
return "outcome";
return "graphql.outcome";
}
},
@@ -123,7 +135,7 @@ public enum GraphQlObservationDocumentation implements ObservationDocumentation
FIELD_NAME {
@Override
public String asString() {
return "field.name";
return "graphql.field.name";
}
},
@@ -133,7 +145,7 @@ public enum GraphQlObservationDocumentation implements ObservationDocumentation
ERROR_TYPE {
@Override
public String asString() {
return "error.type";
return "graphql.error.type";
}
}
@@ -147,7 +159,7 @@ public enum GraphQlObservationDocumentation implements ObservationDocumentation
FIELD_PATH {
@Override
public String asString() {
return "field.path";
return "graphql.field.path";
}
}

8
spring-graphql/src/test/java/org/springframework/graphql/observation/DefaultDataFetcherObservationConventionTests.java
View File

@@ -59,7 +59,7 @@ class DefaultDataFetcherObservationConventionTests {
builder.mergedField(MergedField.newMergedField(Field.newField("project").build()).build());
});
DataFetcherObservationContext context = new DataFetcherObservationContext(environment);
assertThat(this.convention.getContextualName(context)).isEqualTo("graphQL field project");
assertThat(this.convention.getContextualName(context)).isEqualTo("graphql field project");
}
@Test
@@ -68,7 +68,7 @@ class DefaultDataFetcherObservationConventionTests {
builder.mergedField(MergedField.newMergedField(Field.newField("project").build()).build());
});
DataFetcherObservationContext context = new DataFetcherObservationContext(environment);
assertThat(this.convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("field.name", "project"));
assertThat(this.convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("graphql.field.name", "project"));
}
@Test
@@ -78,7 +78,7 @@ class DefaultDataFetcherObservationConventionTests {
});
DataFetcherObservationContext context = new DataFetcherObservationContext(environment);
context.setError(new IllegalStateException("custom data fetching failure"));
assertThat(this.convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("error.type", "IllegalStateException"));
assertThat(this.convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("graphql.error.type", "IllegalStateException"));
}
@Test
@@ -90,7 +90,7 @@ class DefaultDataFetcherObservationConventionTests {
});
DataFetcherObservationContext context = new DataFetcherObservationContext(environment);
context.setError(new IllegalStateException("custom data fetching failure"));
assertThat(this.convention.getHighCardinalityKeyValues(context)).contains(KeyValue.of("field.path", "/projectBySlug/releases"));
assertThat(this.convention.getHighCardinalityKeyValues(context)).contains(KeyValue.of("graphql.field.path", "/projectBySlug/releases"));
}
private DataFetchingEnvironment createDataFetchingEnvironment(Consumer<DataFetchingEnvironmentImpl.Builder> consumer) {

12
spring-graphql/src/test/java/org/springframework/graphql/observation/DefaultExecutionRequestObservationConventionTests.java
View File

@@ -58,28 +58,28 @@ class DefaultExecutionRequestObservationConventionTests {
ExecutionInput input = ExecutionInput.newExecutionInput().query("{ greeting }")
.operationName("mutation").build();
ExecutionRequestObservationContext context = createObservationContext(input, builder -> {});
assertThat(this.convention.getContextualName(context)).isEqualTo("graphQL mutation");
assertThat(this.convention.getContextualName(context)).isEqualTo("graphql mutation");
}
@Test
void hasOperationKeyValueWhenSuccessfulOutput() {
ExecutionRequestObservationContext context = createObservationContext(this.input, builder -> {
});
assertThat(this.convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("operation", "query"));
assertThat(this.convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("graphql.operation", "query"));
}
@Test
void hasOutcomeKeyValueWhenSuccessfulOutput() {
ExecutionRequestObservationContext context = createObservationContext(this.input, builder -> {
});
assertThat(this.convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("outcome", "SUCCESS"));
assertThat(this.convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("graphql.outcome", "SUCCESS"));
}
@Test
void hasOutcomeKeyValueWhenErrorOutput() {
ExecutionRequestObservationContext context = createObservationContext(this.input,
builder -> builder.addError(new QueryOperationMissingError()));
assertThat(this.convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("outcome", "REQUEST_ERROR"));
assertThat(this.convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("graphql.outcome", "REQUEST_ERROR"));
}
@Test
@@ -87,14 +87,14 @@ class DefaultExecutionRequestObservationConventionTests {
ExecutionRequestObservationContext context = createObservationContext(this.input, builder -> {
});
context.setError(new IllegalStateException("custom internal error"));
assertThat(this.convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("outcome", "INTERNAL_ERROR"));
assertThat(this.convention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("graphql.outcome", "INTERNAL_ERROR"));
}
@Test
void hasExecutionIdKeyValue() {
ExecutionRequestObservationContext context = createObservationContext(this.input, builder -> {
});
assertThat(this.convention.getHighCardinalityKeyValues(context)).contains(KeyValue.of("execution.id", "42"));
assertThat(this.convention.getHighCardinalityKeyValues(context)).contains(KeyValue.of("graphql.execution.id", "42"));
}

44
spring-graphql/src/test/java/org/springframework/graphql/observation/GraphQlObservationInstrumentationTests.java
View File

@@ -61,16 +61,16 @@ class GraphQlObservationInstrumentationTests {
.execute(TestExecutionRequest.forDocument(document));
ResponseHelper response = ResponseHelper.forResponse(responseMono);
TestObservationRegistryAssert.assertThat(this.observationRegistry).hasObservationWithNameEqualTo("graphql.request")
.that().hasLowCardinalityKeyValue("outcome", "SUCCESS")
.hasHighCardinalityKeyValueWithKey("execution.id");
.that().hasLowCardinalityKeyValue("graphql.outcome", "SUCCESS")
.hasHighCardinalityKeyValueWithKey("graphql.execution.id");
TestObservationRegistryAssert.assertThat(this.observationRegistry)
.hasNumberOfObservationsWithNameEqualTo("graphql.datafetcher", 1)
.hasObservationWithNameEqualTo("graphql.datafetcher")
.that()
.hasLowCardinalityKeyValue("outcome", "SUCCESS")
.hasLowCardinalityKeyValue("field.name", "bookById")
.hasHighCardinalityKeyValue("field.path", "/bookById");
.hasLowCardinalityKeyValue("graphql.outcome", "SUCCESS")
.hasLowCardinalityKeyValue("graphql.field.name", "bookById")
.hasHighCardinalityKeyValue("graphql.field.path", "/bookById");
}
@Test
@@ -82,8 +82,8 @@ class GraphQlObservationInstrumentationTests {
.execute(TestExecutionRequest.forDocument(document));
ResponseHelper response = ResponseHelper.forResponse(responseMono);
TestObservationRegistryAssert.assertThat(this.observationRegistry).hasObservationWithNameEqualTo("graphql.request")
.that().hasLowCardinalityKeyValue("outcome", "REQUEST_ERROR")
.hasHighCardinalityKeyValueWithKey("execution.id");
.that().hasLowCardinalityKeyValue("graphql.outcome", "REQUEST_ERROR")
.hasHighCardinalityKeyValueWithKey("graphql.execution.id");
TestObservationRegistryAssert.assertThat(this.observationRegistry)
.hasNumberOfObservationsWithNameEqualTo("graphql.datafetcher", 0);
@@ -108,8 +108,8 @@ class GraphQlObservationInstrumentationTests {
.execute(TestExecutionRequest.forDocument(document));
ResponseHelper response = ResponseHelper.forResponse(responseMono);
TestObservationRegistryAssert.assertThat(this.observationRegistry).hasObservationWithNameEqualTo("graphql.request")
.that().hasLowCardinalityKeyValue("outcome", "SUCCESS")
.hasHighCardinalityKeyValueWithKey("execution.id");
.that().hasLowCardinalityKeyValue("graphql.outcome", "SUCCESS")
.hasHighCardinalityKeyValueWithKey("graphql.execution.id");
TestObservationRegistryAssert.assertThat(this.observationRegistry)
.hasNumberOfObservationsWithNameEqualTo("graphql.datafetcher", 2);
@@ -117,13 +117,13 @@ class GraphQlObservationInstrumentationTests {
TestObservationRegistryAssert.assertThat(this.observationRegistry)
.hasObservationWithNameEqualTo("graphql.datafetcher")
.that()
.hasLowCardinalityKeyValue("outcome", "SUCCESS")
.hasLowCardinalityKeyValue("field.name", "bookById")
.hasHighCardinalityKeyValue("field.path", "/bookById");
.hasLowCardinalityKeyValue("graphql.outcome", "SUCCESS")
.hasLowCardinalityKeyValue("graphql.field.name", "bookById")
.hasHighCardinalityKeyValue("graphql.field.path", "/bookById");
TestObservationRegistryAssert.assertThat(this.observationRegistry)
.hasAnObservationWithAKeyValue("field.name", "author")
.hasAnObservationWithAKeyValue("field.path", "/bookById/author");
.hasAnObservationWithAKeyValue("graphql.field.name", "author")
.hasAnObservationWithAKeyValue("graphql.field.path", "/bookById/author");
}
@Test
@@ -148,17 +148,17 @@ class GraphQlObservationInstrumentationTests {
.execute(TestExecutionRequest.forDocument(document));
ResponseHelper response = ResponseHelper.forResponse(responseMono);
TestObservationRegistryAssert.assertThat(this.observationRegistry).hasObservationWithNameEqualTo("graphql.request")
.that().hasLowCardinalityKeyValue("outcome", "REQUEST_ERROR")
.hasHighCardinalityKeyValueWithKey("execution.id");
.that().hasLowCardinalityKeyValue("graphql.outcome", "REQUEST_ERROR")
.hasHighCardinalityKeyValueWithKey("graphql.execution.id");
TestObservationRegistryAssert.assertThat(this.observationRegistry)
.hasNumberOfObservationsWithNameEqualTo("graphql.datafetcher", 1)
.hasObservationWithNameEqualTo("graphql.datafetcher")
.that()
.hasLowCardinalityKeyValue("outcome", "ERROR")
.hasLowCardinalityKeyValue("error.type", "IllegalStateException")
.hasLowCardinalityKeyValue("field.name", "bookById")
.hasHighCardinalityKeyValue("field.path", "/bookById");
.hasLowCardinalityKeyValue("graphql.outcome", "ERROR")
.hasLowCardinalityKeyValue("graphql.error.type", "IllegalStateException")
.hasLowCardinalityKeyValue("graphql.field.name", "bookById")
.hasHighCardinalityKeyValue("graphql.field.path", "/bookById");
}
@Test
@@ -177,8 +177,8 @@ class GraphQlObservationInstrumentationTests {
ResponseHelper response = ResponseHelper.forResponse(responseMono);
TestObservationRegistryAssert.assertThat(this.observationRegistry).hasObservationWithNameEqualTo("graphql.request")
.that().hasLowCardinalityKeyValue("outcome", "SUCCESS")
.hasHighCardinalityKeyValueWithKey("execution.id");
.that().hasLowCardinalityKeyValue("graphql.outcome", "SUCCESS")
.hasHighCardinalityKeyValueWithKey("graphql.execution.id");
TestObservationRegistryAssert.assertThat(this.observationRegistry)
.hasObservationWithNameEqualTo("graphql.datafetcher")