Align span `op` spec

[marandaneto]

Description

getsentry/develop#693

SDK	Old Operation	New Operation	Link	PR
Java
	< graphql op name >	http.graphql.< op-name >	sentry-java/sentry-apollo/src/main/java/io/sentry/apollo/SentryApolloInterceptor.kt Line 88 in e2ea31c val operation = request.operation.name().name()
	< objectType >.< segment >	???	sentry-java/sentry-graphql/src/main/java/io/sentry/graphql/SentryInstrumentation.java Line 136 in e2ea31c return parent.getName() + "." + parameters.getExecutionStepInfo().getPath().getSegmentName();

[AbhiPrasad]

So this is still undecided on what these should change to, so feedback here is welcome. We can always also amend the develop docs based on this.

[marandaneto]

So this is still undecided on what these should change to, so feedback here is welcome. We can always also amend the develop docs based on this.

Yep, @romtsn said he's gonna take a look at it.

[romtsn]

@AbhiPrasad so here's the summary:

SentrySpanAdvice

Pretty straightforward, because we can treat this essentially as a manual instrumentation/span. Users have to manually add a @SentrySpan annotation to the methods they want to create spans for, they can also (optionally) provide a value which will be used as a span op. If they don't, then the instrumentation defaults to the owner className + methodName.

I don't think we need to have a strict span op for that, since again, it's manual instrumentation.

SentryInstrumentation

The first part will be set to the type that contains the queried fields. For root operations it'd be one of [query|mutation|subscription], otherwise it'd be the parent type; the second part is set to the field being queried.

For example, for this query:

query ShowsAndActors {
    shows {
      actors {
        name
      }
    }
}

There will be 2 spans: query.shows and show.actors.

I think what you suggested should work here, so either just graphql or maybe graphql.server as op name, and description would contain the type + queried fields then.

SentryApolloInterceptor/SentryApollo3Interceptor

It will either set the op to the operation name or apollo.client, e.g. this query:

query LaunchDetails($id:ID!) {
  launch(id: $id) {
    id
    site
    mission {
      name
      missionPatch(size:LARGE)
    }
  }
}

will set it to LaunchDetails.

http.graphql.[query|mutation|subscription] should be fine here I guess. Or it could also be graphql.client.[query|mutation|subscription]. The description already contains the operation type + name anyway.

cc @adinauer

[marandaneto]

@romtsn

I don't think we need to have a strict span op for that, since again, it's manual instrumentation.

If we want to leverage our performance issues feature for AOP instrumentation, we'd need a defined op, the details can go under the description.

Do you have any suggestions here?

[romtsn]

@romtsn

I don't think we need to have a strict span op for that, since again, it's manual instrumentation.

If we want to leverage our performance issues feature for AOP instrumentation, we'd need a defined op, the details can go under the description.

Do you have any suggestions here?

I was not aware we can detect slow custom spans. Could you provide more details on how we would do that? Will we allow users to set their custom thresholds for slow spans?

[marandaneto]

@romtsn This is new and I don't have more context about that, but this feature requires a pre-defined op and that's all we need for now afaik.

[romtsn]

Sorry, I don't understand how can we have a pre-defined op for custom spans, therefore I don't have any suggestions for this specific case.

[AbhiPrasad]

Users have to manually add a @SentrySpan annotation to the methods they want to create spans

If these are methods, could we default to applying the function operation name? They are using the annotation the measure the time of a function call right? Users can overwrite this if they want. This is in-line with some of the other usages of function that we have in other SDKs.

then the instrumentation defaults to the owner className + methodName

Can this be the span description instead? The operation than will stay consistent, while the description can have a higher range of unique values.

[romtsn]

If these are methods, could we default to applying the function operation name? They are using the annotation the measure the time of a function call right? Users can overwrite this if they want. This is in-line with some of the other usages of function that we have in other SDKs.

sounds good 👍

Can this be the span description instead? The operation than will stay consistent, while the description can have a higher range of unique values.

Yes, we can default to this for the description. It can be still overwritten by users though, but I think it's alright.

As for the other 2, can we agree on http.graphql.[query|mutation|subscription] for clients and graphql for servers?

[AbhiPrasad]

As for the other 2, can we agree on http.graphql.[query|mutation|subscription] for clients and graphql for servers?

Sounds good to me!