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.

Sign up for GitHub

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

Jump to bottom

Fix GraphQL integration reconfiguration #3859

Merged
merged 1 commit into from
Sep 17, 2024
Merged

Fix GraphQL integration reconfiguration #3859

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 22 additions & 1 deletion Rakefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ desc 'Run RSpec'
# rubocop:disable Metrics/BlockLength
namespace :spec do
task all: [:main, :benchmark,
:graphql, :graphql_unified_trace_patcher, :graphql_trace_patcher, :graphql_tracing_patcher,
:rails, :railsredis, :railsredis_activesupport, :railsactivejob,
:elasticsearch, :http, :redis, :sidekiq, :sinatra, :hanami, :hanami_autoinstrument,
:profiling, :crashtracking]
Expand All @@ -101,6 +102,27 @@ namespace :spec do
t.rspec_opts = args.to_a.join(' ')
end

RSpec::Core::RakeTask.new(:graphql) do |t, args|
t.pattern = 'spec/datadog/tracing/contrib/graphql/**/*_spec.rb'
t.exclude_pattern = 'spec/datadog/tracing/contrib/graphql/{unified_trace,trace,tracing}_patcher_spec.rb'
t.rspec_opts = args.to_a.join(' ')
end

RSpec::Core::RakeTask.new(:graphql_unified_trace_patcher) do |t, args|
t.pattern = 'spec/datadog/tracing/contrib/graphql/unified_trace_patcher_spec.rb'
t.rspec_opts = args.to_a.join(' ')
end

RSpec::Core::RakeTask.new(:graphql_trace_patcher) do |t, args|
t.pattern = 'spec/datadog/tracing/contrib/graphql/trace_patcher_spec.rb'
t.rspec_opts = args.to_a.join(' ')
end

RSpec::Core::RakeTask.new(:graphql_tracing_patcher) do |t, args|
t.pattern = 'spec/datadog/tracing/contrib/graphql/tracing_patcher_spec.rb'
t.rspec_opts = args.to_a.join(' ')
end

desc '' # "Explicitly hiding from `rake -T`"
RSpec::Core::RakeTask.new(:opentelemetry) do |t, args|
t.pattern = 'spec/datadog/opentelemetry/**/*_spec.rb,spec/datadog/opentelemetry_spec.rb'
Expand Down Expand Up @@ -223,7 +245,6 @@ namespace :spec do
:excon,
:faraday,
:grape,
:graphql,
:grpc,
:http,
:httpclient,
Expand Down
33 changes: 22 additions & 11 deletions docs/GettingStarted.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -845,8 +845,14 @@ To activate your integration, use the `Datadog.configure` method:

```ruby
# Inside Rails initializer or equivalent
# For graphql >= v2.2
Datadog.configure do |c|
c.tracing.instrument :graphql, schemas: [YourSchema], **options
c.tracing.instrument :graphql, with_unified_tracer: true, **options
end

# For graphql < v2.2
Datadog.configure do |c|
c.tracing.instrument :graphql, **options
end

# Then run a GraphQL query
Expand All @@ -859,31 +865,38 @@ The `instrument :graphql` method accepts the following parameters. Additional op
| ------------------------ | -------------------------- | -------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------- |
| `enabled` | `DD_TRACE_GRAPHQL_ENABLED` | `Bool` | Whether the integration should create spans. | `true` |
| `schemas` | | `Array` | Array of `GraphQL::Schema` objects (that support class-based schema only) to trace. If you do not provide any, then tracing will applied to all the schemas. | `[]` |
| `with_unified_tracer` | | `Bool` | Enable to instrument with `UnifiedTrace` tracer, enabling support for API Catalog. `with_deprecated_tracer` has priority over this. Default is `false`, using `GraphQL::Tracing::DataDogTrace` (Added in v2.2) | `false` |
| `with_deprecated_tracer` | | `Bool` | Enable to instrument with deprecated `GraphQL::Tracing::DataDogTracing`. This has priority over `with_unified_tracer`. Default is `false`, using `GraphQL::Tracing::DataDogTrace` | `false` |
| `with_unified_tracer` | | `Bool` | (Recommended) Enable to instrument with `UnifiedTrace` tracer for `graphql` >= v2.2, **enabling support for API Catalog**. `with_deprecated_tracer` has priority over this. Default is `false`, using `GraphQL::Tracing::DataDogTrace` instead | `false` |
| `with_deprecated_tracer` | | `Bool` | Enable to instrument with deprecated `GraphQL::Tracing::DataDogTracing`. This has priority over `with_unified_tracer`. Default is `false`, using `GraphQL::Tracing::DataDogTrace` instead | `false` |
| `service_name` | | `String` | Service name used for graphql instrumentation | `'ruby-graphql'` |

Once an instrumentation strategy is selected (`with_unified_tracer: true`, `with_deprecated_tracer: true`, or *no option set* which defaults to `GraphQL::Tracing::DataDogTrace`), it is not possible to change the instrumentation strategy in the same Ruby process.
This is especially important for [auto instrumented applications](#rails-or-hanami-applications) because an automatic initial instrumentation is always applied at startup, thus such applications will always instrument GraphQL with the default strategy (`GraphQL::Tracing::DataDogTrace`).

**Manually configuring GraphQL schemas**

If you prefer to individually configure the tracer settings for a schema (e.g. you have multiple schemas), in the schema definition, you can add the following [using the GraphQL API](http://graphql-ruby.org/queries/tracing.html):
If you prefer, you can individually configure the tracer settings per schema (e.g. you have multiple schemas with distinct instrumentation options).

Do _NOT_ `c.tracing.instrument :graphql` in `Datadog.configure` if you choose to configure schema settings manually, as to avoid double tracing. These two means of configuring GraphQL tracing are mutually exclusive.

To instrument each schema individually, you add the following [using the GraphQL API](http://graphql-ruby.org/queries/tracing.html):

With `GraphQL::Tracing::DataDogTrace`
For `graphql` >= v2.2:

```ruby
class YourSchema < GraphQL::Schema
trace_with GraphQL::Tracing::DataDogTrace
trace_with Datadog::Tracing::Contrib::GraphQL::UnifiedTrace
end
```

With `UnifiedTracer` (Added in v2.2)
For `graphql` < v2.2:

```ruby
class YourSchema < GraphQL::Schema
trace_with Datadog::Tracing::Contrib::GraphQL::UnifiedTrace
trace_with GraphQL::Tracing::DataDogTrace
end
```

or with `GraphQL::Tracing::DataDogTracing` (deprecated)
Using the deprecated tracer GraphQL (`GraphQL::Tracing::DataDogTracing`):

```ruby
class YourSchema < GraphQL::Schema
Expand All @@ -893,8 +906,6 @@ end

**Note**: This integration does not support define-style schemas. Only class-based schemas are supported.

Do _NOT_ `instrument :graphql` in `Datadog.configure` if you choose to configure manually, as to avoid double tracing. These two means of configuring GraphQL tracing are considered mutually exclusive.

**Adding custom tags to Datadog spans**

You can add custom tags to Datadog spans by implementing the `prepare_span` method in a subclass, then manually configuring your schema.
Expand Down
21 changes: 9 additions & 12 deletions lib/datadog/tracing/contrib/graphql/patcher.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,32 +21,29 @@ def target_version
end

def patch
# DEV-3.0: We should remove as many patching options as possible, given the alternatives do not
# DEV-3.0: provide any benefit to the recommended `with_unified_tracer` patching method.
# DEV-3.0: `with_deprecated_tracer` is likely safe to remove.
# DEV-3.0: `with_unified_tracer: false` should be removed if possible.
# DEV-3.0: `with_unified_tracer: true` should be the default and hopefully not even necessary as an option.
if configuration[:with_deprecated_tracer]
TracingPatcher.patch!(schemas, trace_options)
TracingPatcher.patch!(schemas)
elsif Integration.trace_supported?
if configuration[:with_unified_tracer]
UnifiedTracePatcher.patch!(schemas, trace_options)
UnifiedTracePatcher.patch!(schemas)
else
TracePatcher.patch!(schemas, trace_options)
TracePatcher.patch!(schemas)
end
else
Datadog.logger.warn(
"GraphQL version (#{target_version}) does not support GraphQL::Tracing::DataDogTrace"\
'or Datadog::Tracing::Contrib::GraphQL::UnifiedTrace.'\
'Falling back to GraphQL::Tracing::DataDogTracing.'
)
TracingPatcher.patch!(schemas, trace_options)
TracingPatcher.patch!(schemas)
end
end

def trace_options
{
service: configuration[:service_name],
analytics_enabled: Contrib::Analytics.enabled?(configuration[:analytics_enabled]),
analytics_sample_rate: configuration[:analytics_sample_rate]
}
end

def configuration
Datadog.configuration.tracing[:graphql]
end
Expand Down
6 changes: 3 additions & 3 deletions lib/datadog/tracing/contrib/graphql/trace_patcher.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ module GraphQL
module TracePatcher
module_function

def patch!(schemas, options)
def patch!(schemas)
if schemas.empty?
::GraphQL::Schema.trace_with(::GraphQL::Tracing::DataDogTrace, **options)
::GraphQL::Schema.trace_with(::GraphQL::Tracing::DataDogTrace)
else
schemas.each do |schema|
schema.trace_with(::GraphQL::Tracing::DataDogTrace, **options)
schema.trace_with(::GraphQL::Tracing::DataDogTrace)
end
end
end
Expand Down
6 changes: 3 additions & 3 deletions lib/datadog/tracing/contrib/graphql/tracing_patcher.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ module GraphQL
module TracingPatcher
module_function

def patch!(schemas, options)
def patch!(schemas)
if schemas.empty?
::GraphQL::Schema.tracer(::GraphQL::Tracing::DataDogTracing.new(**options))
::GraphQL::Schema.tracer(::GraphQL::Tracing::DataDogTracing.new)
else
schemas.each do |schema|
if schema.respond_to? :use
schema.use(::GraphQL::Tracing::DataDogTracing, **options)
schema.use(::GraphQL::Tracing::DataDogTracing)
else
Datadog.logger.warn("Unable to patch #{schema}: Please migrate to class-based schema.")
end
Expand Down
22 changes: 13 additions & 9 deletions lib/datadog/tracing/contrib/graphql/unified_trace.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,7 @@ module GraphQL
# which is required to use features such as API Catalog.
# DEV-3.0: This tracer should be the default one in the next major version.
module UnifiedTrace
# @param analytics_enabled [Boolean] Deprecated
# @param analytics_sample_rate [Float] Deprecated
# @param service [String|nil] The service name to be set on the spans
def initialize(*args, analytics_enabled: false, analytics_sample_rate: 1.0, service: nil, **kwargs)
@analytics_enabled = analytics_enabled
@analytics_sample_rate = analytics_sample_rate

@service_name = service
def initialize(*args, **kwargs)
Copy link
Contributor

Choose a reason for hiding this comment

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

⚪ Code Quality Violation

Optional arguments should appear at the end (...read more)

The rule "Optional arguments should appear at the end" is an important programming practice in Ruby. It is used to ensure that the optional parameters in a method definition are placed after the required parameters. This rule is important because when a method is called, Ruby fills in the arguments from left to right. If an optional argument is placed before a required argument and the method is called with fewer arguments, Ruby will assign the provided arguments to the optional parameters, leaving the required parameters without values and causing an error.

Non-compliance with this rule often leads to unexpected behavior or bugs in the code, which can be quite challenging to debug. This is particularly true when the method is called with fewer arguments than defined. The errors caused by this can be hard to track down, as they may not occur at the place where the method is defined, but rather in some distant place where the method is called.

To avoid this, always place optional parameters at the end of the list of parameters in your method definitions. This way, Ruby will fill in the required parameters first, and only then use any remaining arguments for the optional ones. If there are no remaining arguments, the optional parameters will be set to their default values. This keeps your code clear, predictable, and free of unnecessary bugs.

View in Datadog  Leave us feedback  Documentation

Copy link
Contributor

Choose a reason for hiding this comment

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

⚪ Code Quality Violation

Optional arguments should appear at the end (...read more)

The rule "Optional arguments should appear at the end" is an important programming practice in Ruby. It is used to ensure that the optional parameters in a method definition are placed after the required parameters. This rule is important because when a method is called, Ruby fills in the arguments from left to right. If an optional argument is placed before a required argument and the method is called with fewer arguments, Ruby will assign the provided arguments to the optional parameters, leaving the required parameters without values and causing an error.

Non-compliance with this rule often leads to unexpected behavior or bugs in the code, which can be quite challenging to debug. This is particularly true when the method is called with fewer arguments than defined. The errors caused by this can be hard to track down, as they may not occur at the place where the method is defined, but rather in some distant place where the method is called.

To avoid this, always place optional parameters at the end of the list of parameters in your method definitions. This way, Ruby will fill in the required parameters first, and only then use any remaining arguments for the optional ones. If there are no remaining arguments, the optional parameters will be set to their default values. This keeps your code clear, predictable, and free of unnecessary bugs.

View in Datadog  Leave us feedback  Documentation

@has_prepare_span = respond_to?(:prepare_span)
super
end
Expand Down Expand Up @@ -139,7 +132,18 @@ def platform_resolve_type_key(type, *args, **kwargs)
private

def trace(callable, trace_key, resource, **kwargs)
Tracing.trace("graphql.#{trace_key}", resource: resource, service: @service_name, type: 'graphql') do |span|
config = Datadog.configuration.tracing[:graphql]

Tracing.trace(
"graphql.#{trace_key}",
type: 'graphql',
resource: resource,
service: config[:service_name]
) do |span|
if Contrib::Analytics.enabled?(config[:analytics_enabled])
Contrib::Analytics.set_sample_rate(span, config[:analytics_sample_rate])
end

yield(span) if block_given?

prepare_span(trace_key, kwargs, span) if @has_prepare_span
Expand Down
9 changes: 6 additions & 3 deletions lib/datadog/tracing/contrib/graphql/unified_trace_patcher.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,15 @@ module GraphQL
module UnifiedTracePatcher
module_function

def patch!(schemas, options)
# TODO: `GraphQL::Schema.trace_with` and `YOUR_SCHEMA.trace_with` don't mix.
# TODO: They create duplicate spans when combined.
# TODO: We should measure how frequently users use `YOUR_SCHEMA.trace_with`, and hopefully we can remove it.
def patch!(schemas)
if schemas.empty?
::GraphQL::Schema.trace_with(UnifiedTrace, **options)
::GraphQL::Schema.trace_with(UnifiedTrace)
else
schemas.each do |schema|
schema.trace_with(UnifiedTrace, **options)
schema.trace_with(UnifiedTrace)
end
end
end
Expand Down
Loading
Loading