Add Sentry tracing

[patch0]

Closes #3655

Adds support for sending traces to Sentry. The data names follow the OpenTelemetry Semantic Conventions, which is how Sentry say data should be sent.

Points for consideration

I've aliased execute_multiplex / analyze_multiplex to graphql.execute_multiplex / graphql.analyze_multiplex operations, rather than graphql.execute / graphql.analyze which is what other tracers do. This is because the initial entry point for queries seems to be execute_multiplex, followed by an execute_query call later. If there is just one operation in the execute_multiplex span, we'd end up with two spans with identical names in the same transaction, which could be confusing. The description could be postponed until the execute_query is called, but I felt it was useful to have an overall description set at the top of the span.

When an operation name is not set, I've not used anonymous as the identifier which is what the base class does, but followed the OpenTelemetry spec which states that the name should be omitted.

If the operation name and type are both missing, this defaults to GraphQL Operation, which is what has been suggested by the OpenTelemetry convention.

No attempt is made to sanitize the GraphQL document for any sensitive data.

I've not snake-cased the data names, although Sentry suggests this is preferred.

Questions ❓

In testing I've found that the tests for GraphQL::Tracing::SentryTracing (the legacy tracing) use GraphQL::Tracing::SentryTrace unless I remove require 'graphql/tracing/sentry_trace' form graphql/tracing.rb. Is there a better way to ensure that the SentryTracing spec actually tests SentryTracing?

[rmosolgo]

Hi, thanks for working this up! I'll take a look soon.

[patch0]

Hi, thanks for working this up! I'll take a look soon.

Thanks @rmosolgo :) @sl0thentr0py, one of the engineers from Sentry is also interested in it too.

[rmosolgo]

Thanks again for your work on this! I took a look and:

• Removed the legacy-style SentryTracing class. Since we don't have to support backwards-compatibility, let's just add the new one.
• Updated the tracing docs to use the new-style *Trace modules

Sorry about the trouble with testing. In fact, LegacyTracing classes are made to use the new-style modules instead, unless a specific opt-in option is given:

graphql-ruby/lib/graphql/tracing/platform_tracing.rb

Lines 75 to 91 in 0f32887

	def self.use(schema_defn, options = {})
	if options[:legacy_tracing]
	tracer = self.new(**options)
	schema_defn.tracer(tracer)
	else
	tracing_name = self.name.split("::").last
	trace_name = tracing_name.sub("Tracing", "Trace")
	if GraphQL::Tracing.const_defined?(trace_name, false)
	trace_module = GraphQL::Tracing.const_get(trace_name)
	schema_defn.trace_with(trace_module, **options)
	else
	tracer = self.new(**options)
	schema_defn.tracer(tracer)
	end
	end
	end

(I added that option after implementing the under-the-hood migration, just in case people ran into trouble. But I never documented it, and I never heard any reports of issues with the new modules. It's time to deprecate (and eventually remove) those old ones!)

[sl0thentr0py]

late to the party but the sentry data / span looks good to me from our side! thank you for adding this :)

[sl0thentr0py]

alright so we like to make things as zero-configurable as possible, so what I'm going to do on our side now is

• detect this gem
• add an (opt-out) patch to automatically enable this tracing on each graphql schema class

let me know if you see a problem with that! I think this is generally useful for all our users.

[rmosolgo]

👍 Certainly fine with me. It should work to add it to all classes by adding to GraphQL::Schema, then all classes would inherit it, for example:

# add tracing to all GraphQL schemas in the app: 
GraphQL::Schema.trace_with(GraphQL::Tracing::SentryTrace)

The only problem is ... there's no way to remove it after it's added there. (Under the hood, trace_with does an include(...) into a Ruby class -- I don't know any way of un-including a module!)

Another approach might be to add it on the inherited hook, for example:

# Monkey-patch this to include tracing on all child schemas, unless it's been disabled: 
class GraphQL::Schema 
  def self.included(child_class)
    if Sentry.trace_graphql_by_default? # somehow implement your opt-out configuration 
      child_class.trace_with(GraphQL::Tracing::SentryTrace)
    end 
  end 
end 

Let me know if I can help with anything while you're looking for a solution!

[sl0thentr0py]

yep something like that, thanks for the samples!

We will just have a top-level switch to turn off the whole patch and then people can of course add it manually to the ones they prefer. We just prefer to make it easier for the 99% use case to work with minimal configuration.