Skip to content

Commit

Permalink
Update GraphQL instrumentation to match spec (#6179)
Browse files Browse the repository at this point in the history 
* Update GraphQL instrumentation to match spec

* add back removed method as deprecated

* revert accidental change

* change method order
  • Loading branch information
@laurit
laurit authored Jun 17, 2022
1 parent 84aa843 commit c2f9e7a
Show file tree
Hide file tree
Showing 9 changed files with 34 additions and 56 deletions.
1 change: 0 additions & 1 deletion instrumentation/graphql-java-12.0/javaagent/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,4 @@

| System property | Type | Default | Description |
|---|---|---------|--------------------------------------------------------------------------------------------|
| `otel.instrumentation.graphql.experimental-span-attributes` | Boolean | `false` | Enable the capture of experimental span attributes. |
| `otel.instrumentation.graphql.query-sanitizer.enabled` | Boolean | `true` | Whether to remove sensitive information from query source that is added as span attribute. |
4 changes: 0 additions & 4 deletions instrumentation/graphql-java-12.0/javaagent/build.gradle.kts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,3 @@ dependencies {

testImplementation(project(":instrumentation:graphql-java-12.0:testing"))
}

tasks.withType<Test>().configureEach {
jvmArgs("-Dotel.instrumentation.graphql.experimental-span-attributes=true")
}
3 changes: 0 additions & 3 deletions ...t/src/main/java/io/opentelemetry/javaagent/instrumentation/graphql/GraphqlSingletons.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,9 @@ public final class GraphqlSingletons {

private static final boolean QUERY_SANITIZATION_ENABLED =
Config.get().getBoolean("otel.instrumentation.graphql.query-sanitizer.enabled", true);
private static final boolean CAPTURE_EXPERIMENTAL_SPAN_ATTRIBUTES =
Config.get().getBoolean("otel.instrumentation.graphql.experimental-span-attributes", false);

private static final GraphQLTelemetry TELEMETRY =
GraphQLTelemetry.builder(GlobalOpenTelemetry.get())
.setCaptureExperimentalSpanAttributes(CAPTURE_EXPERIMENTAL_SPAN_ATTRIBUTES)
.setSanitizeQuery(QUERY_SANITIZATION_ENABLED)
.build();

Expand Down
16 changes: 4 additions & 12 deletions ...12.0/library/src/main/java/io/opentelemetry/instrumentation/graphql/GraphQLTelemetry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,12 @@ public static GraphQLTelemetryBuilder builder(OpenTelemetry openTelemetry) {
}

private final Instrumenter<InstrumentationExecutionParameters, ExecutionResult> instrumenter;
private final boolean captureExperimentalSpanAttributes;
private final boolean sanitizeQuery;

GraphQLTelemetry(
OpenTelemetry openTelemetry,
boolean captureExperimentalSpanAttributes,
boolean sanitizeQuery) {
GraphQLTelemetry(OpenTelemetry openTelemetry, boolean sanitizeQuery) {
InstrumenterBuilder<InstrumentationExecutionParameters, ExecutionResult> builder =
Instrumenter.<InstrumentationExecutionParameters, ExecutionResult>builder(
openTelemetry, INSTRUMENTATION_NAME, ignored -> "GraphQL Query")
openTelemetry, INSTRUMENTATION_NAME, ignored -> "GraphQL Operation")
.setSpanStatusExtractor(
(spanStatusBuilder, instrumentationExecutionParameters, executionResult, error) -> {
if (!executionResult.getErrors().isEmpty()) {
Expand All @@ -54,20 +50,16 @@ public static GraphQLTelemetryBuilder builder(OpenTelemetry openTelemetry) {
error);
}
});
if (captureExperimentalSpanAttributes) {
builder.addAttributesExtractor(new ExperimentalAttributesExtractor());
}
builder.addAttributesExtractor(new GraphqlAttributesExtractor());

this.instrumenter = builder.newInstrumenter();
this.captureExperimentalSpanAttributes = captureExperimentalSpanAttributes;
this.sanitizeQuery = sanitizeQuery;
}

/**
* Returns a new {@link Instrumentation} that generates telemetry for received GraphQL requests.
*/
public Instrumentation newInstrumentation() {
return new OpenTelemetryInstrumentation(
instrumenter, captureExperimentalSpanAttributes, sanitizeQuery);
return new OpenTelemetryInstrumentation(instrumenter, sanitizeQuery);
}
}
5 changes: 2 additions & 3 deletions ...brary/src/main/java/io/opentelemetry/instrumentation/graphql/GraphQLTelemetryBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ public final class GraphQLTelemetryBuilder {

private final OpenTelemetry openTelemetry;

private boolean captureExperimentalSpanAttributes;
private boolean sanitizeQuery = true;

GraphQLTelemetryBuilder(OpenTelemetry openTelemetry) {
Expand All @@ -25,9 +24,9 @@ public final class GraphQLTelemetryBuilder {
* removed in the future, so only enable this if you know you do not require attributes filled by
* this instrumentation to be stable across versions.
*/
@Deprecated
public GraphQLTelemetryBuilder setCaptureExperimentalSpanAttributes(
boolean captureExperimentalSpanAttributes) {
this.captureExperimentalSpanAttributes = captureExperimentalSpanAttributes;
return this;
}

Expand All @@ -42,6 +41,6 @@ public GraphQLTelemetryBuilder setSanitizeQuery(boolean sanitizeQuery) {
* GraphQLTelemetryBuilder}.
*/
public GraphQLTelemetry build() {
return new GraphQLTelemetry(openTelemetry, captureExperimentalSpanAttributes, sanitizeQuery);
return new GraphQLTelemetry(openTelemetry, sanitizeQuery);
}
}
13 changes: 7 additions & 6 deletions ...phql/ExperimentalAttributesExtractor.java → ...n/graphql/GraphqlAttributesExtractor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,18 @@
import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
import java.util.Locale;
import javax.annotation.Nullable;

final class ExperimentalAttributesExtractor
final class GraphqlAttributesExtractor
implements AttributesExtractor<InstrumentationExecutionParameters, ExecutionResult> {
// https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-graphql/src/enums/AttributeNames.ts
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/instrumentation/graphql.md
private static final AttributeKey<String> OPERATION_NAME =
AttributeKey.stringKey("graphql.operation.name");
private static final AttributeKey<String> OPERATION_TYPE =
AttributeKey.stringKey("graphql.operation.type");
private static final AttributeKey<String> GRAPHQL_SOURCE =
AttributeKey.stringKey("graphql.source");
private static final AttributeKey<String> GRAPHQL_DOCUMENT =
AttributeKey.stringKey("graphql.document");

@Override
public void onStart(
Expand All @@ -39,8 +40,8 @@ public void onEnd(
OpenTelemetryInstrumentationState state = request.getInstrumentationState();
attributes.put(OPERATION_NAME, state.getOperationName());
if (state.getOperation() != null) {
attributes.put(OPERATION_TYPE, state.getOperation().name());
attributes.put(OPERATION_TYPE, state.getOperation().name().toLowerCase(Locale.ROOT));
}
attributes.put(GRAPHQL_SOURCE, state.getQuery());
attributes.put(GRAPHQL_DOCUMENT, state.getQuery());
}
}
21 changes: 9 additions & 12 deletions .../src/main/java/io/opentelemetry/instrumentation/graphql/OpenTelemetryInstrumentation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,19 @@
import io.opentelemetry.context.Scope;
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
import java.util.Locale;

final class OpenTelemetryInstrumentation extends SimpleInstrumentation {
private static final NodeVisitor sanitizingVisitor = new SanitizingVisitor();
private static final AstTransformer astTransformer = new AstTransformer();

private final Instrumenter<InstrumentationExecutionParameters, ExecutionResult> instrumenter;
private final boolean captureExperimentalSpanAttributes;
private final boolean sanitizeQuery;

OpenTelemetryInstrumentation(
Instrumenter<InstrumentationExecutionParameters, ExecutionResult> instrumenter,
boolean captureExperimentalSpanAttributes,
boolean sanitizeQuery) {
this.instrumenter = instrumenter;
this.captureExperimentalSpanAttributes = captureExperimentalSpanAttributes;
this.sanitizeQuery = sanitizeQuery;
}

Expand Down Expand Up @@ -98,24 +96,23 @@ public InstrumentationContext<ExecutionResult> beginExecuteOperation(
OperationDefinition operationDefinition =
parameters.getExecutionContext().getOperationDefinition();
Operation operation = operationDefinition.getOperation();
String operationType = operation.name().toLowerCase(Locale.ROOT);
String operationName = operationDefinition.getName();

String spanName = operation.name();
String spanName = operationType;
if (operationName != null && !operationName.isEmpty()) {
spanName += " " + operationName;
}
span.updateName(spanName);

if (captureExperimentalSpanAttributes) {
state.setOperation(operation);
state.setOperationName(operationName);
state.setOperation(operation);
state.setOperationName(operationName);

Node<?> node = operationDefinition;
if (sanitizeQuery) {
node = sanitize(node);
}
state.setQuery(AstPrinter.printAst(node));
Node<?> node = operationDefinition;
if (sanitizeQuery) {
node = sanitize(node);
}
state.setQuery(AstPrinter.printAst(node));

return SimpleInstrumentationContext.noOp();
}
Expand Down
5 changes: 1 addition & 4 deletions ...java-12.0/library/src/test/java/io/opentelemetry/instrumentation/graphql/GraphqlTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,7 @@ protected InstrumentationExtension getTesting() {

@Override
protected void configure(GraphQL.Builder builder) {
GraphQLTelemetry telemetry =
GraphQLTelemetry.builder(testing.getOpenTelemetry())
.setCaptureExperimentalSpanAttributes(true)
.build();
GraphQLTelemetry telemetry = GraphQLTelemetry.builder(testing.getOpenTelemetry()).build();
builder.instrumentation(telemetry.newInstrumentation());
}
}
22 changes: 11 additions & 11 deletions ...0/testing/src/main/java/io/opentelemetry/instrumentation/graphql/AbstractGraphqlTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,16 +140,16 @@ void successfulQuery() {
trace ->
trace.hasSpansSatisfyingExactly(
span ->
span.hasName("QUERY findBookById")
span.hasName("query findBookById")
.hasKind(SpanKind.INTERNAL)
.hasNoParent()
.hasAttributesSatisfyingExactly(
equalTo(
AttributeKey.stringKey("graphql.operation.name"),
"findBookById"),
equalTo(AttributeKey.stringKey("graphql.operation.type"), "QUERY"),
equalTo(AttributeKey.stringKey("graphql.operation.type"), "query"),
normalizedQueryEqualsTo(
AttributeKey.stringKey("graphql.source"),
AttributeKey.stringKey("graphql.document"),
"query findBookById { bookById(id: ?) { name } }")),
span -> span.hasName("fetchBookById").hasParent(trace.getSpan(0))));
}
Expand All @@ -172,13 +172,13 @@ void successfulQueryWithoutName() {
trace ->
trace.hasSpansSatisfyingExactly(
span ->
span.hasName("QUERY")
span.hasName("query")
.hasKind(SpanKind.INTERNAL)
.hasNoParent()
.hasAttributesSatisfyingExactly(
equalTo(AttributeKey.stringKey("graphql.operation.type"), "QUERY"),
equalTo(AttributeKey.stringKey("graphql.operation.type"), "query"),
normalizedQueryEqualsTo(
AttributeKey.stringKey("graphql.source"),
AttributeKey.stringKey("graphql.document"),
"query { bookById(id: ?) { name } }")),
span -> span.hasName("fetchBookById").hasParent(trace.getSpan(0))));
}
Expand All @@ -195,7 +195,7 @@ void parseError() {
trace ->
trace.hasSpansSatisfyingExactly(
span ->
span.hasName("GraphQL Query")
span.hasName("GraphQL Operation")
.hasKind(SpanKind.INTERNAL)
.hasNoParent()
.hasAttributesSatisfying(Attributes::isEmpty)
Expand Down Expand Up @@ -241,7 +241,7 @@ void validationError() {
trace ->
trace.hasSpansSatisfyingExactly(
span ->
span.hasName("GraphQL Query")
span.hasName("GraphQL Operation")
.hasKind(SpanKind.INTERNAL)
.hasNoParent()
.hasAttributesSatisfying(Attributes::isEmpty)
Expand Down Expand Up @@ -286,16 +286,16 @@ void successfulMutation() {
trace ->
trace.hasSpansSatisfyingExactly(
span ->
span.hasName("MUTATION addNewBook")
span.hasName("mutation addNewBook")
.hasKind(SpanKind.INTERNAL)
.hasNoParent()
.hasAttributesSatisfyingExactly(
equalTo(
AttributeKey.stringKey("graphql.operation.name"), "addNewBook"),
equalTo(
AttributeKey.stringKey("graphql.operation.type"), "MUTATION"),
AttributeKey.stringKey("graphql.operation.type"), "mutation"),
normalizedQueryEqualsTo(
AttributeKey.stringKey("graphql.source"),
AttributeKey.stringKey("graphql.document"),
"mutation addNewBook { addBook(id: ?, name: ?, author: ?) { id } }"))));
}

Expand Down

0 comments on commit c2f9e7a

Please sign in to comment.