Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve server side GraphQL support for spring-graphql and Nextflix DGS #2856

Merged
merged 21 commits into from
Aug 3, 2023
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@

- Add HTTP response code to Spring WebFlux transactions ([#2870](https://github.com/getsentry/sentry-java/pull/2870))
- Add `sampled` to Dynamic Sampling Context ([#2869](https://github.com/getsentry/sentry-java/pull/2869))
- Improve server side GraphQL support for spring-graphql and Nextflix DGS ([#2856](https://github.com/getsentry/sentry-java/pull/2856))
- If you have already been using `SentryDataFetcherExceptionHandler` that still works but has been deprecated. Please use `SentryGenericDataFetcherExceptionHandler` combined with `SentryInstrumentation` instead for better error reporting.
- More exceptions and errors caught and reported to Sentry by also looking at the `ExecutionResult` (more specifically its `errors`)
- More details for Sentry events: query, variables and response (where possible)
- Breadcrumbs for operation (query, mutation, subscription), data fetchers and data loaders (Spring only)
- Better hub propagation by using `GraphQLContext`

### Fixes

Expand Down
4 changes: 4 additions & 0 deletions buildSrc/src/main/java/Config.kt
Original file line number Diff line number Diff line change
Expand Up @@ -73,16 +73,20 @@ object Config {
val jacksonDatabind = "com.fasterxml.jackson.core:jackson-databind"

val springBootStarter = "org.springframework.boot:spring-boot-starter:$springBootVersion"
val springBootStarterGraphql = "org.springframework.boot:spring-boot-starter-graphql:$springBootVersion"
val springBootStarterTest = "org.springframework.boot:spring-boot-starter-test:$springBootVersion"
val springBootStarterWeb = "org.springframework.boot:spring-boot-starter-web:$springBootVersion"
val springBootStarterWebsocket = "org.springframework.boot:spring-boot-starter-websocket:$springBootVersion"
val springBootStarterWebflux = "org.springframework.boot:spring-boot-starter-webflux:$springBootVersion"
val springBootStarterAop = "org.springframework.boot:spring-boot-starter-aop:$springBootVersion"
val springBootStarterSecurity = "org.springframework.boot:spring-boot-starter-security:$springBootVersion"
val springBootStarterJdbc = "org.springframework.boot:spring-boot-starter-jdbc:$springBootVersion"

val springBoot3Starter = "org.springframework.boot:spring-boot-starter:$springBoot3Version"
val springBoot3StarterGraphql = "org.springframework.boot:spring-boot-starter-graphql:$springBoot3Version"
val springBoot3StarterTest = "org.springframework.boot:spring-boot-starter-test:$springBoot3Version"
val springBoot3StarterWeb = "org.springframework.boot:spring-boot-starter-web:$springBoot3Version"
val springBoot3StarterWebsocket = "org.springframework.boot:spring-boot-starter-websocket:$springBoot3Version"
val springBoot3StarterWebflux = "org.springframework.boot:spring-boot-starter-webflux:$springBoot3Version"
val springBoot3StarterAop = "org.springframework.boot:spring-boot-starter-aop:$springBoot3Version"
val springBoot3StarterSecurity = "org.springframework.boot:spring-boot-starter-security:$springBoot3Version"
Expand Down
48 changes: 48 additions & 0 deletions sentry-graphql/api/sentry-graphql.api
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,71 @@ public final class io/sentry/graphql/BuildConfig {
public static final field VERSION_NAME Ljava/lang/String;
}

public final class io/sentry/graphql/ExceptionReporter {
public fun <init> (Z)V
public fun captureThrowable (Ljava/lang/Throwable;Lio/sentry/graphql/ExceptionReporter$ExceptionDetails;Lgraphql/ExecutionResult;)V
}

public final class io/sentry/graphql/ExceptionReporter$ExceptionDetails {
public fun <init> (Lio/sentry/IHub;Lgraphql/execution/instrumentation/parameters/InstrumentationExecutionParameters;Z)V
public fun <init> (Lio/sentry/IHub;Lgraphql/schema/DataFetchingEnvironment;Z)V
public fun getHub ()Lio/sentry/IHub;
public fun getQuery ()Ljava/lang/String;
public fun getVariables ()Ljava/util/Map;
public fun isSubscription ()Z
}

public final class io/sentry/graphql/GraphqlStringUtils {
public fun <init> ()V
public static fun fieldToString (Lgraphql/execution/MergedField;)Ljava/lang/String;
public static fun objectTypeToString (Lgraphql/schema/GraphQLObjectType;)Ljava/lang/String;
public static fun typeToString (Lgraphql/schema/GraphQLOutputType;)Ljava/lang/String;
}

public final class io/sentry/graphql/NoOpSubscriptionHandler : io/sentry/graphql/SentrySubscriptionHandler {
public static fun getInstance ()Lio/sentry/graphql/NoOpSubscriptionHandler;
public fun onSubscriptionResult (Ljava/lang/Object;Lio/sentry/IHub;Lio/sentry/graphql/ExceptionReporter;Lgraphql/execution/instrumentation/parameters/InstrumentationFieldFetchParameters;)Ljava/lang/Object;
}

public final class io/sentry/graphql/SentryDataFetcherExceptionHandler : graphql/execution/DataFetcherExceptionHandler {
public fun <init> (Lgraphql/execution/DataFetcherExceptionHandler;)V
public fun <init> (Lio/sentry/IHub;Lgraphql/execution/DataFetcherExceptionHandler;)V
public fun onException (Lgraphql/execution/DataFetcherExceptionHandlerParameters;)Lgraphql/execution/DataFetcherExceptionHandlerResult;
}

public final class io/sentry/graphql/SentryGenericDataFetcherExceptionHandler : graphql/execution/DataFetcherExceptionHandler {
public fun <init> (Lgraphql/execution/DataFetcherExceptionHandler;)V
public fun <init> (Lio/sentry/IHub;Lgraphql/execution/DataFetcherExceptionHandler;)V
public fun onException (Lgraphql/execution/DataFetcherExceptionHandlerParameters;)Lgraphql/execution/DataFetcherExceptionHandlerResult;
}

public final class io/sentry/graphql/SentryGraphqlExceptionHandler {
public fun <init> (Lgraphql/execution/DataFetcherExceptionHandler;)V
public fun onException (Ljava/lang/Throwable;Lgraphql/schema/DataFetchingEnvironment;Lgraphql/execution/DataFetcherExceptionHandlerParameters;)Lgraphql/execution/DataFetcherExceptionHandlerResult;
}

public final class io/sentry/graphql/SentryInstrumentation : graphql/execution/instrumentation/SimpleInstrumentation {
public static final field SENTRY_EXCEPTIONS_CONTEXT_KEY Ljava/lang/String;
public static final field SENTRY_HUB_CONTEXT_KEY Ljava/lang/String;
public fun <init> ()V
public fun <init> (Lio/sentry/IHub;)V
public fun <init> (Lio/sentry/IHub;Lio/sentry/graphql/SentryInstrumentation$BeforeSpanCallback;)V
public fun <init> (Lio/sentry/graphql/SentryInstrumentation$BeforeSpanCallback;)V
public fun <init> (Lio/sentry/graphql/SentryInstrumentation$BeforeSpanCallback;Lio/sentry/graphql/SentrySubscriptionHandler;Lio/sentry/graphql/ExceptionReporter;)V
public fun <init> (Lio/sentry/graphql/SentryInstrumentation$BeforeSpanCallback;Lio/sentry/graphql/SentrySubscriptionHandler;Z)V
public fun <init> (Lio/sentry/graphql/SentrySubscriptionHandler;Z)V
public fun beginExecuteOperation (Lgraphql/execution/instrumentation/parameters/InstrumentationExecuteOperationParameters;)Lgraphql/execution/instrumentation/InstrumentationContext;
public fun beginExecution (Lgraphql/execution/instrumentation/parameters/InstrumentationExecutionParameters;)Lgraphql/execution/instrumentation/InstrumentationContext;
public fun createState ()Lgraphql/execution/instrumentation/InstrumentationState;
public fun instrumentDataFetcher (Lgraphql/schema/DataFetcher;Lgraphql/execution/instrumentation/parameters/InstrumentationFieldFetchParameters;)Lgraphql/schema/DataFetcher;
public fun instrumentExecutionResult (Lgraphql/ExecutionResult;Lgraphql/execution/instrumentation/parameters/InstrumentationExecutionParameters;)Ljava/util/concurrent/CompletableFuture;
}

public abstract interface class io/sentry/graphql/SentryInstrumentation$BeforeSpanCallback {
public abstract fun execute (Lio/sentry/ISpan;Lgraphql/schema/DataFetchingEnvironment;Ljava/lang/Object;)Lio/sentry/ISpan;
}

public abstract interface class io/sentry/graphql/SentrySubscriptionHandler {
public abstract fun onSubscriptionResult (Ljava/lang/Object;Lio/sentry/IHub;Lio/sentry/graphql/ExceptionReporter;Lgraphql/execution/instrumentation/parameters/InstrumentationFieldFetchParameters;)Ljava/lang/Object;
}

3 changes: 3 additions & 0 deletions sentry-graphql/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,11 @@ dependencies {
testImplementation(kotlin(Config.kotlinStdLib))
testImplementation(Config.TestLibs.kotlinTestJunit)
testImplementation(Config.TestLibs.mockitoKotlin)
testImplementation(Config.TestLibs.mockitoInline)
testImplementation(Config.TestLibs.mockWebserver)
testImplementation(Config.Libs.okhttp)
testImplementation(Config.Libs.springBootStarterGraphql)
testImplementation("com.netflix.graphql.dgs:graphql-error-types:4.9.2")
testImplementation(Config.Libs.graphQlJava)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
package io.sentry.graphql;

import graphql.ExecutionResult;
import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters;
import graphql.language.AstPrinter;
import graphql.schema.DataFetchingEnvironment;
import io.sentry.Hint;
import io.sentry.IHub;
import io.sentry.SentryEvent;
import io.sentry.SentryLevel;
import io.sentry.SentryOptions;
import io.sentry.exception.ExceptionMechanismException;
import io.sentry.protocol.Mechanism;
import io.sentry.protocol.Request;
import io.sentry.protocol.Response;
import java.util.HashMap;
import java.util.Map;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

@ApiStatus.Internal
public final class ExceptionReporter {
private final boolean captureRequestBodyForNonSubscriptions;

public ExceptionReporter(final boolean captureRequestBodyForNonSubscriptions) {
this.captureRequestBodyForNonSubscriptions = captureRequestBodyForNonSubscriptions;
}

private static final @NotNull String MECHANISM_TYPE = "GraphqlInstrumentation";

public void captureThrowable(
final @NotNull Throwable throwable,
final @NotNull ExceptionDetails exceptionDetails,
final @Nullable ExecutionResult result) {
final @NotNull IHub hub = exceptionDetails.getHub();
final @NotNull Mechanism mechanism = new Mechanism();
mechanism.setType(MECHANISM_TYPE);
mechanism.setHandled(false);
final @NotNull Throwable mechanismException =
new ExceptionMechanismException(mechanism, throwable, Thread.currentThread());
final @NotNull SentryEvent event = new SentryEvent(mechanismException);
event.setLevel(SentryLevel.FATAL);

final @NotNull Hint hint = new Hint();
setRequestDetailsOnEvent(hub, exceptionDetails, event);

if (result != null && isAllowedToAttachBody(hub)) {
final @NotNull Response response = new Response();
final @NotNull Map<String, Object> responseBody = result.toSpecification();
response.setData(responseBody);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you set all the information besides the data?

Copy link
Member Author

Choose a reason for hiding this comment

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

You mean statuscode, headers and cookies?

Copy link
Member Author

Choose a reason for hiding this comment

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

I can probably attach some of it for Spring in an event processor but since the response isn't finished yet it might very well be wrong / incomplete. Would you still like me to add it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add it after the request is made? This does not need to be GraphQL-specific since it makes sense for every integration.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can you add it after the request is made?

I don't understand. Do you mean after the response is finished? We'd have to delay sending an event to Sentry, probably put the response stuff on the scope and pick it up from there once ready. Sounds brittle and complicated.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, after the request is "finished".
I don't know the internals to tell how complicated is, but we do that with transactions and spans right, we collect all the information and "capture" at the end once everything is figured out, maybe we need something similar.
It's not a must but wishable.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can we do this in a follow up PR?

event.getContexts().setResponse(response);
}

hub.captureEvent(event, hint);
}

private boolean isAllowedToAttachBody(final @NotNull IHub hub) {
final @NotNull SentryOptions options = hub.getOptions();
return options.isSendDefaultPii()
&& !SentryOptions.RequestSize.NONE.equals(options.getMaxRequestBodySize());
}

private void setRequestDetailsOnEvent(
final @NotNull IHub hub,
final @NotNull ExceptionDetails exceptionDetails,
final @NotNull SentryEvent event) {
hub.configureScope(
(scope) -> {
final @Nullable Request scopeRequest = scope.getRequest();
final @NotNull Request request = scopeRequest == null ? new Request() : scopeRequest;
setDetailsOnRequest(hub, exceptionDetails, request);
event.setRequest(request);
});
}

private void setDetailsOnRequest(
final @NotNull IHub hub,
final @NotNull ExceptionDetails exceptionDetails,
final @NotNull Request request) {
request.setApiTarget("graphql");

if (isAllowedToAttachBody(hub)
&& (exceptionDetails.isSubscription() || captureRequestBodyForNonSubscriptions)) {
final @NotNull Map<String, Object> data = new HashMap<>();

data.put("query", exceptionDetails.getQuery());

final @Nullable Map<String, Object> variables = exceptionDetails.getVariables();
if (variables != null && !variables.isEmpty()) {
data.put("variables", variables);
}

// for Spring HTTP this will be replaced by RequestBodyExtractingEventProcessor
// for non subscription (websocket) errors
request.setData(data);
}
}

public static final class ExceptionDetails {

private final @NotNull IHub hub;
private final @Nullable InstrumentationExecutionParameters instrumentationExecutionParameters;
private final @Nullable DataFetchingEnvironment dataFetchingEnvironment;

private final boolean isSubscription;

public ExceptionDetails(
final @NotNull IHub hub,
final @Nullable InstrumentationExecutionParameters instrumentationExecutionParameters,
final boolean isSubscription) {
this.hub = hub;
this.instrumentationExecutionParameters = instrumentationExecutionParameters;
dataFetchingEnvironment = null;
this.isSubscription = isSubscription;
}

public ExceptionDetails(
final @NotNull IHub hub,
final @Nullable DataFetchingEnvironment dataFetchingEnvironment,
final boolean isSubscription) {
this.hub = hub;
this.dataFetchingEnvironment = dataFetchingEnvironment;
instrumentationExecutionParameters = null;
this.isSubscription = isSubscription;
}

public @Nullable String getQuery() {
if (instrumentationExecutionParameters != null) {
return instrumentationExecutionParameters.getQuery();
}
if (dataFetchingEnvironment != null) {
return AstPrinter.printAst(dataFetchingEnvironment.getDocument());
}
return null;
}

public @Nullable Map<String, Object> getVariables() {
if (instrumentationExecutionParameters != null) {
return instrumentationExecutionParameters.getVariables();
}
if (dataFetchingEnvironment != null) {
return dataFetchingEnvironment.getVariables();
}
return null;
}

public boolean isSubscription() {
return isSubscription;
}

public @NotNull IHub getHub() {
return hub;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package io.sentry.graphql;

import graphql.execution.MergedField;
import graphql.schema.GraphQLNamedOutputType;
import graphql.schema.GraphQLObjectType;
import graphql.schema.GraphQLOutputType;
import io.sentry.util.StringUtils;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

@ApiStatus.Internal
public final class GraphqlStringUtils {

public static @Nullable String fieldToString(final @Nullable MergedField field) {
if (field == null) {
return null;
}

return field.getName();
}

public static @Nullable String typeToString(final @Nullable GraphQLOutputType type) {
if (type == null) {
return null;
}

if (type instanceof GraphQLNamedOutputType) {
final @NotNull GraphQLNamedOutputType namedType = (GraphQLNamedOutputType) type;
return namedType.getName();
}

return StringUtils.toString(type);
}

public static @Nullable String objectTypeToString(final @Nullable GraphQLObjectType type) {
if (type == null) {
return null;
}

return type.getName();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package io.sentry.graphql;

import graphql.execution.instrumentation.parameters.InstrumentationFieldFetchParameters;
import io.sentry.IHub;
import org.jetbrains.annotations.NotNull;

public final class NoOpSubscriptionHandler implements SentrySubscriptionHandler {

private static final @NotNull NoOpSubscriptionHandler instance = new NoOpSubscriptionHandler();

private NoOpSubscriptionHandler() {}

public static @NotNull NoOpSubscriptionHandler getInstance() {
return instance;
}

@Override
public @NotNull Object onSubscriptionResult(
@NotNull Object result,
@NotNull IHub hub,
@NotNull ExceptionReporter exceptionReporter,
@NotNull InstrumentationFieldFetchParameters parameters) {
return result;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,18 @@
import io.sentry.Hint;
import io.sentry.HubAdapter;
import io.sentry.IHub;
import io.sentry.SentryIntegrationPackageStorage;
import io.sentry.util.Objects;
import org.jetbrains.annotations.NotNull;

/**
* Captures exceptions that occur during data fetching, passes them to Sentry and invokes a delegate
* exception handler.
*
* @deprecated please use {@link SentryGenericDataFetcherExceptionHandler} in combination with
* {@link SentryInstrumentation} instead for better error reporting.
*/
@Deprecated
public final class SentryDataFetcherExceptionHandler implements DataFetcherExceptionHandler {
private final @NotNull IHub hub;
private final @NotNull DataFetcherExceptionHandler delegate;
Expand All @@ -23,6 +28,7 @@ public SentryDataFetcherExceptionHandler(
final @NotNull IHub hub, final @NotNull DataFetcherExceptionHandler delegate) {
this.hub = Objects.requireNonNull(hub, "hub is required");
this.delegate = Objects.requireNonNull(delegate, "delegate is required");
SentryIntegrationPackageStorage.getInstance().addIntegration("GrahQLLegacyExceptionHandler");
}

public SentryDataFetcherExceptionHandler(final @NotNull DataFetcherExceptionHandler delegate) {
Expand Down
Loading