comment_propagation: 'full' throws an error when DD_TRACE_ENABLED is false

[nightpool]

Current behaviour

All DB methods throw an error when DD_TRACE_ENABLED is set to false

/app/vendor/bundle/ruby/2.6.0/bundler/gems/rails2-aae3e4374b07/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:227:in `rescue in log': TypeError: can't convert nil into Integer: SET standard_conforming_strings = on (ActiveRecord::StatementInvalid)
        from /app/vendor/bundle/ruby/2.6.0/bundler/gems/rails2-aae3e4374b07/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:203:in `log'
        from /app/vendor/bundle/ruby/2.6.0/bundler/gems/rails2-aae3e4374b07/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb:520:in `execute'
        from /app/vendor/bundle/ruby/2.6.0/bundler/gems/rails2-aae3e4374b07/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb:269:in `set_standard_conforming_strings'
        from /app/vendor/bundle/ruby/2.6.0/bundler/gems/rails2-aae3e4374b07/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb:975:in `configure_connection'
... [snip] ...
/app/vendor/bundle/ruby/2.6.0/gems/ddtrace-1.11.1/lib/datadog/tracing/distributed/trace_context.rb:116:in `format': can't convert nil into Integer (TypeError)
        from /app/vendor/bundle/ruby/2.6.0/gems/ddtrace-1.11.1/lib/datadog/tracing/distributed/trace_context.rb:116:in `build_traceparent_string'
        from /app/vendor/bundle/ruby/2.6.0/gems/ddtrace-1.11.1/lib/datadog/tracing/distributed/trace_context.rb:93:in `build_traceparent'
        from /app/vendor/bundle/ruby/2.6.0/gems/ddtrace-1.11.1/lib/datadog/tracing/contrib/propagation/sql_comment.rb:32:in `prepend_comment'
        from /app/vendor/bundle/ruby/2.6.0/gems/ddtrace-1.11.1/lib/datadog/tracing/contrib/pg/instrumentation.rb:99:in `block in trace'
        from /app/vendor/bundle/ruby/2.6.0/gems/ddtrace-1.11.1/lib/datadog/tracing/tracer.rb:524:in `skip_trace'
        from /app/vendor/bundle/ruby/2.6.0/gems/ddtrace-1.11.1/lib/datadog/tracing/tracer.rb:137:in `trace'
        from /app/vendor/bundle/ruby/2.6.0/gems/ddtrace-1.11.1/lib/datadog/tracing.rb:18:in `trace'
        from /app/vendor/bundle/ruby/2.6.0/gems/ddtrace-1.11.1/lib/datadog/tracing/contrib/pg/instrumentation.rb:86:in `trace'
        from /app/vendor/bundle/ruby/2.6.0/gems/ddtrace-1.11.1/lib/datadog/tracing/contrib/pg/instrumentation.rb:24:in `exec'
        from /app/vendor/bundle/ruby/2.6.0/bundler/gems/rails2-aae3e4374b07/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb:524:in `block in execute'
        from /app/vendor/bundle/ruby/2.6.0/bundler/gems/rails2-aae3e4374b07/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:206:in `block in log'
... [snip] ...

Expected behaviour

build_traceparent_string correctly handles the case where trace_id and span_id might be nil.

This seems pretty straightforward. We call the build_traceparent private method directly from SqlComment:

https://github.com/DataDog/dd-trace-rb/blob/1dc95e27d927978e547c2e12b38cb7e3aff3b92e/lib/datadog/tracing/contrib/propagation/sql_comment.rb#L32

But in the case where tracing is disabled, trace_op is set to dummy nil values by skip_trace:

https://github.com/DataDog/dd-trace-rb/blob/1dc95e27d927978e547c2e12b38cb7e3aff3b92e/lib/datadog/tracing/tracer.rb#L519-L528

which bypasses build_span where we set parent_trace_id to 0:

https://github.com/DataDog/dd-trace-rb/blob/28c3c3e1b19a625efe7ba3e107cadea3da0a75c5/lib/datadog/tracing/trace_operation.rb#L214

Which blows up when we try to serialize it:

https://github.com/DataDog/dd-trace-rb/blob/867b104d548fca14ba132d747a868e048851ff89/lib/datadog/tracing/distributed/trace_context.rb#L116

/app/vendor/bundle/ruby/2.6.0/gems/ddtrace-1.11.1/lib/datadog/tracing/distributed/trace_context.rb:116:in `format': can't convert nil into Integer (TypeError)

Steps to reproduce

1. Configure c.tracing.instrument :pg, comment_propagation: 'full'
2. Configure DD_TRACE_ENABLED to be false

Environment

• ddtrace version: 1.11.1
• Ruby version: 2.6
• Relevant library versions: pg version 0.21.0

[delner]

Thanks for reporting this! @TonyCTHsu any thoughts? Given you've done some work on this DBM feature?

[TonyCTHsu]

👋 @nightpool , we have already released a fix for this in 1.12.0, could you upgrade to 1.12.0 and give it a try?

https://github.com/DataDog/dd-trace-rb/blob/master/CHANGELOG.md#1120---2023-06-02 https://github.com/DataDog/dd-trace-rb/pull/2866

[camsteffen]

I hit this error in 1.12.1.

[TonyCTHsu]

👋 @camsteffen , Could you provide me your backtrace and ways to reproduce it?

Currently, I expect to see

Sql comment propagation with `full` mode is aborted, because tracing is disabled. Please set `Datadog.configuration.tracing.enabled = true` to continue.

[camsteffen]

Maybe my case is a little bit different. Here is a reproduction script.

rails new dd-int-nil && cd dd-int-nil
bundle add sidekiq
bundle add ddtrace --require 'ddtrace/auto_instrument'
rails generate sidekiq:job my
cat <<EOF > config/initializers/datadog.rb
Rails.application.config.after_initialize do |app|
  Datadog.configure do |c|
    c.tracing.enabled = false
    c.tracing.distributed_tracing.propagation_style = ["tracecontext"]
    c.tracing.instrument :sidekiq, distributed_tracing: true
  end
end
EOF
rails runner 'MyJob.perform_async'

Error output:

E, [2023-09-01T10:32:06.410484 #46076] ERROR -- ddtrace: [ddtrace] (ruby/3.1.2/lib/ruby/gems/3.1.0/gems/ddtrace-1.14.0/lib/datadog/tracing/distributed/propagation.rb:59:in `rescue in block in inject!') Error injecting distributed trace data. Cause: can't convert nil into Integer Location: ruby/3.1.2/lib/ruby/gems/3.1.0/gems/ddtrace-1.14.0/lib/datadog/tracing/distributed/trace_context.rb:124:in `format'

[TonyCTHsu]

👋 @camsteffen , This is an rescued error when propagating trace context for distributed tracing.

The code tried to inject context with the trace, however, since tracing is disabled. The trace failed to converted into the format could be injected. We rescued this error and output the warning.

As far as I could tell, you code would still ran without problem.

[camsteffen]

That's true it wouldn't break my code, but the error is an unnecessary distraction.