-
Notifications
You must be signed in to change notification settings - Fork 3k
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
feat(monitoring): track graphql errors in metrics #6087
feat(monitoring): track graphql errors in metrics #6087
Conversation
@@ -126,9 +126,20 @@ void getGraphQL(HttpServletRequest request, HttpServletResponse response) { | |||
throw new UnsupportedOperationException("GraphQL gets not supported."); | |||
} | |||
|
|||
private void observeErrors(ExecutionResult executionResult) { | |||
executionResult.getErrors().forEach(graphQLError -> { | |||
String errorType = graphQLError.getErrorType().toString(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we also have our own exception type called DataHubGraphQLError that has an "error code" field. This would be useful to extract as it can show error codes like 401, 404, 500 etc.
If the first error is not an instance of DataHubGraphQLError, then we can increment he "500" error code bucket
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm seeing metrics like this if I run it locally:
metrics_com_datahub_graphql_GraphQLController_errorType_DataFetchingException_Count
DataFetchingException
is the result of DataHubGraphQLError.getErrorType()
: here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that what you meant?
…nto measure-graphql-errors
metadata-service/graphql-servlet-impl/src/main/java/com/datahub/graphql/GraphQLController.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks good to me. Hoping you have tested this doesn't affect anything locally
Actually - looks like compilation is failing. Please address first! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix checkstyle and then we can get this in :)
Checklist