Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions instrumentation/graphql-java/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
|------------------------------------------------------------------------|---------|---------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `otel.instrumentation.graphql.query-sanitizer.enabled` | Boolean | `true` | Whether to remove sensitive information from query source that is added as span attribute. |
| `otel.instrumentation.graphql.add-operation-name-to-span-name.enabled` | Boolean | `false` | Whether GraphQL operation name is added to the span name. <p>**WARNING**: GraphQL operation name is provided by the client and can have high cardinality. Use only when the server is not exposed to malicious clients. |
| `otel.instrumentation.graphql.capture-query` | Boolean | `true` | Whether to capture the query in `graphql.document` span attribute. |

# Settings for the GraphQL 20 instrumentation

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@

public final class GraphqlSingletons {

private static final boolean CAPTURE_QUERY =
AgentInstrumentationConfig.get()
.getBoolean("otel.instrumentation.graphql.capture-query", true);
private static final boolean QUERY_SANITIZATION_ENABLED =
AgentInstrumentationConfig.get()
.getBoolean("otel.instrumentation.graphql.query-sanitizer.enabled", true);
Expand All @@ -23,6 +26,7 @@ public final class GraphqlSingletons {

private static final GraphQLTelemetry TELEMETRY =
GraphQLTelemetry.builder(GlobalOpenTelemetry.get())
.setCaptureQuery(CAPTURE_QUERY)
.setSanitizeQuery(QUERY_SANITIZATION_ENABLED)
.setAddOperationNameToSpanName(ADD_OPERATION_NAME_TO_SPAN_NAME)
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,17 @@ public static GraphQLTelemetryBuilder builder(OpenTelemetry openTelemetry) {
private final OpenTelemetryInstrumentationHelper helper;

GraphQLTelemetry(
OpenTelemetry openTelemetry, boolean sanitizeQuery, boolean addOperationNameToSpanName) {
OpenTelemetry openTelemetry,
boolean captureQuery,
boolean sanitizeQuery,
boolean addOperationNameToSpanName) {
helper =
OpenTelemetryInstrumentationHelper.create(
openTelemetry, INSTRUMENTATION_NAME, sanitizeQuery, addOperationNameToSpanName);
openTelemetry,
INSTRUMENTATION_NAME,
captureQuery,
sanitizeQuery,
addOperationNameToSpanName);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,24 @@ public final class GraphQLTelemetryBuilder {

private final OpenTelemetry openTelemetry;

private boolean captureQuery = true;
private boolean sanitizeQuery = true;
private boolean addOperationNameToSpanName = false;

GraphQLTelemetryBuilder(OpenTelemetry openTelemetry) {
this.openTelemetry = openTelemetry;
}

/**
* Sets whether query should be captured in {@code graphql.document} span attribute. Default is
* {@code true}.
*/
@CanIgnoreReturnValue
public GraphQLTelemetryBuilder setCaptureQuery(boolean captureQuery) {
this.captureQuery = captureQuery;
return this;
}

/** Sets whether sensitive information should be removed from queries. Default is {@code true}. */
@CanIgnoreReturnValue
public GraphQLTelemetryBuilder setSanitizeQuery(boolean sanitizeQuery) {
Expand All @@ -45,6 +56,7 @@ public GraphQLTelemetryBuilder setAddOperationNameToSpanName(boolean addOperatio
* GraphQLTelemetryBuilder}.
*/
public GraphQLTelemetry build() {
return new GraphQLTelemetry(openTelemetry, sanitizeQuery, addOperationNameToSpanName);
return new GraphQLTelemetry(
openTelemetry, captureQuery, sanitizeQuery, addOperationNameToSpanName);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@

public final class GraphqlSingletons {

private static final boolean CAPTURE_QUERY =
AgentInstrumentationConfig.get()
.getBoolean("otel.instrumentation.graphql.capture-query", true);
private static final boolean QUERY_SANITIZATION_ENABLED =
AgentInstrumentationConfig.get()
.getBoolean("otel.instrumentation.graphql.query-sanitizer.enabled", true);
Expand All @@ -29,6 +32,7 @@ public final class GraphqlSingletons {

private static final GraphQLTelemetry TELEMETRY =
GraphQLTelemetry.builder(GlobalOpenTelemetry.get())
.setCaptureQuery(CAPTURE_QUERY)
.setSanitizeQuery(QUERY_SANITIZATION_ENABLED)
.setDataFetcherInstrumentationEnabled(DATA_FETCHER_ENABLED)
.setTrivialDataFetcherInstrumentationEnabled(TRIVIAL_DATA_FETCHER_ENABLED)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,14 @@ public static GraphQLTelemetryBuilder builder(OpenTelemetry openTelemetry) {

GraphQLTelemetry(
OpenTelemetry openTelemetry,
boolean captureQuery,
boolean sanitizeQuery,
Instrumenter<DataFetchingEnvironment, Object> dataFetcherInstrumenter,
boolean createSpansForTrivialDataFetcher,
boolean addOperationNameToSpanName) {
helper =
GraphqlInstrumenterFactory.createInstrumentationHelper(
openTelemetry, sanitizeQuery, addOperationNameToSpanName);
openTelemetry, captureQuery, sanitizeQuery, addOperationNameToSpanName);
this.dataFetcherInstrumenter = dataFetcherInstrumenter;
this.createSpansForTrivialDataFetcher = createSpansForTrivialDataFetcher;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ public final class GraphQLTelemetryBuilder {

private final OpenTelemetry openTelemetry;

private boolean captureQuery = true;
private boolean sanitizeQuery = true;
private boolean dataFetcherInstrumentationEnabled = false;
private boolean trivialDataFetcherInstrumentationEnabled = false;
Expand All @@ -23,6 +24,16 @@ public final class GraphQLTelemetryBuilder {
this.openTelemetry = openTelemetry;
}

/**
* Sets whether query should be captured in {@code graphql.document} span attribute. Default is
* {@code true}.
*/
@CanIgnoreReturnValue
public GraphQLTelemetryBuilder setCaptureQuery(boolean captureQuery) {
this.captureQuery = captureQuery;
return this;
}

/** Sets whether sensitive information should be removed from queries. Default is {@code true}. */
@CanIgnoreReturnValue
public GraphQLTelemetryBuilder setSanitizeQuery(boolean sanitizeQuery) {
Expand Down Expand Up @@ -68,6 +79,7 @@ public GraphQLTelemetryBuilder setAddOperationNameToSpanName(boolean addOperatio
public GraphQLTelemetry build() {
return new GraphQLTelemetry(
openTelemetry,
captureQuery,
sanitizeQuery,
GraphqlInstrumenterFactory.createDataFetcherInstrumenter(
openTelemetry, dataFetcherInstrumentationEnabled),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,16 @@ final class GraphqlInstrumenterFactory {
private static final String INSTRUMENTATION_NAME = "io.opentelemetry.graphql-java-20.0";

static OpenTelemetryInstrumentationHelper createInstrumentationHelper(
OpenTelemetry openTelemetry, boolean sanitizeQuery, boolean addOperationNameToSpanName) {
OpenTelemetry openTelemetry,
boolean captureQuery,
boolean sanitizeQuery,
boolean addOperationNameToSpanName) {
return OpenTelemetryInstrumentationHelper.create(
openTelemetry, INSTRUMENTATION_NAME, sanitizeQuery, addOperationNameToSpanName);
openTelemetry,
INSTRUMENTATION_NAME,
captureQuery,
sanitizeQuery,
addOperationNameToSpanName);
}

static Instrumenter<DataFetchingEnvironment, Object> createDataFetcherInstrumenter(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ public void onEnd(
attributes.put(
GRAPHQL_OPERATION_TYPE, request.getOperation().name().toLowerCase(Locale.ROOT));
}
attributes.put(GRAPHQL_DOCUMENT, request.getQuery());
if (request.getQuery() != null) {
attributes.put(GRAPHQL_DOCUMENT, request.getQuery());
}
Comment on lines -48 to +50
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think put null is a no-op

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In many places we use AttributesExtractorUtil.internalSet that skips null values. I suspect that the marshaling in sdk does not skip nulls, but didn't verify.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we originally did that because it was previously undocumented behavior, but that changed with open-telemetry/opentelemetry-java#7271, probably we could revisit those places now, but not a priority

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,21 +47,25 @@ public final class OpenTelemetryInstrumentationHelper {
private static final AstTransformer astTransformer = new AstTransformer();

private final Instrumenter<OpenTelemetryInstrumentationState, ExecutionResult> instrumenter;
private final boolean captureQuery;
private final boolean sanitizeQuery;
private final boolean addOperationNameToSpanName;

private OpenTelemetryInstrumentationHelper(
Instrumenter<OpenTelemetryInstrumentationState, ExecutionResult> instrumenter,
boolean captureQuery,
boolean sanitizeQuery,
boolean addOperationNameToSpanName) {
this.instrumenter = instrumenter;
this.captureQuery = captureQuery;
this.sanitizeQuery = sanitizeQuery;
this.addOperationNameToSpanName = addOperationNameToSpanName;
}

public static OpenTelemetryInstrumentationHelper create(
OpenTelemetry openTelemetry,
String instrumentationName,
boolean captureQuery,
boolean sanitizeQuery,
boolean addOperationNameToSpanName) {
InstrumenterBuilder<OpenTelemetryInstrumentationState, ExecutionResult> builder =
Expand All @@ -83,7 +87,7 @@ public static OpenTelemetryInstrumentationHelper create(
builder.addAttributesExtractor(new GraphqlAttributesExtractor());

return new OpenTelemetryInstrumentationHelper(
builder.buildInstrumenter(), sanitizeQuery, addOperationNameToSpanName);
builder.buildInstrumenter(), captureQuery, sanitizeQuery, addOperationNameToSpanName);
}

public InstrumentationContext<ExecutionResult> beginExecution(
Expand Down Expand Up @@ -133,11 +137,13 @@ public InstrumentationContext<ExecutionResult> beginExecuteOperation(
state.setOperation(operation);
state.setOperationName(operationName);

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

return SimpleInstrumentationContext.noOp();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -356,11 +356,22 @@ protected static AttributeAssertion normalizedQueryEqualsTo(
stringAssert ->
stringAssert.satisfies(
querySource -> {
String normalized = querySource.replaceAll("(?s)\\s+", " ");
if (normalized.startsWith("query {")) {
normalized = normalized.substring("query ".length());
}
assertThat(normalized).isEqualTo(value);
String normalized = normalizeQuery(querySource);
String valueNormalized = normalizeQuery(value);
assertThat(normalized).isEqualTo(valueNormalized);
}));
}

private static String normalizeQuery(String query) {
if (query == null) {
return null;
}

String normalized =
query.replaceAll("(?s)\\s+", " ").replaceAll("([{:,]) ", "$1").replaceAll(" ([{}])", "$1");
if (normalized.startsWith("query{")) {
normalized = normalized.substring("query".length());
}
return normalized;
}
}
Loading