diff --git a/lib/ddtrace/contrib/active_model_serializers/patcher.rb b/lib/ddtrace/contrib/active_model_serializers/patcher.rb index 35c994f81a..3fbb31cfbd 100644 --- a/lib/ddtrace/contrib/active_model_serializers/patcher.rb +++ b/lib/ddtrace/contrib/active_model_serializers/patcher.rb @@ -21,13 +21,6 @@ def patch begin # Subscribe to ActiveModelSerializers events Events.subscribe! - - # Set service info - get_option(:tracer).set_service_info( - get_option(:service_name), - Ext::APP, - Datadog::Ext::AppTypes::WEB - ) rescue StandardError => e Datadog::Tracer.log.error("Unable to apply ActiveModelSerializers integration: #{e}") end diff --git a/lib/ddtrace/contrib/active_record/configuration/settings.rb b/lib/ddtrace/contrib/active_record/configuration/settings.rb index b577aeef2a..c7dd7f033a 100644 --- a/lib/ddtrace/contrib/active_record/configuration/settings.rb +++ b/lib/ddtrace/contrib/active_record/configuration/settings.rb @@ -17,11 +17,9 @@ class Settings < Contrib::Configuration::Settings lazy: true option :orm_service_name - option :service_name, depends_on: [:tracer] do |value| - (value || Utils.adapter_name).tap do |service_name| - tracer.set_service_info(service_name, Ext::APP, Datadog::Ext::AppTypes::DB) - end - end + option :service_name, + default: -> { Utils.adapter_name }, + lazy: true option :tracer, default: Datadog.tracer do |value| value.tap do diff --git a/lib/ddtrace/contrib/dalli/patcher.rb b/lib/ddtrace/contrib/dalli/patcher.rb index 85e8a584e4..79ef0fd041 100644 --- a/lib/ddtrace/contrib/dalli/patcher.rb +++ b/lib/ddtrace/contrib/dalli/patcher.rb @@ -21,13 +21,6 @@ def patch begin add_pin! ::Dalli::Server.send(:include, Instrumentation) - - # TODO: When Dalli pin is removed, set service info. - # get_option(:tracer).set_service_info( - # get_option(:service_name), - # Ext::APP, - # Datadog::Ext::AppTypes::CACHE - # ) rescue StandardError => e Datadog::Tracer.log.error("Unable to apply Dalli integration: #{e}") end diff --git a/lib/ddtrace/contrib/faraday/middleware.rb b/lib/ddtrace/contrib/faraday/middleware.rb index 95854b2f86..9ebde4ab7a 100644 --- a/lib/ddtrace/contrib/faraday/middleware.rb +++ b/lib/ddtrace/contrib/faraday/middleware.rb @@ -82,8 +82,6 @@ def analytics_sample_rate def setup_service! return if options[:service_name] == datadog_configuration[:service_name] - - Patcher.register_service(options[:service_name]) end end end diff --git a/lib/ddtrace/contrib/faraday/patcher.rb b/lib/ddtrace/contrib/faraday/patcher.rb index fa1f3969a4..e6ef88047a 100644 --- a/lib/ddtrace/contrib/faraday/patcher.rb +++ b/lib/ddtrace/contrib/faraday/patcher.rb @@ -22,9 +22,6 @@ def patch add_pin! add_middleware! - - # TODO: When Faraday pin is removed, set service info. - # register_service(get_option(:service_name)) rescue StandardError => e Datadog::Tracer.log.error("Unable to apply Faraday integration: #{e}") end @@ -45,14 +42,6 @@ def add_middleware! ::Faraday::Middleware.register_middleware(ddtrace: Middleware) end - def register_service(name) - get_option(:tracer).set_service_info( - name, - Ext::APP, - Datadog::Ext::AppTypes::WEB - ) - end - def get_option(option) Datadog.configuration[:faraday].get_option(option) end diff --git a/lib/ddtrace/contrib/grape/patcher.rb b/lib/ddtrace/contrib/grape/patcher.rb index a68b229971..e32465711b 100644 --- a/lib/ddtrace/contrib/grape/patcher.rb +++ b/lib/ddtrace/contrib/grape/patcher.rb @@ -26,13 +26,6 @@ def patch add_pin! - # TODO: When Grape pin is removed, set service info. - # get_option(:tracer).set_service_info( - # get_option(:service_name), - # Ext::APP, - # Datadog::Ext::AppTypes::WEB - # ) - # Subscribe to ActiveSupport events Datadog::Contrib::Grape::Endpoint.subscribe rescue StandardError => e diff --git a/lib/ddtrace/contrib/graphql/configuration/settings.rb b/lib/ddtrace/contrib/graphql/configuration/settings.rb index 68a35ed983..a90d226537 100644 --- a/lib/ddtrace/contrib/graphql/configuration/settings.rb +++ b/lib/ddtrace/contrib/graphql/configuration/settings.rb @@ -17,10 +17,7 @@ class Settings < Contrib::Configuration::Settings lazy: true option :schemas - option :service_name, default: Ext::SERVICE_NAME, depends_on: [:tracer] do |value| - get_option(:tracer).set_service_info(value, Ext::APP, Datadog::Ext::AppTypes::WEB) - value - end + option :service_name, default: Ext::SERVICE_NAME end end end diff --git a/lib/ddtrace/contrib/grpc/patcher.rb b/lib/ddtrace/contrib/grpc/patcher.rb index 7d76309eb4..ebec127ed8 100644 --- a/lib/ddtrace/contrib/grpc/patcher.rb +++ b/lib/ddtrace/contrib/grpc/patcher.rb @@ -23,13 +23,6 @@ def patch add_pin! - # TODO: When GRPC pin is removed, set service info. - # get_option(:tracer).set_service_info( - # get_option(:service_name), - # Ext::APP, - # Datadog::Ext::AppTypes::WEB - # ) - prepend_interceptor rescue StandardError => e Datadog::Tracer.log.error("Unable to apply gRPC integration: #{e}") diff --git a/lib/ddtrace/contrib/racecar/patcher.rb b/lib/ddtrace/contrib/racecar/patcher.rb index 2528325bdd..a4eddd21f9 100644 --- a/lib/ddtrace/contrib/racecar/patcher.rb +++ b/lib/ddtrace/contrib/racecar/patcher.rb @@ -21,14 +21,6 @@ def patch begin # Subscribe to Racecar events Events.subscribe! - - # Set service info - configuration = Datadog.configuration[:racecar] - configuration[:tracer].set_service_info( - configuration[:service_name], - Ext::APP, - Datadog::Ext::AppTypes::WORKER - ) rescue StandardError => e Datadog::Tracer.log.error("Unable to apply Racecar integration: #{e}") end diff --git a/lib/ddtrace/contrib/rack/configuration/settings.rb b/lib/ddtrace/contrib/rack/configuration/settings.rb index 00367c240d..3594bb8321 100644 --- a/lib/ddtrace/contrib/rack/configuration/settings.rb +++ b/lib/ddtrace/contrib/rack/configuration/settings.rb @@ -29,17 +29,9 @@ class Settings < Contrib::Configuration::Settings option :quantize, default: {} option :request_queuing, default: false - option :service_name, default: Ext::SERVICE_NAME, depends_on: [:tracer] do |value| - get_option(:tracer).set_service_info(value, Ext::APP, Datadog::Ext::AppTypes::WEB) - value - end + option :service_name, default: Ext::SERVICE_NAME - option :web_service_name, default: Ext::WEBSERVER_SERVICE_NAME, depends_on: [:tracer, :request_queuing] do |value| - if get_option(:request_queuing) - get_option(:tracer).set_service_info(value, Ext::WEBSERVER_APP, Datadog::Ext::AppTypes::WEB) - end - value - end + option :web_service_name, default: Ext::WEBSERVER_SERVICE_NAME end end end diff --git a/lib/ddtrace/contrib/rails/framework.rb b/lib/ddtrace/contrib/rails/framework.rb index 379709edc7..c85971edcc 100644 --- a/lib/ddtrace/contrib/rails/framework.rb +++ b/lib/ddtrace/contrib/rails/framework.rb @@ -26,7 +26,6 @@ def self.setup activate_rack!(config) activate_active_record!(config) - set_service_info!(config) # By default, default service would be guessed from the script # being executed, but here we know better, get it from Rails config. @@ -64,12 +63,6 @@ def self.activate_active_record!(config) tracer: config[:tracer] ) end - - def self.set_service_info!(config) - tracer = config[:tracer] - tracer.set_service_info(config[:controller_service], Ext::APP, Datadog::Ext::AppTypes::WEB) - tracer.set_service_info(config[:cache_service], Ext::APP, Datadog::Ext::AppTypes::CACHE) - end end end end diff --git a/lib/ddtrace/contrib/rake/patcher.rb b/lib/ddtrace/contrib/rake/patcher.rb index fb8f8cfedb..83c2fae81a 100644 --- a/lib/ddtrace/contrib/rake/patcher.rb +++ b/lib/ddtrace/contrib/rake/patcher.rb @@ -21,13 +21,6 @@ def patch begin # Add instrumentation patch to Rake task ::Rake::Task.send(:include, Instrumentation) - - # Set service info - get_option(:tracer).set_service_info( - get_option(:service_name), - Ext::APP, - Datadog::Ext::AppTypes::WORKER - ) rescue StandardError => e Datadog::Tracer.log.error("Unable to apply Rake integration: #{e}") end diff --git a/lib/ddtrace/contrib/rest_client/configuration/settings.rb b/lib/ddtrace/contrib/rest_client/configuration/settings.rb index ceed92d9c1..34733640a7 100644 --- a/lib/ddtrace/contrib/rest_client/configuration/settings.rb +++ b/lib/ddtrace/contrib/rest_client/configuration/settings.rb @@ -16,10 +16,7 @@ class Settings < Contrib::Configuration::Settings lazy: true option :distributed_tracing, default: true - option :service_name, default: Ext::SERVICE_NAME, depends_on: [:tracer] do |value| - get_option(:tracer).set_service_info(value, Ext::APP, Datadog::Ext::AppTypes::WEB) - value - end + option :service_name, default: Ext::SERVICE_NAME end end end diff --git a/lib/ddtrace/contrib/shoryuken/tracer.rb b/lib/ddtrace/contrib/shoryuken/tracer.rb index 2ad32ca2aa..f7295bbd8e 100644 --- a/lib/ddtrace/contrib/shoryuken/tracer.rb +++ b/lib/ddtrace/contrib/shoryuken/tracer.rb @@ -8,7 +8,6 @@ class Tracer def initialize(options = {}) @tracer = options[:tracer] || configuration[:tracer] @shoryuken_service = options[:service_name] || configuration[:service_name] - set_service_info(@shoryuken_service) end def call(worker_instance, queue, sqs_msg, body) @@ -40,15 +39,6 @@ def resource(worker_instance, body) def configuration Datadog.configuration[:shoryuken] end - - def set_service_info(service) - return if @tracer.nil? || @tracer.services[service] - @tracer.set_service_info( - service, - Ext::APP, - Datadog::Ext::AppTypes::WORKER - ) - end end end end diff --git a/lib/ddtrace/contrib/sidekiq/client_tracer.rb b/lib/ddtrace/contrib/sidekiq/client_tracer.rb index c2773bab90..aa7bacefd6 100644 --- a/lib/ddtrace/contrib/sidekiq/client_tracer.rb +++ b/lib/ddtrace/contrib/sidekiq/client_tracer.rb @@ -11,7 +11,6 @@ class ClientTracer def initialize(options = {}) super @sidekiq_service = options[:client_service_name] || configuration[:client_service_name] - set_service_info(@sidekiq_service) end # Client middleware arguments are documented here: diff --git a/lib/ddtrace/contrib/sidekiq/server_tracer.rb b/lib/ddtrace/contrib/sidekiq/server_tracer.rb index cffc0b0d89..fd3303cb49 100644 --- a/lib/ddtrace/contrib/sidekiq/server_tracer.rb +++ b/lib/ddtrace/contrib/sidekiq/server_tracer.rb @@ -17,7 +17,6 @@ def call(worker, job, queue) resource = job_resource(job) service = service_from_worker_config(resource) || @sidekiq_service - set_service_info(service) @tracer.trace(Ext::SPAN_JOB, service: service, span_type: Datadog::Ext::AppTypes::WORKER) do |span| span.resource = resource diff --git a/lib/ddtrace/contrib/sidekiq/tracing.rb b/lib/ddtrace/contrib/sidekiq/tracing.rb index 87c265d5e8..afc44f1df9 100644 --- a/lib/ddtrace/contrib/sidekiq/tracing.rb +++ b/lib/ddtrace/contrib/sidekiq/tracing.rb @@ -22,16 +22,6 @@ def job_resource(job) job['class'] end end - - def set_service_info(service) - # Ensure the tracer knows about this service. - return if @tracer.services[service] - @tracer.set_service_info( - service, - Ext::APP, - Datadog::Ext::AppTypes::WORKER - ) - end end end end diff --git a/lib/ddtrace/contrib/sinatra/configuration/settings.rb b/lib/ddtrace/contrib/sinatra/configuration/settings.rb index 8ebebabc03..44bc4d8f4d 100644 --- a/lib/ddtrace/contrib/sinatra/configuration/settings.rb +++ b/lib/ddtrace/contrib/sinatra/configuration/settings.rb @@ -24,10 +24,7 @@ class Settings < Contrib::Configuration::Settings option :headers, default: DEFAULT_HEADERS option :resource_script_names, default: false - option :service_name, default: Ext::SERVICE_NAME, depends_on: [:tracer] do |value| - get_option(:tracer).set_service_info(value, Ext::APP, Datadog::Ext::AppTypes::WEB) - value - end + option :service_name, default: Ext::SERVICE_NAME end end end diff --git a/lib/ddtrace/encoding.rb b/lib/ddtrace/encoding.rb index b3feb6e2ff..5368ea39e4 100644 --- a/lib/ddtrace/encoding.rb +++ b/lib/ddtrace/encoding.rb @@ -21,11 +21,6 @@ def encode_traces(traces) encode(to_send) end - # Encodes services hash - def encode_services(services) - encode(services) - end - # Defines the underlying format used during traces or services encoding. # This method must be implemented and should only be used by the internal functions. def encode(_) diff --git a/lib/ddtrace/pin.rb b/lib/ddtrace/pin.rb index 9fe6e809b7..5ff908819a 100644 --- a/lib/ddtrace/pin.rb +++ b/lib/ddtrace/pin.rb @@ -57,8 +57,6 @@ def datadog_pin end def service_name=(name) - tracer.set_service_info(name, app, app_type) if name && app && app_type - @service_name = name end diff --git a/lib/ddtrace/sync_writer.rb b/lib/ddtrace/sync_writer.rb index 3c489d60e1..b5ac47f065 100644 --- a/lib/ddtrace/sync_writer.rb +++ b/lib/ddtrace/sync_writer.rb @@ -10,9 +10,17 @@ def initialize(options = {}) end end - def write(trace, services) + def write(trace, services = nil) + unless services.nil? + Datadog::Patcher.do_once('SyncWriter#write') do + Datadog::Tracer.log.warn(%( + write: Writing services has been deprecated and no longer need to be provided. + write(traces, services) can be updted to write(traces) + )) + end + end + perform_concurrently( - proc { flush_services(services) }, proc { flush_trace(trace) } ) rescue => e @@ -25,10 +33,6 @@ def perform_concurrently(*tasks) tasks.map { |task| Thread.new(&task) }.each(&:join) end - def flush_services(services) - transport.send(:services, services) - end - def flush_trace(trace) processed_traces = Pipeline.process!([trace]) transport.send(:traces, processed_traces) diff --git a/lib/ddtrace/tracer.rb b/lib/ddtrace/tracer.rb index d36980ea40..2557720c95 100644 --- a/lib/ddtrace/tracer.rb +++ b/lib/ddtrace/tracer.rb @@ -20,7 +20,7 @@ module Datadog # of these function calls and sub-requests would be encapsulated within a single trace. # rubocop:disable Metrics/ClassLength class Tracer - attr_reader :sampler, :services, :tags, :provider + attr_reader :sampler, :tags, :provider attr_accessor :enabled, :writer attr_writer :default_service @@ -67,6 +67,15 @@ def self.debug_logging log.level == Logger::DEBUG end + def services + # Only log each deprecation warning once (safeguard against log spam) + Datadog::Patcher.do_once('Tracer#set_service_info') do + Datadog::Tracer.log.warn('services: Usage of Tracer.services has been deprecated') + end + + {} + end + # Shorthand that calls the `shutdown!` method of a registered worker. # It's useful to ensure that the Trace Buffer is properly flushed before # shutting down the application. @@ -110,7 +119,6 @@ def initialize(options = {}) @context_flush = options[:partial_flush] ? Datadog::ContextFlush.new(options) : nil @mutex = Mutex.new - @services = {} @tags = {} end @@ -162,14 +170,16 @@ def configure(options = {}) # Set the information about the given service. A valid example is: # # tracer.set_service_info('web-application', 'rails', 'web') + # + # set_service_info is deprecated, no service information needs to be tracked def set_service_info(service, app, app_type) - @services[service] = { - 'app' => app, - 'app_type' => app_type - } - - return unless Datadog::Tracer.debug_logging - Datadog::Tracer.log.debug("set_service_info: service: #{service} app: #{app} type: #{app_type}") + # Only log each deprecation warning once (safeguard against log spam) + Datadog::Patcher.do_once('Tracer#set_service_info') do + Datadog::Tracer.log.warn(%( + set_service_info: Usage of set_service_info has been deprecated, + service information no longer needs to be reported to the trace agent. + )) + end end # A default value for service. One should really override this one @@ -380,8 +390,7 @@ def write(trace) Datadog::Tracer.log.debug(str) end - @writer.write(trace, @services) - @services = {} + @writer.write(trace) end private :write, :guess_context_and_parent diff --git a/lib/ddtrace/transport.rb b/lib/ddtrace/transport.rb index 6b9e49e29d..e84726d02d 100644 --- a/lib/ddtrace/transport.rb +++ b/lib/ddtrace/transport.rb @@ -11,7 +11,7 @@ module Datadog # rubocop:disable Metrics/ClassLength class HTTPTransport attr_accessor :hostname, :port - attr_reader :traces_endpoint, :services_endpoint + attr_reader :traces_endpoint DEFAULT_AGENT_HOST = '127.0.0.1'.freeze DEFAULT_TRACE_AGENT_PORT = '8126'.freeze @@ -27,21 +27,18 @@ class HTTPTransport V4 = 'v0.4'.freeze => { version: V4, traces_endpoint: '/v0.4/traces'.freeze, - services_endpoint: '/v0.4/services'.freeze, encoder: Encoding::MsgpackEncoder, fallback: 'v0.3'.freeze }.freeze, V3 = 'v0.3'.freeze => { version: V3, traces_endpoint: '/v0.3/traces'.freeze, - services_endpoint: '/v0.3/services'.freeze, encoder: Encoding::MsgpackEncoder, fallback: 'v0.2'.freeze }.freeze, V2 = 'v0.2'.freeze => { version: V2, traces_endpoint: '/v0.2/traces'.freeze, - services_endpoint: '/v0.2/services'.freeze, encoder: Encoding::JSONEncoder }.freeze }.freeze @@ -78,10 +75,7 @@ def initialize(options = {}) def send(endpoint, data) case endpoint when :services - payload = @encoder.encode_services(data) - status_code = post(@api[:services_endpoint], payload) do |response| - process_callback(:services, response) - end + return nil when :traces count = data.length payload = @encoder.encode_traces(data) diff --git a/lib/ddtrace/workers.rb b/lib/ddtrace/workers.rb index 6e05a560a5..a85675ad99 100644 --- a/lib/ddtrace/workers.rb +++ b/lib/ddtrace/workers.rb @@ -16,7 +16,6 @@ class AsyncTransport SHUTDOWN_TIMEOUT = 1 attr_reader \ - :service_buffer, :trace_buffer def initialize(options = {}) @@ -24,7 +23,6 @@ def initialize(options = {}) # Callbacks @trace_task = options[:on_trace] - @service_task = options[:on_service] @runtime_metrics_task = options[:on_runtime_metrics] # Intervals @@ -35,7 +33,6 @@ def initialize(options = {}) # Buffers buffer_size = options.fetch(:buffer_size, 100) @trace_buffer = TraceBuffer.new(buffer_size) - @service_buffer = TraceBuffer.new(buffer_size) # Threading @shutdown = ConditionVariable.new @@ -60,21 +57,6 @@ def callback_traces end end - # Callback function that process traces and executes the +send_services()+ method. - def callback_services - return true if @service_buffer.empty? - - begin - services = @service_buffer.pop() - @service_task.call(services[0], @transport) unless @service_task.nil? - rescue StandardError => e - # ensures that the thread will not die because of an exception. - # TODO[manu]: findout the reason and reschedule the send if it's not - # a fatal exception - Datadog::Tracer.log.error("Error during services flush: dropped #{services.length} items. Cause: #{e}") - end - end - def callback_runtime_metrics @runtime_metrics_task.call unless @runtime_metrics_task.nil? rescue StandardError => e @@ -91,13 +73,12 @@ def start end end - # Closes all available queues and waits for the trace and service buffer to flush + # Closes all available queues and waits for the trace buffer to flush def stop @mutex.synchronize do return unless @run @trace_buffer.close - @service_buffer.close @run = false @shutdown.signal end @@ -117,12 +98,6 @@ def enqueue_trace(trace) @trace_buffer.push(trace) end - # Enqueue an item in the service internal buffer. This operation is thread-safe. - def enqueue_service(service) - return if service == {} # no use to send this, not worth it - @service_buffer.push(service) - end - private alias flush_data callback_traces @@ -131,11 +106,10 @@ def perform loop do @back_off = flush_data ? @flush_interval : [@back_off * BACK_OFF_RATIO, BACK_OFF_MAX].min - callback_services callback_runtime_metrics @mutex.synchronize do - return if !@run && @trace_buffer.empty? && @service_buffer.empty? + return if !@run && @trace_buffer.empty? @shutdown.wait(@mutex, @back_off) if @run # do not wait when shutting down end end diff --git a/lib/ddtrace/writer.rb b/lib/ddtrace/writer.rb index cbed7d0ac3..31af72a390 100644 --- a/lib/ddtrace/writer.rb +++ b/lib/ddtrace/writer.rb @@ -34,31 +34,25 @@ def initialize(options = {}) Runtime::Metrics.new end - @services = {} - # handles the thread creation after an eventual fork @mutex_after_fork = Mutex.new @pid = nil @traces_flushed = 0 - @services_flushed = 0 - # one worker for both services and traces, each have their own queues + # one worker for traces @worker = nil end - # spawns two different workers for spans and services; - # they share the same transport which is thread-safe + # spawns a worker for spans; they share the same transport which is thread-safe def start @pid = Process.pid @trace_handler = ->(items, transport) { send_spans(items, transport) } - @service_handler = ->(items, transport) { send_services(items, transport) } @runtime_metrics_handler = -> { send_runtime_metrics } @worker = Datadog::Workers::AsyncTransport.new( transport: @transport, buffer_size: @buff_size, on_trace: @trace_handler, - on_service: @service_handler, on_runtime_metrics: @runtime_metrics_handler, interval: @flush_interval ) @@ -66,7 +60,7 @@ def start @worker.start() end - # stops both workers for spans and services. + # stops worker for spans. def stop @worker.stop() @worker = nil @@ -83,17 +77,6 @@ def send_spans(traces, transport) status end - # flush services to the trace-agent, handles services only - def send_services(services, transport) - return true if services.empty? - - code = transport.send(:services, services) - status = !transport.server_error?(code) - @services_flushed += 1 if status - - status - end - def send_runtime_metrics return unless Datadog.configuration.runtime_metrics_enabled @@ -101,7 +84,16 @@ def send_runtime_metrics end # enqueue the trace for submission to the API - def write(trace, services) + def write(trace, services = nil) + unless services.nil? + Datadog::Patcher.do_once('Writer#write') do + Datadog::Tracer.log.warn(%( + write: Writing services has been deprecated and no longer need to be provided. + write(traces, services) can be updted to write(traces) + )) + end + end + # In multiprocess environments, the main process initializes the +Writer+ instance and if # the process forks (i.e. a web server like Unicorn or Puma with multiple workers) the new # processes will share the same +Writer+ until the first write (COW). Because of that, @@ -122,14 +114,12 @@ def write(trace, services) runtime_metrics.associate_with_span(trace.first) unless trace.empty? @worker.enqueue_trace(trace) - @worker.enqueue_service(services) end # stats returns a dictionary of stats about the writer. def stats { traces_flushed: @traces_flushed, - services_flushed: @services_flushed, transport: @transport.stats } end diff --git a/spec/ddtrace/contrib/active_record/tracer_spec.rb b/spec/ddtrace/contrib/active_record/tracer_spec.rb index db58feabcc..50c1e69d66 100644 --- a/spec/ddtrace/contrib/active_record/tracer_spec.rb +++ b/spec/ddtrace/contrib/active_record/tracer_spec.rb @@ -32,7 +32,6 @@ let(:spans) { tracer.writer.spans } let(:span) { spans.first } - let(:services) { tracer.writer.services } it_behaves_like 'analytics for integration' do let(:analytics_enabled_var) { Datadog::Contrib::ActiveRecord::Ext::ENV_ANALYTICS_ENABLED } @@ -42,8 +41,6 @@ it 'calls the instrumentation when is used standalone' do # expect service and trace is sent expect(spans.size).to eq(1) - expect(services).to_not be_empty - expect(services['mysql2']).to eq('app' => 'active_record', 'app_type' => 'db') expect(span.service).to eq('mysql2') expect(span.name).to eq('mysql2.query') diff --git a/spec/ddtrace/contrib/redis/redis_spec.rb b/spec/ddtrace/contrib/redis/redis_spec.rb index 043eb58ae0..35eb929fe5 100644 --- a/spec/ddtrace/contrib/redis/redis_spec.rb +++ b/spec/ddtrace/contrib/redis/redis_spec.rb @@ -193,28 +193,6 @@ def all_spans it_behaves_like 'a span with common tags' end end - - context 'service name' do - let(:services) { tracer.writer.services } - let(:service_name) { 'redis-test' } - - before(:each) do - redis.set 'FOO', 'bar' - tracer.writer.services # empty queue - Datadog.configure( - client, - service_name: service_name, - tracer: tracer, - app_type: Datadog::Ext::AppTypes::CACHE - ) - redis.set 'FOO', 'bar' - end - - it do - expect(services).to have(1).items - expect(services[service_name]).to eq('app' => 'redis', 'app_type' => 'cache') - end - end end it_behaves_like 'a Redis driver', :ruby diff --git a/spec/ddtrace/integration_spec.rb b/spec/ddtrace/integration_spec.rb index 2688ec18e6..658c38b23a 100644 --- a/spec/ddtrace/integration_spec.rb +++ b/spec/ddtrace/integration_spec.rb @@ -44,21 +44,16 @@ def agent_receives_span_step1 end def agent_receives_span_step2 - tracer.set_service_info('my.service', 'rails', 'web') - create_trace # Timeout after 3 seconds, waiting for 1 flush wait_for_flush(:traces_flushed) - # Timeout after 3 seconds, waiting for 1 flush - wait_for_flush(:services_flushed) - stats = tracer.writer.stats expect(stats[:traces_flushed]).to eq(1) - expect(stats[:services_flushed]).to eq(1) - # Number of successes can be 1 or 2 because services count as one flush too - expect(stats[:transport][:success]).to be >= 1 + expect(stats[:services_flushed]).to be_nil + # Number of successes will only be 1 since we do not flush services + expect(stats[:transport][:success]).to eq(1) expect(stats[:transport][:client_error]).to eq(0) expect(stats[:transport][:server_error]).to eq(0) expect(stats[:transport][:internal_error]).to eq(0) @@ -74,7 +69,7 @@ def agent_receives_span_step3(previous_success) stats = tracer.writer.stats expect(stats[:traces_flushed]).to eq(2) - expect(stats[:services_flushed]).to eq(1) + expect(stats[:services_flushed]).to be_nil expect(stats[:transport][:success]).to be > previous_success expect(stats[:transport][:client_error]).to eq(0) expect(stats[:transport][:server_error]).to eq(0) @@ -92,8 +87,6 @@ def agent_receives_span_step3(previous_success) include_context 'agent-based test' before(:each) do - tracer.set_service_info('my.service', 'rails', 'web') - tracer.trace('my.short.op') do |span| @span = span span.service = 'my.service' @@ -108,7 +101,7 @@ def agent_receives_span_step3(previous_success) expect(@first_shutdown).to be true expect(@span.finished?).to be true expect(stats[:traces_flushed]).to eq(1) - expect(stats[:services_flushed]).to eq(1) + expect(stats[:services_flushed]).to be_nil expect(stats[:transport][:client_error]).to eq(0) expect(stats[:transport][:server_error]).to eq(0) expect(stats[:transport][:internal_error]).to eq(0) @@ -119,8 +112,6 @@ def agent_receives_span_step3(previous_success) include_context 'agent-based test' before(:each) do - tracer.set_service_info('my.service', 'rails', 'web') - tracer.trace('my.short.op') do |span| span.service = 'my.service' end @@ -140,7 +131,7 @@ def agent_receives_span_step3(previous_success) it do expect(@shutdown_results.count(true)).to eq(1) expect(stats[:traces_flushed]).to eq(1) - expect(stats[:services_flushed]).to eq(1) + expect(stats[:services_flushed]).to be_nil expect(stats[:transport][:client_error]).to eq(0) expect(stats[:transport][:server_error]).to eq(0) expect(stats[:transport][:internal_error]).to eq(0) diff --git a/spec/ddtrace/pin_spec.rb b/spec/ddtrace/pin_spec.rb index 26c70d091a..e9a38bf918 100644 --- a/spec/ddtrace/pin_spec.rb +++ b/spec/ddtrace/pin_spec.rb @@ -24,13 +24,6 @@ context 'when given sufficient info' do let(:options) { { app: 'test-app', app_type: 'test-type', tracer: tracer } } let(:tracer) { get_test_tracer } - - it 'sets the service info' do - expect(tracer.services.key?(service_name)).to be true - expect(tracer.services[service_name]).to eq( - 'app' => 'test-app', 'app_type' => 'test-type' - ) - end end context 'when given insufficient info' do diff --git a/spec/ddtrace/tracer_spec.rb b/spec/ddtrace/tracer_spec.rb index 6a140058f0..692ec344c7 100644 --- a/spec/ddtrace/tracer_spec.rb +++ b/spec/ddtrace/tracer_spec.rb @@ -131,4 +131,26 @@ end end end + + describe '#set_service_info' do + include_context 'tracer logging' + + # Ensure we have a clean `@done_once` before and after each test + # so we can properly test the behavior here, and we don't pollute other tests + before(:each) { Datadog::Patcher.instance_variable_set(:@done_once, nil) } + after(:each) { Datadog::Patcher.instance_variable_set(:@done_once, nil) } + + before(:each) do + # Call multiple times to assert we only log once + tracer.set_service_info('service-A', 'app-A', 'app_type-A') + tracer.set_service_info('service-B', 'app-B', 'app_type-B') + tracer.set_service_info('service-C', 'app-C', 'app_type-C') + tracer.set_service_info('service-D', 'app-D', 'app_type-D') + end + + it 'generates a single deprecation warnings' do + expect(log_buffer.length).to be > 1 + expect(log_buffer).to contain_line_with('Usage of set_service_info has been deprecated') + end + end end diff --git a/spec/ddtrace/transport_spec.rb b/spec/ddtrace/transport_spec.rb index 0c929d356f..5fb018ccbf 100644 --- a/spec/ddtrace/transport_spec.rb +++ b/spec/ddtrace/transport_spec.rb @@ -154,34 +154,12 @@ end end + # Sending of services is deprecated and just returns `nil` context 'services' do subject(:code) { transport.send(:services, services) } let(:services) { get_test_services } - it_behaves_like 'an encoded transport' - - context 'when the agent returns a 404' do - before(:each) do - original_post = transport.method(:post) - call_count = 0 - allow(transport).to receive(:post) do |url, *rest| - if call_count > 0 - original_post.call(url, *rest) - else - call_count += 1 - 404 - end - end - end - - it 'appropriately downgrades the API' do - expect(transport.instance_variable_get(:@api)[:version]).to eq(described_class::V3) - code = transport.send(:services, services) - # HTTPTransport should downgrade the encoder and API level - expect(transport.instance_variable_get(:@api)[:version]).to eq(described_class::V2) - expect(transport.success?(code)).to be true - end - end + it { expect(code).to be_nil } end context 'admin' do diff --git a/spec/ddtrace/workers_integration_spec.rb b/spec/ddtrace/workers_integration_spec.rb index e30e350887..7ae281208e 100644 --- a/spec/ddtrace/workers_integration_spec.rb +++ b/spec/ddtrace/workers_integration_spec.rb @@ -25,7 +25,7 @@ let(:writer) do Datadog::Writer.new.tap do |w| # write some stuff to trigger a #start - w.write([], {}) + w.write([]) # now stop the writer and replace worker with ours, if we don't do # this the old worker will still be used. w.stop @@ -78,7 +78,7 @@ def wait_for_flush(num = 1, period = 0.1) it 'flushes the trace correctly' do expect(stats[:traces_flushed]).to be >= 1 - expect(stats[:services_flushed]).to eq(0) + expect(stats[:services_flushed]).to be_nil # Sanity checks expect(dump[200]).to_not be nil @@ -165,36 +165,23 @@ def wait_for_flush(num = 1, period = 0.1) describe 'when setting service info' do let(:dumped_services) { dump[200][:services] } - let(:service_payload) { JSON.parse(dumped_services[0]) } # Test that services are correctly flushed, with two of them context 'for two services' do before(:each) do - tracer.set_service_info('my.service', 'rails', 'web') - tracer.set_service_info('my.other.service', 'golang', 'api') tracer.start_span('my.op').finish - - wait_for_flush { writer.stats[:services_flushed] >= 1 } end it 'flushes the services correctly' do - expect(stats[:services_flushed]).to eq(1) + expect(stats[:services_flushed]).to be_nil # Sanity checks expect(dump[200]).to_not be nil expect(dump[500]).to_not be nil expect(dump[500]).to eq({}) - expect(dumped_services).to_not be nil - - # Unmarshalling data - expect(dumped_services).to have(1).items - expect(dumped_services[0]).to be_a_kind_of(String) - expect(service_payload).to be_a_kind_of(Hash) - expect(service_payload).to eq( - 'my.service' => { 'app' => 'rails', 'app_type' => 'web' }, - 'my.other.service' => { 'app' => 'golang', 'app_type' => 'api' } - ) + # No services information was sent + expect(dumped_services).to be_nil end end end @@ -208,7 +195,6 @@ def wait_for_flush(num = 1, period = 0.1) # Enqueue some work for a final flush worker.enqueue_trace(get_test_traces(1)) - worker.enqueue_service(get_test_services) # Interrupt back off and flush everything immediately @shutdown_beg = Time.now @@ -234,7 +220,7 @@ def wait_for_flush(num = 1, period = 0.1) it do expect(trace_task).to have_received(:call).once - expect(service_task).to have_received(:call).once + expect(service_task).to_not have_received(:call) expect(@shutdown_end - @shutdown_beg).to be < Datadog::Workers::AsyncTransport::SHUTDOWN_TIMEOUT end end diff --git a/spec/ddtrace/workers_spec.rb b/spec/ddtrace/workers_spec.rb index e143bb7f49..fe590b3bd5 100644 --- a/spec/ddtrace/workers_spec.rb +++ b/spec/ddtrace/workers_spec.rb @@ -11,18 +11,15 @@ transport: nil, buffer_size: 100, on_trace: task, - on_service: task, interval: 0.5 ) worker.enqueue_trace(get_test_traces(1)) - worker.enqueue_service(get_test_services) expect { worker.callback_traces }.to_not raise_error - expect { worker.callback_services }.to_not raise_error lines = buf.string.lines - expect(lines.count).to eq 2 + expect(lines.count).to eq 1 end end end diff --git a/spec/support/faux_writer.rb b/spec/support/faux_writer.rb index a034603b5a..95b086e129 100644 --- a/spec/support/faux_writer.rb +++ b/spec/support/faux_writer.rb @@ -11,14 +11,12 @@ def initialize(options = {}) # easy access to registered components @spans = [] - @services = {} end - def write(trace, services) + def write(trace) @mutex.synchronize do - super(trace, services) + super(trace) @spans << trace - @services = @services.merge(services) unless services.empty? end end @@ -55,12 +53,4 @@ def trace0_spans spans end end - - def services - @mutex.synchronize do - services = @services - @services = {} - services - end - end end diff --git a/spec/support/spy_transport.rb b/spec/support/spy_transport.rb index a17b5c7ce5..082716f66d 100644 --- a/spec/support/spy_transport.rb +++ b/spec/support/spy_transport.rb @@ -15,8 +15,6 @@ def initialize(options = {}) def send(endpoint, data) data = case endpoint - when :services - @helper_encoder.encode_services(data) when :traces @helper_encoder.encode_traces(data) end diff --git a/spec/support/test_access_patch.rb b/spec/support/test_access_patch.rb index debd8e5390..45f4e7263f 100644 --- a/spec/support/test_access_patch.rb +++ b/spec/support/test_access_patch.rb @@ -21,7 +21,6 @@ class Span end class HTTPTransport remove_method :traces_endpoint - remove_method :services_endpoint - attr_accessor :traces_endpoint, :services_endpoint, :encoder, :headers + attr_accessor :traces_endpoint, :encoder, :headers end end diff --git a/test/contrib/rails/rails_sidekiq_test.rb b/test/contrib/rails/rails_sidekiq_test.rb index 61ecec57bf..a437a16ec0 100644 --- a/test/contrib/rails/rails_sidekiq_test.rb +++ b/test/contrib/rails/rails_sidekiq_test.rb @@ -35,33 +35,4 @@ class EmptyWorker def perform; end end - - test 'Sidekiq middleware uses Rails configuration if available' do - @tracer.configure(enabled: false, debug: true, host: 'tracer.example.com', port: 7777) - Datadog::Contrib::Rails::Framework.setup - db_adapter = get_adapter_name - - # add Sidekiq middleware - Sidekiq::Testing.server_middleware do |chain| - chain.add(Datadog::Contrib::Sidekiq::ServerTracer, tracer: @tracer, service_name: 'rails-sidekiq') - end - - # do something to force middleware execution - EmptyWorker.perform_async - assert_equal( - @tracer.services, - app_name => { - 'app' => 'rails', 'app_type' => 'web' - }, - "#{app_name}-#{db_adapter}" => { - 'app' => 'active_record', 'app_type' => 'db' - }, - "#{app_name}-cache" => { - 'app' => 'rails', 'app_type' => 'cache' - }, - 'rails-sidekiq' => { - 'app' => 'sidekiq', 'app_type' => 'worker' - } - ) - end end diff --git a/test/contrib/rails/tracer_test.rb b/test/contrib/rails/tracer_test.rb index b5ba8737d9..ce5ef0b4c2 100644 --- a/test/contrib/rails/tracer_test.rb +++ b/test/contrib/rails/tracer_test.rb @@ -25,89 +25,9 @@ class TracerTest < ActionDispatch::IntegrationTest assert Datadog.configuration[:rails][:tracer] end - test 'a default service and database should be properly set' do - services = Datadog.configuration[:rails][:tracer].services + test 'a default database should be properly set' do Datadog::Contrib::Rails::Framework.setup adapter_name = get_adapter_name refute_equal(adapter_name, 'defaultdb') - assert_equal( - { - app_name => { - 'app' => 'rails', 'app_type' => 'web' - }, - "#{app_name}-#{adapter_name}" => { - 'app' => 'active_record', 'app_type' => 'db' - }, - "#{app_name}-cache" => { - 'app' => 'rails', 'app_type' => 'cache' - } - }, - services - ) - end - - test 'database service can be changed by user' do - update_config(:database_service, 'customer-db') - tracer = Datadog.configuration[:rails][:tracer] - - assert_equal( - { - app_name => { - 'app' => 'rails', 'app_type' => 'web' - }, - 'customer-db' => { - 'app' => 'active_record', 'app_type' => 'db' - }, - "#{app_name}-cache" => { - 'app' => 'rails', 'app_type' => 'cache' - } - }, - tracer.services - ) - end - - test 'application service can be changed by user' do - tracer = Datadog.configuration[:rails][:tracer] - update_config(:controller_service, 'my-custom-app') - adapter_name = get_adapter_name() - - assert_equal( - { - app_name => { - 'app' => 'rack', 'app_type' => 'web' - }, - 'my-custom-app' => { - 'app' => 'rails', 'app_type' => 'web' - }, - "#{app_name}-#{adapter_name}" => { - 'app' => 'active_record', 'app_type' => 'db' - }, - "#{app_name}-cache" => { - 'app' => 'rails', 'app_type' => 'cache' - } - }, - tracer.services - ) - end - - test 'cache service can be changed by user' do - update_config(:cache_service, 'service-cache') - tracer = Datadog.configuration[:rails][:tracer] - adapter_name = get_adapter_name() - - assert_equal( - { - app_name => { - 'app' => 'rails', 'app_type' => 'web' - }, - "#{app_name}-#{adapter_name}" => { - 'app' => 'active_record', 'app_type' => 'db' - }, - 'service-cache' => { - 'app' => 'rails', 'app_type' => 'cache' - } - }, - tracer.services - ) end end diff --git a/test/contrib/sidekiq/server_tracer_test.rb b/test/contrib/sidekiq/server_tracer_test.rb index 1801eb7cfa..70dc3f561c 100644 --- a/test/contrib/sidekiq/server_tracer_test.rb +++ b/test/contrib/sidekiq/server_tracer_test.rb @@ -42,9 +42,6 @@ def test_empty spans = @writer.spans() assert_equal(1, spans.length) - services = @writer.services() - assert_equal(1, services.length) - span = spans[0] assert_equal('sidekiq', span.service) assert_equal('ServerTracerTest::EmptyWorker', span.resource) @@ -64,9 +61,6 @@ def test_error spans = @writer.spans() assert_equal(1, spans.length) - services = @writer.services() - assert_equal(1, services.length) - span = spans[0] assert_equal('sidekiq', span.service) assert_equal('ServerTracerTest::ErrorWorker', span.resource) @@ -85,9 +79,6 @@ def test_custom spans = @writer.spans() assert_equal(2, spans.length) - services = @writer.services() - assert_equal(2, services.length) - custom, empty = spans assert_equal('sidekiq', empty.service) diff --git a/test/contrib/sidekiq/tracer_configure_test.rb b/test/contrib/sidekiq/tracer_configure_test.rb index 2b72c62cf6..ef9155abfb 100644 --- a/test/contrib/sidekiq/tracer_configure_test.rb +++ b/test/contrib/sidekiq/tracer_configure_test.rb @@ -13,13 +13,6 @@ def test_configuration_defaults chain.add(Datadog::Contrib::Sidekiq::ServerTracer, tracer: @tracer) end EmptyWorker.perform_async() - - assert_equal( - @writer.services, - 'sidekiq' => { - 'app' => 'sidekiq', 'app_type' => 'worker' - } - ) end def test_configuration_custom @@ -33,12 +26,5 @@ def test_configuration_custom ) end EmptyWorker.perform_async() - - assert_equal( - @tracer.services, - 'my-sidekiq' => { - 'app' => 'sidekiq', 'app_type' => 'worker' - } - ) end end diff --git a/test/helper.rb b/test/helper.rb index 937432b9e3..ea05aad200 100644 --- a/test/helper.rb +++ b/test/helper.rb @@ -121,14 +121,12 @@ def initialize(options = {}) # easy access to registered components @spans = [] - @services = {} end - def write(trace, services) + def write(trace) @mutex.synchronize do - super(trace, services) + super(trace) @spans << trace - @services = @services.merge(services) unless services.empty? end end @@ -165,14 +163,6 @@ def trace0_spans spans end end - - def services - @mutex.synchronize do - services = @services - @services = {} - services - end - end end # FauxTransport is a dummy HTTPTransport that doesn't send data to an agent. @@ -196,8 +186,6 @@ def initialize(hostname, port) def send(endpoint, data) data = case endpoint - when :services - @helper_encoder.encode_services(data) when :traces @helper_encoder.encode_traces(data) end @@ -234,8 +222,7 @@ def helper_dump module Datadog class HTTPTransport remove_method :traces_endpoint - remove_method :services_endpoint - attr_accessor :traces_endpoint, :services_endpoint, :encoder, :headers + attr_accessor :traces_endpoint, :encoder, :headers end end diff --git a/test/sync_writer_test.rb b/test/sync_writer_test.rb index 585dc0ac1b..adaf86e09d 100644 --- a/test/sync_writer_test.rb +++ b/test/sync_writer_test.rb @@ -12,11 +12,9 @@ def setup def test_sync_write trace = get_test_traces(1).first - services = get_test_services - @sync_writer.write(trace, services) + @sync_writer.write(trace) assert_includes(@transport.calls, [:traces, [trace]]) - assert_includes(@transport.calls, [:services, services]) end def test_sync_write_filtering @@ -27,8 +25,8 @@ def test_sync_write_filtering Pipeline::SpanFilter.new { |span| span.name == 'span_1' } ) - @sync_writer.write(trace1, {}) - @sync_writer.write(trace2, {}) + @sync_writer.write(trace1) + @sync_writer.write(trace2) refute_includes(@transport.calls, [:traces, [trace1]]) assert_includes(@transport.calls, [:traces, [trace2]]) diff --git a/test/tracer_test.rb b/test/tracer_test.rb index c5d3de70c2..3575a149b4 100644 --- a/test/tracer_test.rb +++ b/test/tracer_test.rb @@ -120,12 +120,6 @@ def test_trace_no_block_not_finished assert_nil(span.end_time) end - def test_set_service_info - tracer = get_test_tracer - tracer.set_service_info('rest-api', 'rails', 'web') - assert_equal(tracer.services['rest-api'], 'app' => 'rails', 'app_type' => 'web') - end - def test_set_tags tracer = get_test_tracer tracer.set_tags('env' => 'test', 'component' => 'core')