Merge pull request #975 from ericmustin/fix_presto_tests_and_integration

Fix presto tests, add to CI, update integration details
DataDog · Mar 17, 2020 · 5eec549 · 5eec549
2 parents 1d31eaa + 505a290
commit 5eec549
Show file tree

Hide file tree

Showing 6 changed files with 54 additions and 9 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -15,6 +15,8 @@ test_containers:
       - POSTGRES_PASSWORD=postgres
       - POSTGRES_USER=postgres
       - POSTGRES_DB=postgres
+  - &container_presto
+    image: prestosql/presto
   - &container_mysql
     image: mysql:5.6
     environment:
@@ -164,6 +166,7 @@ orbs:
               - BUNDLE_GEMFILE: /app/Gemfile
               - TEST_DATADOG_INTEGRATION: 1
           - *container_postgres
+          - *container_presto
           - *container_mysql
           - *container_elasticsearch
           - *container_redis

diff --git a/Appraisals b/Appraisals
@@ -180,6 +180,7 @@ elsif Gem::Version.new('2.1.0') <= Gem::Version.new(RUBY_VERSION) \
       gem 'delayed_job'
       gem 'delayed_job_active_record'
       gem 'elasticsearch-transport'
+      gem 'presto-client', '>=  0.5.14'      
       gem 'ethon'
       gem 'excon'
       gem 'hiredis'
@@ -335,6 +336,7 @@ elsif Gem::Version.new('2.2.0') <= Gem::Version.new(RUBY_VERSION) \
       gem 'delayed_job'
       gem 'delayed_job_active_record'
       gem 'elasticsearch-transport'
+      gem 'presto-client', '>=  0.5.14'
       gem 'ethon'
       gem 'excon'
       gem 'faraday'
@@ -580,6 +582,7 @@ elsif Gem::Version.new('2.4.0') <= Gem::Version.new(RUBY_VERSION) \
       gem 'hiredis'
       gem 'mongo', '>= 2.8.0'
       gem 'mysql2', '< 0.5', platform: :ruby
+      gem 'presto-client', '>=  0.5.14'
       gem 'racecar', '>= 0.3.5'
       gem 'rack'
       gem 'rack-test'
@@ -691,6 +694,7 @@ elsif Gem::Version.new('2.5.0') <= Gem::Version.new(RUBY_VERSION) \
       gem 'hiredis'
       gem 'mongo', '>= 2.8.0'
       gem 'mysql2', '< 0.5', platform: :ruby
+      gem 'presto-client', '>=  0.5.14'      
       gem 'racecar', '>= 0.3.5'
       gem 'rack'
       gem 'rack-test'

diff --git a/Rakefile b/Rakefile
@@ -241,6 +241,7 @@ task :ci do
       sh 'bundle exec appraisal contrib-old rake spec:http'
       sh 'bundle exec appraisal contrib-old rake spec:mongodb'
       sh 'bundle exec appraisal contrib-old rake spec:mysql2'
+      sh 'bundle exec appraisal contrib-old rake spec:presto'
       sh 'bundle exec appraisal contrib-old rake spec:rack'
       sh 'bundle exec appraisal contrib-old rake spec:rake'
       sh 'bundle exec appraisal contrib-old rake spec:redis'
@@ -303,6 +304,7 @@ task :ci do
       sh 'bundle exec appraisal contrib rake spec:http'
       sh 'bundle exec appraisal contrib rake spec:mongodb'
       sh 'bundle exec appraisal contrib rake spec:mysql2'
+      sh 'bundle exec appraisal contrib rake spec:presto'
       sh 'bundle exec appraisal contrib rake spec:racecar'
       sh 'bundle exec appraisal contrib rake spec:rack'
       sh 'bundle exec appraisal contrib rake spec:rake'
@@ -371,6 +373,7 @@ task :ci do
       sh 'bundle exec appraisal contrib rake spec:http'
       sh 'bundle exec appraisal contrib rake spec:mongodb'
       sh 'bundle exec appraisal contrib rake spec:mysql2'
+      sh 'bundle exec appraisal contrib rake spec:presto'
       sh 'bundle exec appraisal contrib rake spec:racecar'
       sh 'bundle exec appraisal contrib rake spec:rack'
       sh 'bundle exec appraisal contrib rake spec:rake'
@@ -443,6 +446,7 @@ task :ci do
       sh 'bundle exec appraisal contrib rake spec:http'
       sh 'bundle exec appraisal contrib rake spec:mongodb'
       sh 'bundle exec appraisal contrib rake spec:mysql2'
+      sh 'bundle exec appraisal contrib rake spec:presto'
       sh 'bundle exec appraisal contrib rake spec:racecar'
       sh 'bundle exec appraisal contrib rake spec:rack'
       sh 'bundle exec appraisal contrib rake spec:rake'
@@ -500,6 +504,7 @@ task :ci do
       sh 'bundle exec appraisal contrib rake spec:http'
       sh 'bundle exec appraisal contrib rake spec:mongodb'
       sh 'bundle exec appraisal contrib rake spec:mysql2'
+      sh 'bundle exec appraisal contrib rake spec:presto'
       sh 'bundle exec appraisal contrib rake spec:racecar'
       sh 'bundle exec appraisal contrib rake spec:rack'
       sh 'bundle exec appraisal contrib rake spec:rake'
@@ -567,6 +572,7 @@ task :ci do
       sh 'bundle exec appraisal contrib rake spec:http'
       sh 'bundle exec appraisal contrib rake spec:mongodb'
       sh 'bundle exec appraisal contrib rake spec:mysql2'
+      sh 'bundle exec appraisal contrib rake spec:presto'
       sh 'bundle exec appraisal contrib rake spec:racecar'
       sh 'bundle exec appraisal contrib rake spec:rack'
       sh 'bundle exec appraisal contrib rake spec:rake'
@@ -633,6 +639,7 @@ task :ci do
       sh 'bundle exec appraisal contrib rake spec:http'
       sh 'bundle exec appraisal contrib rake spec:mongodb'
       sh 'bundle exec appraisal contrib rake spec:mysql2'
+      sh 'bundle exec appraisal contrib rake spec:presto'
       sh 'bundle exec appraisal contrib rake spec:racecar'
       sh 'bundle exec appraisal contrib rake spec:rack'
       sh 'bundle exec appraisal contrib rake spec:rake'

diff --git a/docker-compose.yml b/docker-compose.yml
@@ -38,6 +38,7 @@ services:
       - mongodb
       - mysql
       - postgres
+      - presto
       - redis
     env_file: ./.env
     environment:
@@ -49,6 +50,8 @@ services:
       - TEST_MONGODB_HOST=mongodb
       - TEST_MYSQL_HOST=mysql
       - TEST_POSTGRES_HOST=postgres
+      - TEST_PRESTO_HOST=presto
+      - TEST_PRESTO_PORT=8080      
       - TEST_REDIS_HOST=redis
     stdin_open: true
     tty: true
@@ -66,6 +69,7 @@ services:
       - mongodb
       - mysql
       - postgres
+      - presto
       - redis
     env_file: ./.env
     environment:
@@ -77,6 +81,8 @@ services:
       - TEST_MONGODB_HOST=mongodb
       - TEST_MYSQL_HOST=mysql
       - TEST_POSTGRES_HOST=postgres
+      - TEST_PRESTO_HOST=presto
+      - TEST_PRESTO_PORT=8080      
       - TEST_REDIS_HOST=redis
     stdin_open: true
     tty: true
@@ -94,6 +100,7 @@ services:
       - mongodb
       - mysql
       - postgres
+      - presto
       - redis
     env_file: ./.env
     environment:
@@ -105,6 +112,8 @@ services:
       - TEST_MONGODB_HOST=mongodb
       - TEST_MYSQL_HOST=mysql
       - TEST_POSTGRES_HOST=postgres
+      - TEST_PRESTO_HOST=presto
+      - TEST_PRESTO_PORT=8080      
       - TEST_REDIS_HOST=redis
     stdin_open: true
     tty: true
@@ -122,6 +131,7 @@ services:
       - mongodb
       - mysql
       - postgres
+      - presto
       - redis
     env_file: ./.env
     environment:
@@ -133,6 +143,8 @@ services:
       - TEST_MONGODB_HOST=mongodb
       - TEST_MYSQL_HOST=mysql
       - TEST_POSTGRES_HOST=postgres
+      - TEST_PRESTO_HOST=presto
+      - TEST_PRESTO_PORT=8080      
       - TEST_REDIS_HOST=redis
     stdin_open: true
     tty: true
@@ -181,6 +193,7 @@ services:
       - mongodb
       - mysql
       - postgres
+      - presto
       - redis
     env_file: ./.env
     environment:
@@ -192,6 +205,8 @@ services:
       - TEST_MONGODB_HOST=mongodb
       - TEST_MYSQL_HOST=mysql
       - TEST_POSTGRES_HOST=postgres
+      - TEST_PRESTO_HOST=presto
+      - TEST_PRESTO_PORT=8080      
       - TEST_REDIS_HOST=redis
     stdin_open: true
     tty: true
@@ -209,6 +224,7 @@ services:
       - mongodb
       - mysql
       - postgres
+      - presto
       - redis
     env_file: ./.env
     environment:
@@ -220,6 +236,8 @@ services:
       - TEST_MONGODB_HOST=mongodb
       - TEST_MYSQL_HOST=mysql
       - TEST_POSTGRES_HOST=postgres
+      - TEST_PRESTO_HOST=presto
+      - TEST_PRESTO_PORT=8080      
       - TEST_REDIS_HOST=redis
     stdin_open: true
     tty: true

diff --git a/lib/ddtrace/contrib/presto/instrumentation.rb b/lib/ddtrace/contrib/presto/instrumentation.rb
@@ -25,9 +25,9 @@ def run(query)
                   span.set_tag(Ext::TAG_QUERY_ASYNC, false)
                 rescue StandardError => e
                   Datadog::Logger.log.debug("error preparing span for presto: #{e}")
-                ensure
-                  super(query)
                 end
+
+                super(query)
               end
             end
 
@@ -40,9 +40,9 @@ def query(query, &blk)
                   span.set_tag(Ext::TAG_QUERY_ASYNC, !blk.nil?)
                 rescue StandardError => e
                   Datadog::Logger.log.debug("error preparing span for presto: #{e}")
-                ensure
-                  super(query, &blk)
                 end
+
+                super(query, &blk)
               end
             end
 
@@ -56,9 +56,9 @@ def kill(query_id)
                   span.set_tag(Ext::TAG_QUERY_ID, query_id)
                 rescue StandardError => e
                   Datadog::Logger.log.debug("error preparing span for presto: #{e}")
-                ensure
-                  super(query_id)
                 end
+
+                super(query_id)
               end
             end
 
@@ -91,8 +91,8 @@ def decorate!(span)
               set_nilable_tag!(span, :model_version, Ext::TAG_MODEL_VERSION)
 
               # Set analytics sample rate
-              if Contrib::Analytics.enabled?(configuration[:analytics_enabled])
-                Contrib::Analytics.set_sample_rate(span, configuration[:analytics_sample_rate])
+              if Contrib::Analytics.enabled?(datadog_configuration[:analytics_enabled])
+                Contrib::Analytics.set_sample_rate(span, datadog_configuration[:analytics_sample_rate])
               end
             end
 

diff --git a/spec/ddtrace/contrib/presto/client_spec.rb b/spec/ddtrace/contrib/presto/client_spec.rb
@@ -1,6 +1,7 @@
 require 'spec_helper'
 require 'ddtrace'
 require 'presto-client'
+require 'ddtrace/contrib/analytics_examples'
 
 RSpec.describe 'Presto::Client instrumentation' do
   let(:tracer) { get_test_tracer }
@@ -193,12 +194,20 @@ def suppress_warnings
       end
     end
 
+    shared_examples_for 'a sampled trace' do
+      it_behaves_like 'analytics for integration' do
+        let(:analytics_enabled_var) { Datadog::Contrib::Presto::Ext::ENV_ANALYTICS_ENABLED }
+        let(:analytics_sample_rate_var) { Datadog::Contrib::Presto::Ext::ENV_ANALYTICS_SAMPLE_RATE }
+      end
+    end
+
     describe '#run operation' do
       before(:each) { client.run('SELECT 1') }
 
       it_behaves_like 'a Presto trace'
       it_behaves_like 'a configurable Presto trace'
       it_behaves_like 'a synchronous query trace'
+      it_behaves_like 'a sampled trace'
 
       it 'has a query resource'  do
         expect(span.resource).to eq('SELECT 1')
@@ -230,7 +239,7 @@ def suppress_warnings
         it 'is an error' do
           expect(span).to have_error
           expect(span).to have_error_type('Presto::Client::PrestoQueryError')
-          expect(span).to have_error_message("Column 'banana' cannot be resolved")
+          expect(span).to have_error_message(/Column 'banana' cannot be resolved/)
         end
       end
     end
@@ -253,6 +262,7 @@ def suppress_warnings
         it_behaves_like 'a configurable Presto trace'
         it_behaves_like 'a query trace'
         it_behaves_like 'a synchronous query trace'
+        it_behaves_like 'a sampled trace'
       end
 
       context 'given a block parameter' do
@@ -262,6 +272,7 @@ def suppress_warnings
         it_behaves_like 'a configurable Presto trace'
         it_behaves_like 'a query trace'
         it_behaves_like 'an asynchronous query trace'
+        it_behaves_like 'a sampled trace'
       end
     end
 
@@ -272,6 +283,7 @@ def suppress_warnings
 
       it_behaves_like 'a Presto trace'
       it_behaves_like 'a configurable Presto trace'
+      it_behaves_like 'a sampled trace'
 
       it 'has a kill resource' do
         expect(span.resource).to eq(Datadog::Contrib::Presto::Ext::SPAN_KILL)
@@ -291,6 +303,7 @@ def suppress_warnings
 
       it_behaves_like 'a Presto trace'
       it_behaves_like 'a configurable Presto trace'
+      it_behaves_like 'a sampled trace'
 
       it 'has a query resource' do
         expect(span.resource).to eq('SELECT 1')