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

Adapt span op and description for graphql #2607

Merged
merged 4 commits into from
Mar 16, 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
- The Spring Boot integration can now be configured to add the `SentryAppender` to specific loggers instead of the `ROOT` logger ([#2173](https://github.com/getsentry/sentry-java/pull/2173))
- You can specify the loggers using `"sentry.logging.loggers[0]=foo.bar` and `"sentry.logging.loggers[1]=baz` in your `application.properties`
- Add capabilities to track Jetpack Compose composition/rendering time ([#2507](https://github.com/getsentry/sentry-java/pull/2507))
- Adapt span op and description for graphql to fit spec ([#2607](https://github.com/getsentry/sentry-java/pull/2607))

### Fixes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,11 @@ class SentryApollo3HttpInterceptor @JvmOverloads constructor(private val hub: IH
val method = request.method

val operationName = operationNameFromHeaders(request)
val operation = operationName ?: "apollo.client"
val operationType = request.valueForHeader(SENTRY_APOLLO_3_OPERATION_TYPE) ?: method
val operationType = request.valueForHeader(SENTRY_APOLLO_3_OPERATION_TYPE)
val operation = if (operationType != null) "http.graphql.$operationType" else "http.graphql"
val operationId = request.valueForHeader("X-APOLLO-OPERATION-ID")
val variables = request.valueForHeader(SENTRY_APOLLO_3_VARIABLES)
val description = "$operationType ${operationName ?: urlDetails.urlOrFallback}"
val description = "${operationType ?: method} ${operationName ?: urlDetails.urlOrFallback}"

return activeSpan.startChild(operation, description).apply {
urlDetails.applyToSpan(this)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ class SentryApollo3InterceptorTest {
private fun assertTransactionDetails(it: SentryTransaction) {
assertEquals(1, it.spans.size)
val httpClientSpan = it.spans.first()
assertEquals("LaunchDetails", httpClientSpan.op)
assertEquals("http.graphql", httpClientSpan.op)
assertTrue { httpClientSpan.description?.startsWith("Post LaunchDetails") == true }
assertNotNull(httpClientSpan.data) {
assertNotNull(it["operationId"])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ class SentryApollo3InterceptorWithVariablesTest {
private fun assertTransactionDetails(it: SentryTransaction) {
assertEquals(1, it.spans.size)
val httpClientSpan = it.spans.first()
assertEquals("LaunchDetails", httpClientSpan.op)
assertEquals("http.graphql.query", httpClientSpan.op)
assertEquals("query LaunchDetails", httpClientSpan.description)
assertNotNull(httpClientSpan.data) {
assertNotNull(it["operationId"])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,9 @@ class SentryApolloInterceptor(
is Subscription -> "subscription"
else -> request.operation.javaClass.simpleName
}
val op = "http.graphql.$operationType"
val description = "$operationType $operation"
return activeSpan.startChild(operation, description)
return activeSpan.startChild(op, description)
}

private fun finish(span: ISpan, request: InterceptorRequest, response: InterceptorResponse? = null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ class SentryApolloInterceptorTest {
private fun assertTransactionDetails(it: SentryTransaction) {
assertEquals(1, it.spans.size)
val httpClientSpan = it.spans.first()
assertEquals("LaunchDetails", httpClientSpan.op)
assertEquals("http.graphql.query", httpClientSpan.op)
assertEquals("query LaunchDetails", httpClientSpan.description)
assertNotNull(httpClientSpan.data) {
assertNotNull(it["operationId"])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public SentryInstrumentation() {
final TracingState tracingState = parameters.getInstrumentationState();
final ISpan transaction = tracingState.getTransaction();
if (transaction != null) {
final ISpan span = transaction.startChild(findDataFetcherTag(parameters));
final ISpan span = createSpan(transaction, parameters);
try {
final Object result = dataFetcher.get(environment);
if (result instanceof CompletableFuture) {
Expand Down Expand Up @@ -127,8 +127,8 @@ private void finish(
finish(span, environment, null);
}

private @NotNull String findDataFetcherTag(
final @NotNull InstrumentationFieldFetchParameters parameters) {
private @NotNull ISpan createSpan(
@NotNull ISpan transaction, @NotNull InstrumentationFieldFetchParameters parameters) {
final GraphQLOutputType type = parameters.getExecutionStepInfo().getParent().getType();
GraphQLObjectType parent;
if (type instanceof GraphQLNonNull) {
Expand All @@ -137,7 +137,9 @@ private void finish(
parent = (GraphQLObjectType) type;
}

return parent.getName() + "." + parameters.getExecutionStepInfo().getPath().getSegmentName();
return transaction.startChild(
"graphql",
parent.getName() + "." + parameters.getExecutionStepInfo().getPath().getSegmentName());
}

static final class TracingState implements InstrumentationState {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ class SentryInstrumentationTest {
assertTrue(result.errors.isEmpty())
assertEquals(1, fixture.activeSpan.children.size)
val span = fixture.activeSpan.children.first()
assertEquals("Query.shows", span.operation)
assertEquals("graphql", span.operation)
assertEquals("Query.shows", span.description)
assertTrue(span.isFinished)
assertEquals(SpanStatus.OK, span.status)
}
Expand All @@ -89,7 +90,8 @@ class SentryInstrumentationTest {
assertTrue(result.errors.isNotEmpty())
assertEquals(1, fixture.activeSpan.children.size)
val span = fixture.activeSpan.children.first()
assertEquals("Query.shows", span.operation)
assertEquals("graphql", span.operation)
assertEquals("Query.shows", span.description)
assertTrue(span.isFinished)
assertEquals(SpanStatus.INTERNAL_ERROR, span.status)
}
Expand All @@ -113,7 +115,8 @@ class SentryInstrumentationTest {
assertTrue(result.errors.isEmpty())
assertEquals(1, fixture.activeSpan.children.size)
val span = fixture.activeSpan.children.first()
assertEquals("Query.shows", span.operation)
assertEquals("graphql", span.operation)
assertEquals("Query.shows", span.description)
assertNotNull(span.isSampled) {
assertFalse(it)
}
Expand All @@ -128,7 +131,7 @@ class SentryInstrumentationTest {
assertTrue(result.errors.isEmpty())
assertEquals(1, fixture.activeSpan.children.size)
val span = fixture.activeSpan.children.first()
assertEquals("Query.shows", span.operation)
assertEquals("graphql", span.operation)
assertEquals("changed", span.description)
assertTrue(span.isFinished)
}
Expand Down