Support ignoring queries in the pg instrumentation

[akiellor]

This PR adds support for ignoring queries sent to the pg instrumentation, similar to untraced_hosts for the net_http instrumentation.

The motivation for the feature is to ignore some frequent but low-value queries, in our case, the ; query which is used by ActiveRecord to check if a connection is active.

[linux-foundation-easycla]

• ❌ - login: @akiellor / name: Andrew Kiellor . The commit (894b0d7) is not authorized under a signed CLA. Please click here to be authorized. For further assistance with EasyCLA, please submit a support request ticket.

[arielvalentin]

Thank you for your contribution. We strive to reduce the number of configuration options and complexity in client instrumentation.

Generally speaking we recommend using the otel collector or vendor solution to filter spans. Have you considered adding a span filter for this in your collector?

https://opentelemetry.io/docs/collector/transforming-telemetry/#basic-filtering

[akiellor]

@arielvalentin, thats an interesting perspective. At $work we've been taking the "dumb-pipes" approach and keeping as much logic out of collector as possible. This has the advantage of folks being able to get a good sense of how the app is instrumented as our logs emit a waterfall view of the trace in a cognitively similar way to our production observability tool.

What is driving the philosophy of leveraging the collector for this sort of thing?

[robertlaurin]

At $work we've been taking the "dumb-pipes" approach and keeping as much logic out of collector as possible.

That's interesting as the collectors exist as the place where post processing of telemetry data is well supported, consider a finished span is immutable, and the on_finish hook in span processors are unable to modify anything here.

This has the advantage of folks being able to get a good sense of how the app is instrumented as our logs emit a waterfall view of the trace in a cognitively similar way to our production observability tool.

I can definitely appreciate that.

What is driving the philosophy of leveraging the collector for this sort of thing?

From the otel col repo

The OpenTelemetry Collector offers a vendor-agnostic implementation on how to receive, process and export telemetry data.

So depending on what your deployment looks like it might make sense to configure standard rules for all applications in your collection infra rather than have to make sure every app is configured the same across language implementations.

Context is really everything here, so what makes sense for one company may not for another. However control of span volume in a collector is fairly standard practice.

That said, from the perspective as one of the maintainers here we are really putting in a lot of effort constrain the amount of config options we have to support and are pushing users to use existing approaches rather than adding another config opt.

For example if you want your application to not emit db spans with the db statement of ;, you can add a custom sampler to your app to drop them.

This approach is really flexible for spans that have no children. It also has the benefit that you don't have to introduce a proc call to every single db query for every user of the otel pg instrumentation library regardless of them using this feature or not.

require 'bundler/inline'

gemfile(true) do
  source 'https://rubygems.org'

  gem 'pry'
  gem 'opentelemetry-api'
  gem 'opentelemetry-sdk'
  gem 'opentelemetry-exporter-otlp'
end

require 'opentelemetry/exporter/otlp'
require 'opentelemetry-api'
require 'opentelemetry-sdk'

span_processor = OpenTelemetry::SDK::Trace::Export::BatchSpanProcessor.new(
  OpenTelemetry::Exporter::OTLP::Exporter.new(
    endpoint: 'http://localhost:4318/v1/traces',
  )
)

OpenTelemetry::SDK.configure do |c|
  c.service_name = "custom-sampled"
  c.use_all
  c.add_span_processor(span_processor)
end

class FilterSampler
  RESULT = OpenTelemetry::SDK::Trace::Samplers::Result
  DECISION = OpenTelemetry::SDK::Trace::Samplers::Decision

  def initialize(sampler)
    @sampler = sampler
  end

  def description
    "My custom sampler"
  end

  def should_sample?(trace_id:, parent_context:, links:, name:, kind:, attributes:)
    if attributes && attributes["db.statement"] == ";"
      RESULT.new(decision: DECISION::DROP, tracestate: @tracestate)
    else
      @sampler.should_sample?(trace_id: trace_id, parent_context: parent_context, links: links, name: name, kind: kind, attributes: attributes)
    end
  end
end

OpenTelemetry.tracer_provider.sampler = FilterSampler.new(
  OpenTelemetry::SDK::Trace::Samplers::ALWAYS_ON
)

tracer = OpenTelemetry.tracer_provider.tracer('Sampler')

tracer.in_span("parent_span") do
  1..10.times do
    tracer.in_span("noisy db call", attributes: { "db.statement"=> ";" }, kind: :client) {}
    tracer.in_span("real db call", attributes: { "db.statement"=> "select * from hats;" }, kind: :client) {}
  end
end

OpenTelemetry.tracer_provider.shutdown

[akiellor]

@robertlaurin thank you for the comprehensive response, this seems like a very reasonable path forward and I may go with it.

Would taking this approach imply that all events will be subjected to the sampler, i.e. every span will be tested to see if it matches attributes && attributes["db.statement"] == ";"?

[robertlaurin]

Would taking this approach imply that all events will be subjected to the sampler, i.e. every span will be tested to see if it matches attributes && attributes["db.statement"] == ";"?

That's correct, every span is evaluated against the sampler on create so you want to keep that in mind when you write your sampling logic. Depending on the volume of spans you emit in any given operation in your service this may or may not be considered a hot path.

Internally at our org we typically short circuit all the sampler rules with a kind check, to make sure we don't look for a db attribute on any of the server, internal, consumer, producer spans.

For example we would do the following to drop database spans in a sampler

kind == :client && attributes&.key?('db.system')