From 079d98d2bb3837f8d4978ac516b0ab238c4da31b Mon Sep 17 00:00:00 2001 From: Don Morrison Date: Sat, 9 Nov 2019 14:53:25 -0800 Subject: [PATCH 01/32] Add API OpenTelemetry::Adapter * for access to global config, e.g., OpenTelemetry::Adapter.tracer --- api/lib/opentelemetry.rb | 1 + api/lib/opentelemetry/adapter.rb | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 api/lib/opentelemetry/adapter.rb diff --git a/api/lib/opentelemetry.rb b/api/lib/opentelemetry.rb index 30162fdb62..3455cf4742 100644 --- a/api/lib/opentelemetry.rb +++ b/api/lib/opentelemetry.rb @@ -6,6 +6,7 @@ require 'logger' require 'opentelemetry/error' +require 'opentelemetry/adapter' require 'opentelemetry/context' require 'opentelemetry/distributed_context' require 'opentelemetry/internal' diff --git a/api/lib/opentelemetry/adapter.rb b/api/lib/opentelemetry/adapter.rb new file mode 100644 index 0000000000..cecd3ae294 --- /dev/null +++ b/api/lib/opentelemetry/adapter.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +# Copyright 2019 OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + # The basic interface for Adapter objects + class Adapter + class << self + attr_reader :config + + def install(config = {}) + @config = config + new.install + end + + def tracer + OpenTelemetry.tracer_factory.tracer(config[:name], config[:version]) + end + end + end +end From 5ac7d4edd9f45161512fe41859bdf3ce987d5a53 Mon Sep 17 00:00:00 2001 From: David Davis Date: Fri, 8 Nov 2019 09:55:27 -0800 Subject: [PATCH 02/32] Add faraday instrumentation adapter Gemfile/gemspec --- adapters/faraday/Gemfile | 11 +++++++ .../opentelemetry/adapters/faraday/version.rb | 13 ++++++++ .../opentelemetry-adapters-faraday.gemspec | 32 +++++++++++++++++++ 3 files changed, 56 insertions(+) create mode 100644 adapters/faraday/Gemfile create mode 100644 adapters/faraday/lib/opentelemetry/adapters/faraday/version.rb create mode 100644 adapters/faraday/opentelemetry-adapters-faraday.gemspec diff --git a/adapters/faraday/Gemfile b/adapters/faraday/Gemfile new file mode 100644 index 0000000000..bea08dcb0c --- /dev/null +++ b/adapters/faraday/Gemfile @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +# Copyright 2019 OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +source 'https://rubygems.org' + +gemspec + +gem 'opentelemetry-api', path: '../../api' diff --git a/adapters/faraday/lib/opentelemetry/adapters/faraday/version.rb b/adapters/faraday/lib/opentelemetry/adapters/faraday/version.rb new file mode 100644 index 0000000000..f7ea89e404 --- /dev/null +++ b/adapters/faraday/lib/opentelemetry/adapters/faraday/version.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +# Copyright 2019 OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module Adapters + module Faraday + VERSION = '0.0.0' + end + end +end diff --git a/adapters/faraday/opentelemetry-adapters-faraday.gemspec b/adapters/faraday/opentelemetry-adapters-faraday.gemspec new file mode 100644 index 0000000000..7673cf1459 --- /dev/null +++ b/adapters/faraday/opentelemetry-adapters-faraday.gemspec @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +# Copyright 2019 OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +lib = File.expand_path('lib', __dir__) +$LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) +require 'opentelemetry/adapters/faraday/version' + +Gem::Specification.new do |spec| + spec.name = 'opentelemetry-adapters-faraday' + spec.version = OpenTelemetry::Adapters::Faraday::VERSION + spec.authors = ['OpenTelemetry Authors'] + spec.email = ['cncf-opentelemetry-contributors@lists.cncf.io'] + + spec.summary = 'Faraday instrumentation adapter for the OpenTelemetry framework' + spec.description = 'Faraday instrumentation adapter for the OpenTelemetry framework' + spec.homepage = 'https://github.com/open-telemetry/opentelemetry-ruby' + spec.license = 'Apache-2.0' + + spec.files = ::Dir.glob('lib/**/*.rb') + + ::Dir.glob('*.md') + + ['LICENSE'] + spec.require_paths = ['lib'] + spec.required_ruby_version = '>= 2.4.0' + + spec.add_dependency 'opentelemetry-api', '~> 0.0' + spec.add_dependency 'faraday', '~> 0.17.0' + + spec.add_development_dependency 'bundler', '>= 1.17' +end From 3f7f6b080e72b812b1f61e0241ba0bb8d9db1d18 Mon Sep 17 00:00:00 2001 From: David Davis Date: Mon, 11 Nov 2019 11:33:52 -0800 Subject: [PATCH 03/32] Add faraday instrumentation adapter --- .../faraday/lib/opentelemetry/adapters.rb | 12 ++++++ .../lib/opentelemetry/adapters/faraday.rb | 21 +++++++++ .../opentelemetry/adapters/faraday/adapter.rb | 33 ++++++++++++++ .../faraday/middlewares/tracer_middleware.rb | 43 +++++++++++++++++++ .../adapters/faraday/patches/rack_builder.rb | 26 +++++++++++ 5 files changed, 135 insertions(+) create mode 100644 adapters/faraday/lib/opentelemetry/adapters.rb create mode 100644 adapters/faraday/lib/opentelemetry/adapters/faraday.rb create mode 100644 adapters/faraday/lib/opentelemetry/adapters/faraday/adapter.rb create mode 100644 adapters/faraday/lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware.rb create mode 100644 adapters/faraday/lib/opentelemetry/adapters/faraday/patches/rack_builder.rb diff --git a/adapters/faraday/lib/opentelemetry/adapters.rb b/adapters/faraday/lib/opentelemetry/adapters.rb new file mode 100644 index 0000000000..e5956b543c --- /dev/null +++ b/adapters/faraday/lib/opentelemetry/adapters.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +# Copyright 2019 OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + # "Instrumentation adapters" are specified by + # https://github.com/open-telemetry/opentelemetry-specification/blob/57714f7547fe4dcb342ad0ad10a80d86118431c7/specification/overview.md#instrumentation-adapters + module Adapters + end +end diff --git a/adapters/faraday/lib/opentelemetry/adapters/faraday.rb b/adapters/faraday/lib/opentelemetry/adapters/faraday.rb new file mode 100644 index 0000000000..80695f4ac4 --- /dev/null +++ b/adapters/faraday/lib/opentelemetry/adapters/faraday.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +# Copyright 2019 OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module Adapters + module Faraday + module_function + + TRACER_NAME = 'faraday' + TRACER_VERSION = Gem.loaded_specs[TRACER_NAME].version.to_s + + def install(config = {name: TRACER_NAME, version: TRACER_VERSION}) + require_relative 'faraday/adapter' + Faraday::Adapter.install(config) + end + end + end +end diff --git a/adapters/faraday/lib/opentelemetry/adapters/faraday/adapter.rb b/adapters/faraday/lib/opentelemetry/adapters/faraday/adapter.rb new file mode 100644 index 0000000000..5625af4aa0 --- /dev/null +++ b/adapters/faraday/lib/opentelemetry/adapters/faraday/adapter.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +# Copyright 2019 OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +require_relative 'middlewares/tracer_middleware' +require_relative 'patches/rack_builder' + +module OpenTelemetry + module Adapters + module Faraday + class Adapter < OpenTelemetry::Adapter + def install + register_tracer_middleware + use_middleware_by_default + end + + private + + def register_tracer_middleware + ::Faraday::Middleware.register_middleware( + open_telemetry: Middlewares::TracerMiddleware + ) + end + + def use_middleware_by_default + ::Faraday::RackBuilder.prepend(Patches::RackBuilder) + end + end + end + end +end diff --git a/adapters/faraday/lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware.rb b/adapters/faraday/lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware.rb new file mode 100644 index 0000000000..b038c2c474 --- /dev/null +++ b/adapters/faraday/lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +# Copyright 2019 OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module Adapters + module Faraday + module Middlewares + class TracerMiddleware < ::Faraday::Middleware + def call(env) + tracer.in_span(env.url.to_s, + attributes: { 'component' => 'http', + 'http.method' => env.method }, + kind: :client) do |span| + trace_request(span, env) + + app.call(env).on_complete { |resp| trace_response(span, resp) } + end + end + + private + + attr_reader :app + + def tracer + Faraday::Adapter.tracer + end + + def trace_request(span, env) + span.set_attribute('http.url', env.url.to_s) + end + + def trace_response(span, response) + span.set_attribute('http.status_code', response.status) + span.set_attribute('http.status_text', response.reason_phrase) + end + end + end + end + end +end diff --git a/adapters/faraday/lib/opentelemetry/adapters/faraday/patches/rack_builder.rb b/adapters/faraday/lib/opentelemetry/adapters/faraday/patches/rack_builder.rb new file mode 100644 index 0000000000..1292ec99a8 --- /dev/null +++ b/adapters/faraday/lib/opentelemetry/adapters/faraday/patches/rack_builder.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +# Copyright 2019 OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 +require_relative '../middlewares/tracer_middleware' + +module OpenTelemetry + module Adapters + module Faraday + module Patches + # Module to be prepended to force Faraday to use the middleware by + # default so the user doesn't have to call `use` for every connection. + module RackBuilder + def adapter(*args) + use(:open_telemetry) unless @handlers.any? do |handler| + handler.klass == Middlewares::TracerMiddleware + end + + super + end + end + end + end + end +end From ed31c858322779c9fb892f2a931b457457ffeab9 Mon Sep 17 00:00:00 2001 From: David Davis Date: Fri, 8 Nov 2019 11:21:23 -0800 Subject: [PATCH 04/32] Add faraday example with docker-compose service * e.g., $ docker-compose run ex-adapter-faraday $ ruby faraday.rb Expected: console output of SpanData --- adapters/faraday/example/Gemfile | 7 +++++++ adapters/faraday/example/faraday.rb | 21 +++++++++++++++++++++ docker-compose.yml | 4 ++++ 3 files changed, 32 insertions(+) create mode 100644 adapters/faraday/example/Gemfile create mode 100644 adapters/faraday/example/faraday.rb diff --git a/adapters/faraday/example/Gemfile b/adapters/faraday/example/Gemfile new file mode 100644 index 0000000000..1c162edde6 --- /dev/null +++ b/adapters/faraday/example/Gemfile @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +source 'https://rubygems.org' + +gem 'faraday' +gem 'opentelemetry-api', path: '../../../api' +gem 'opentelemetry-sdk', path: '../../../sdk' diff --git a/adapters/faraday/example/faraday.rb b/adapters/faraday/example/faraday.rb new file mode 100644 index 0000000000..024e195096 --- /dev/null +++ b/adapters/faraday/example/faraday.rb @@ -0,0 +1,21 @@ +require 'rubygems' +require 'bundler/setup' + +require 'faraday' +require 'opentelemetry/sdk' +require_relative '../lib/opentelemetry/adapters/faraday' + +# Set preferred tracer implementation: +SDK = OpenTelemetry::SDK + +factory = OpenTelemetry.tracer_factory = SDK::Trace::TracerFactory.new +factory.add_span_processor( + SDK::Trace::Export::SimpleSpanProcessor.new( + SDK::Trace::Export::ConsoleSpanExporter.new + ) +) + +OpenTelemetry::Adapters::Faraday.install(name: 'faraday-example', version: '1.0') + +conn = Faraday.new('http://example.com') +conn.get '/' diff --git a/docker-compose.yml b/docker-compose.yml index 8a48f1a0d0..e926ecfa64 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -24,6 +24,10 @@ services: context: . working_dir: /app + ex-adapter-faraday: + <<: *base + working_dir: /app/adapters/faraday/example + sdk: <<: *base working_dir: /app/sdk From 22ccf4fef264c0208b6b016d6f65888f93bfb21d Mon Sep 17 00:00:00 2001 From: David Davis Date: Tue, 12 Nov 2019 14:00:13 -0800 Subject: [PATCH 05/32] Avoid hard-dependency on faraday * want to be able to ship more instrumentation than a user needs, let the auto-installer determine what is applicable --- adapters/faraday/opentelemetry-adapters-faraday.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/adapters/faraday/opentelemetry-adapters-faraday.gemspec b/adapters/faraday/opentelemetry-adapters-faraday.gemspec index 7673cf1459..eee39b4193 100644 --- a/adapters/faraday/opentelemetry-adapters-faraday.gemspec +++ b/adapters/faraday/opentelemetry-adapters-faraday.gemspec @@ -26,7 +26,7 @@ Gem::Specification.new do |spec| spec.required_ruby_version = '>= 2.4.0' spec.add_dependency 'opentelemetry-api', '~> 0.0' - spec.add_dependency 'faraday', '~> 0.17.0' spec.add_development_dependency 'bundler', '>= 1.17' + spec.add_development_dependency 'faraday', '~> 0.17.0' end From 7562579c07058a02468a51f499bb5d546deb1deb Mon Sep 17 00:00:00 2001 From: David Davis Date: Wed, 13 Nov 2019 13:15:44 -0800 Subject: [PATCH 06/32] Fix undefined method `version' for nil:NilClass * when 'faraday' isn't loaded/required already e.g., irb> require 'opentelemetry/adapters/faraday' irb> OpenTelemetry::Adapters::Faraday.install --- adapters/faraday/lib/opentelemetry/adapters/faraday.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/adapters/faraday/lib/opentelemetry/adapters/faraday.rb b/adapters/faraday/lib/opentelemetry/adapters/faraday.rb index 80695f4ac4..fe361bcd4d 100644 --- a/adapters/faraday/lib/opentelemetry/adapters/faraday.rb +++ b/adapters/faraday/lib/opentelemetry/adapters/faraday.rb @@ -10,7 +10,7 @@ module Faraday module_function TRACER_NAME = 'faraday' - TRACER_VERSION = Gem.loaded_specs[TRACER_NAME].version.to_s + TRACER_VERSION = Gem.loaded_specs[TRACER_NAME]&.version.to_s def install(config = {name: TRACER_NAME, version: TRACER_VERSION}) require_relative 'faraday/adapter' From 698735129e60db47c32cb53ac0e4b7459b114f1f Mon Sep 17 00:00:00 2001 From: David Davis Date: Tue, 12 Nov 2019 14:06:57 -0800 Subject: [PATCH 07/32] Doc: reminder to handle common use case --- adapters/faraday/lib/opentelemetry/adapters.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/adapters/faraday/lib/opentelemetry/adapters.rb b/adapters/faraday/lib/opentelemetry/adapters.rb index e5956b543c..55295042cf 100644 --- a/adapters/faraday/lib/opentelemetry/adapters.rb +++ b/adapters/faraday/lib/opentelemetry/adapters.rb @@ -7,6 +7,8 @@ module OpenTelemetry # "Instrumentation adapters" are specified by # https://github.com/open-telemetry/opentelemetry-specification/blob/57714f7547fe4dcb342ad0ad10a80d86118431c7/specification/overview.md#instrumentation-adapters + # + # Adapters should be able to handle the case when the library is not installed on a user's system. module Adapters end end From 92ba1bc7bd7279e860150a01fb33a0327988b20f Mon Sep 17 00:00:00 2001 From: David Davis Date: Tue, 12 Nov 2019 17:49:42 -0800 Subject: [PATCH 08/32] Add explicit 'require' statements in faraday/adapter * will only be called after Adapters::Faraday.install * reduces burden on caller --- .../faraday/lib/opentelemetry/adapters/faraday/adapter.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/adapters/faraday/lib/opentelemetry/adapters/faraday/adapter.rb b/adapters/faraday/lib/opentelemetry/adapters/faraday/adapter.rb index 5625af4aa0..3cb4f7abc1 100644 --- a/adapters/faraday/lib/opentelemetry/adapters/faraday/adapter.rb +++ b/adapters/faraday/lib/opentelemetry/adapters/faraday/adapter.rb @@ -4,6 +4,11 @@ # # SPDX-License-Identifier: Apache-2.0 +# TODO: General Question: should adapters explicitly load the libraries they depend on? +# Or, should adapters assume that the libraries are already loaded by the caller/user? +require 'faraday' +require 'opentelemetry/adapter' + require_relative 'middlewares/tracer_middleware' require_relative 'patches/rack_builder' From 60d41d6edc3e6a0b34707b9a90260128e5dfdad7 Mon Sep 17 00:00:00 2001 From: David Davis Date: Tue, 12 Nov 2019 18:09:13 -0800 Subject: [PATCH 09/32] Use in_span(attributes:) instead of set_attribute * for performance, since span.set_attribute involves a mutex, etc. --- .../adapters/faraday/middlewares/tracer_middleware.rb | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/adapters/faraday/lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware.rb b/adapters/faraday/lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware.rb index b038c2c474..e252ab988d 100644 --- a/adapters/faraday/lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware.rb +++ b/adapters/faraday/lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware.rb @@ -12,10 +12,9 @@ class TracerMiddleware < ::Faraday::Middleware def call(env) tracer.in_span(env.url.to_s, attributes: { 'component' => 'http', - 'http.method' => env.method }, + 'http.method' => env.method, + 'http.url' => env.url.to_s }, kind: :client) do |span| - trace_request(span, env) - app.call(env).on_complete { |resp| trace_response(span, resp) } end end @@ -28,10 +27,6 @@ def tracer Faraday::Adapter.tracer end - def trace_request(span, env) - span.set_attribute('http.url', env.url.to_s) - end - def trace_response(span, response) span.set_attribute('http.status_code', response.status) span.set_attribute('http.status_text', response.reason_phrase) From 33f28c01901b90d68d5a74c176660445be71925f Mon Sep 17 00:00:00 2001 From: David Davis Date: Mon, 11 Nov 2019 11:19:06 -0800 Subject: [PATCH 10/32] Change SDK::Trace::TracerFactory to subclass API's TracerFactory * to allow access to #http_text_format --- sdk/lib/opentelemetry/sdk/trace/tracer_factory.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/lib/opentelemetry/sdk/trace/tracer_factory.rb b/sdk/lib/opentelemetry/sdk/trace/tracer_factory.rb index 5f17334bf3..8a021c82b4 100644 --- a/sdk/lib/opentelemetry/sdk/trace/tracer_factory.rb +++ b/sdk/lib/opentelemetry/sdk/trace/tracer_factory.rb @@ -8,7 +8,7 @@ module OpenTelemetry module SDK module Trace # {TracerFactory} is the SDK implementation of {OpenTelemetry::Trace::TracerFactory}. - class TracerFactory + class TracerFactory < OpenTelemetry::Trace::TracerFactory Key = Struct.new(:name, :version) private_constant(:Key) From 937cf6ca2e9ccfc329049146e075844fcf94fb99 Mon Sep 17 00:00:00 2001 From: David Davis Date: Wed, 13 Nov 2019 13:21:32 -0800 Subject: [PATCH 11/32] Add context propagation --- .../adapters/faraday/middlewares/tracer_middleware.rb | 11 +++++++++++ api/lib/opentelemetry/adapter.rb | 4 ++++ 2 files changed, 15 insertions(+) diff --git a/adapters/faraday/lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware.rb b/adapters/faraday/lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware.rb index e252ab988d..6500736a82 100644 --- a/adapters/faraday/lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware.rb +++ b/adapters/faraday/lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware.rb @@ -15,6 +15,8 @@ def call(env) 'http.method' => env.method, 'http.url' => env.url.to_s }, kind: :client) do |span| + propagate_context(span, env) + app.call(env).on_complete { |resp| trace_response(span, resp) } end end @@ -23,6 +25,15 @@ def call(env) attr_reader :app + def propagate_context(span, env) + formatter.extract(env) + formatter.inject(span.context, env.request_headers) + end + + def formatter + Faraday::Adapter.http_formatter + end + def tracer Faraday::Adapter.tracer end diff --git a/api/lib/opentelemetry/adapter.rb b/api/lib/opentelemetry/adapter.rb index cecd3ae294..e56df8613c 100644 --- a/api/lib/opentelemetry/adapter.rb +++ b/api/lib/opentelemetry/adapter.rb @@ -15,6 +15,10 @@ def install(config = {}) new.install end + def http_formatter + OpenTelemetry.tracer_factory.http_text_format + end + def tracer OpenTelemetry.tracer_factory.tracer(config[:name], config[:version]) end From d42721b4f2be43c3727cb51adf1c9368fcc9e4e8 Mon Sep 17 00:00:00 2001 From: David Davis Date: Wed, 13 Nov 2019 14:24:47 -0800 Subject: [PATCH 12/32] Fix occasional test failure in sdk batch_span_processor_test * RE: "failures on heavily loaded CI server" -- increase delta experimentally, by 10x --- .../sdk/trace/export/batch_span_processor_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/test/opentelemetry/sdk/trace/export/batch_span_processor_test.rb b/sdk/test/opentelemetry/sdk/trace/export/batch_span_processor_test.rb index bb1fa6cc13..40200d4696 100644 --- a/sdk/test/opentelemetry/sdk/trace/export/batch_span_processor_test.rb +++ b/sdk/test/opentelemetry/sdk/trace/export/batch_span_processor_test.rb @@ -221,7 +221,7 @@ def to_span_data end describe 'normally' do - let(:exporter_sleeps_for_millis) { exporter_timeout_millis - 1 } + let(:exporter_sleeps_for_millis) { exporter_timeout_millis - 10 } it 'exporter is not interrupted' do _(exporter.state).must_equal(:not_interrupted) @@ -229,7 +229,7 @@ def to_span_data end describe 'when exporter runs too long' do - let(:exporter_sleeps_for_millis) { exporter_timeout_millis + 1 } + let(:exporter_sleeps_for_millis) { exporter_timeout_millis + 10 } it 'is interrupted by a timeout' do _(exporter.state).must_equal(:called) From 814e3307097c02f5841d01178cd40d0195cd6546 Mon Sep 17 00:00:00 2001 From: David Davis Date: Thu, 14 Nov 2019 09:37:58 -0800 Subject: [PATCH 13/32] Don't extract context, just inject hints: * for outbound requests: inject current span context into request headers * for inbound requests: extract remote span context from request headers, start a new span with parent context (using the extracted context) --- .../adapters/faraday/middlewares/tracer_middleware.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/adapters/faraday/lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware.rb b/adapters/faraday/lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware.rb index 6500736a82..33405567fd 100644 --- a/adapters/faraday/lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware.rb +++ b/adapters/faraday/lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware.rb @@ -25,8 +25,8 @@ def call(env) attr_reader :app + # Outbound requests should only need to inject the current span. def propagate_context(span, env) - formatter.extract(env) formatter.inject(span.context, env.request_headers) end From 4ee20b1b9baef7e21e1d7b26cc17c4bda7bf0c4a Mon Sep 17 00:00:00 2001 From: David Davis Date: Thu, 14 Nov 2019 10:11:58 -0800 Subject: [PATCH 14/32] Load tracer_version lazily * TRACER_VERSION can get locked to empty string if faraday isn't required yet --- adapters/faraday/lib/opentelemetry/adapters/faraday.rb | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/adapters/faraday/lib/opentelemetry/adapters/faraday.rb b/adapters/faraday/lib/opentelemetry/adapters/faraday.rb index fe361bcd4d..86baed9558 100644 --- a/adapters/faraday/lib/opentelemetry/adapters/faraday.rb +++ b/adapters/faraday/lib/opentelemetry/adapters/faraday.rb @@ -10,12 +10,17 @@ module Faraday module_function TRACER_NAME = 'faraday' - TRACER_VERSION = Gem.loaded_specs[TRACER_NAME]&.version.to_s - def install(config = {name: TRACER_NAME, version: TRACER_VERSION}) + def install(config = {name: TRACER_NAME, + version: tracer_version}) require_relative 'faraday/adapter' Faraday::Adapter.install(config) end + + def tracer_version + Gem.loaded_specs[TRACER_NAME]&.version.to_s + end + private_class_method(:tracer_version) end end end From 915b0520e74a90bb102241df152f986f1a2fa094 Mon Sep 17 00:00:00 2001 From: David Davis Date: Thu, 14 Nov 2019 10:35:10 -0800 Subject: [PATCH 15/32] Allow auto-installer to inject a tracer * allow (auto) installer to bind tracer to instrumentation * config[:tracer] will override default_tracer --- api/lib/opentelemetry/adapter.rb | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/api/lib/opentelemetry/adapter.rb b/api/lib/opentelemetry/adapter.rb index e56df8613c..90c02ce613 100644 --- a/api/lib/opentelemetry/adapter.rb +++ b/api/lib/opentelemetry/adapter.rb @@ -4,22 +4,30 @@ # # SPDX-License-Identifier: Apache-2.0 +require 'opentelemetry' + module OpenTelemetry # The basic interface for Adapter objects class Adapter class << self - attr_reader :config + attr_reader :config, + :http_formatter, + :tracer def install(config = {}) @config = config + @http_formatter = config[:http_formatter] || default_http_formatter + @tracer = config[:tracer] || default_tracer new.install end - def http_formatter + private + + def default_http_formatter OpenTelemetry.tracer_factory.http_text_format end - def tracer + def default_tracer OpenTelemetry.tracer_factory.tracer(config[:name], config[:version]) end end From 89ba2cb3292eaaebf3437bf5cbea319131b5f191 Mon Sep 17 00:00:00 2001 From: David Davis Date: Tue, 19 Nov 2019 17:31:07 -0800 Subject: [PATCH 16/32] Fix circular loading issue --- api/lib/opentelemetry/adapter.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/api/lib/opentelemetry/adapter.rb b/api/lib/opentelemetry/adapter.rb index 90c02ce613..d4a0c8ec4c 100644 --- a/api/lib/opentelemetry/adapter.rb +++ b/api/lib/opentelemetry/adapter.rb @@ -4,8 +4,6 @@ # # SPDX-License-Identifier: Apache-2.0 -require 'opentelemetry' - module OpenTelemetry # The basic interface for Adapter objects class Adapter From a45dac29a19d8b3e40a2be1b895b1257835f6b5d Mon Sep 17 00:00:00 2001 From: David Davis Date: Sat, 23 Nov 2019 15:55:16 -0800 Subject: [PATCH 17/32] Avoid requiring changes to API (OpenTelemetry::Adapter) * avoid making API changes now by inlining required methods (#propagator) --- .../opentelemetry/adapters/faraday/adapter.rb | 27 ++++++++++++--- .../faraday/middlewares/tracer_middleware.rb | 6 ++-- api/lib/opentelemetry.rb | 1 - api/lib/opentelemetry/adapter.rb | 33 ------------------- 4 files changed, 26 insertions(+), 41 deletions(-) delete mode 100644 api/lib/opentelemetry/adapter.rb diff --git a/adapters/faraday/lib/opentelemetry/adapters/faraday/adapter.rb b/adapters/faraday/lib/opentelemetry/adapters/faraday/adapter.rb index 3cb4f7abc1..7fe860138e 100644 --- a/adapters/faraday/lib/opentelemetry/adapters/faraday/adapter.rb +++ b/adapters/faraday/lib/opentelemetry/adapters/faraday/adapter.rb @@ -4,10 +4,8 @@ # # SPDX-License-Identifier: Apache-2.0 -# TODO: General Question: should adapters explicitly load the libraries they depend on? -# Or, should adapters assume that the libraries are already loaded by the caller/user? require 'faraday' -require 'opentelemetry/adapter' +require 'opentelemetry' require_relative 'middlewares/tracer_middleware' require_relative 'patches/rack_builder' @@ -15,7 +13,28 @@ module OpenTelemetry module Adapters module Faraday - class Adapter < OpenTelemetry::Adapter + class Adapter + class << self + attr_reader :config, + :propagator, + :tracer + + def install(config = {}) + @config = config + @propagator = OpenTelemetry.tracer_factory.rack_http_text_format + @tracer = config[:tracer] || default_tracer + + new.install + end + + private + + def default_tracer + OpenTelemetry.tracer_factory.tracer(config[:name], + config[:version]) + end + end + def install register_tracer_middleware use_middleware_by_default diff --git a/adapters/faraday/lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware.rb b/adapters/faraday/lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware.rb index 33405567fd..3c1551d09f 100644 --- a/adapters/faraday/lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware.rb +++ b/adapters/faraday/lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware.rb @@ -27,11 +27,11 @@ def call(env) # Outbound requests should only need to inject the current span. def propagate_context(span, env) - formatter.inject(span.context, env.request_headers) + propagator.inject(span.context, env.request_headers) end - def formatter - Faraday::Adapter.http_formatter + def propagator + Faraday::Adapter.propagator end def tracer diff --git a/api/lib/opentelemetry.rb b/api/lib/opentelemetry.rb index 3455cf4742..30162fdb62 100644 --- a/api/lib/opentelemetry.rb +++ b/api/lib/opentelemetry.rb @@ -6,7 +6,6 @@ require 'logger' require 'opentelemetry/error' -require 'opentelemetry/adapter' require 'opentelemetry/context' require 'opentelemetry/distributed_context' require 'opentelemetry/internal' diff --git a/api/lib/opentelemetry/adapter.rb b/api/lib/opentelemetry/adapter.rb deleted file mode 100644 index d4a0c8ec4c..0000000000 --- a/api/lib/opentelemetry/adapter.rb +++ /dev/null @@ -1,33 +0,0 @@ -# frozen_string_literal: true - -# Copyright 2019 OpenTelemetry Authors -# -# SPDX-License-Identifier: Apache-2.0 - -module OpenTelemetry - # The basic interface for Adapter objects - class Adapter - class << self - attr_reader :config, - :http_formatter, - :tracer - - def install(config = {}) - @config = config - @http_formatter = config[:http_formatter] || default_http_formatter - @tracer = config[:tracer] || default_tracer - new.install - end - - private - - def default_http_formatter - OpenTelemetry.tracer_factory.http_text_format - end - - def default_tracer - OpenTelemetry.tracer_factory.tracer(config[:name], config[:version]) - end - end - end -end From a558f7195f146652154ac423eb03b7dd22417acc Mon Sep 17 00:00:00 2001 From: Don Morrison Date: Wed, 4 Dec 2019 13:57:46 -0800 Subject: [PATCH 18/32] sig-feedback: remove name and version from config Removes the name and version from the config as the spec defines these as coming from the instrumentation/adapter and not from the instrumented library. Adds convenience methods for the name and version and provide those on the `Faraday` adapter class. Updates the tracer configuration to use the new name and version. Updates the example to remove the name and version from the call. --- adapters/faraday/example/faraday.rb | 2 +- .../lib/opentelemetry/adapters/faraday.rb | 18 +++++++++++------- .../opentelemetry/adapters/faraday/adapter.rb | 14 ++++++-------- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/adapters/faraday/example/faraday.rb b/adapters/faraday/example/faraday.rb index 024e195096..c66e96eaeb 100644 --- a/adapters/faraday/example/faraday.rb +++ b/adapters/faraday/example/faraday.rb @@ -15,7 +15,7 @@ ) ) -OpenTelemetry::Adapters::Faraday.install(name: 'faraday-example', version: '1.0') +OpenTelemetry::Adapters::Faraday.install conn = Faraday.new('http://example.com') conn.get '/' diff --git a/adapters/faraday/lib/opentelemetry/adapters/faraday.rb b/adapters/faraday/lib/opentelemetry/adapters/faraday.rb index 86baed9558..3790170895 100644 --- a/adapters/faraday/lib/opentelemetry/adapters/faraday.rb +++ b/adapters/faraday/lib/opentelemetry/adapters/faraday.rb @@ -9,18 +9,22 @@ module Adapters module Faraday module_function - TRACER_NAME = 'faraday' - - def install(config = {name: TRACER_NAME, - version: tracer_version}) + def install(config = {}) require_relative 'faraday/adapter' Faraday::Adapter.install(config) end - def tracer_version - Gem.loaded_specs[TRACER_NAME]&.version.to_s + # Convenience method to access the nested module name + def name + Module.nesting[0].to_s + end + + # Convenience method to access the adapter version + def version + VERSION end - private_class_method(:tracer_version) end end end + +require_relative './faraday/version' diff --git a/adapters/faraday/lib/opentelemetry/adapters/faraday/adapter.rb b/adapters/faraday/lib/opentelemetry/adapters/faraday/adapter.rb index 7fe860138e..2f4e9ad713 100644 --- a/adapters/faraday/lib/opentelemetry/adapters/faraday/adapter.rb +++ b/adapters/faraday/lib/opentelemetry/adapters/faraday/adapter.rb @@ -16,22 +16,20 @@ module Faraday class Adapter class << self attr_reader :config, - :propagator, - :tracer + :propagator def install(config = {}) @config = config @propagator = OpenTelemetry.tracer_factory.rack_http_text_format - @tracer = config[:tracer] || default_tracer new.install end - private - - def default_tracer - OpenTelemetry.tracer_factory.tracer(config[:name], - config[:version]) + def tracer + @tracer ||= OpenTelemetry.tracer_factory.tracer( + Faraday.name, + Faraday.version + ) end end From 00ad878595f15e4ba00bf28cffbb7ec2ce3a79a6 Mon Sep 17 00:00:00 2001 From: Don Morrison Date: Wed, 4 Dec 2019 13:59:08 -0800 Subject: [PATCH 19/32] sig-feedback: Use the http text format for propagation Update the `propagator` to use the plain http text format rather than the rack version. --- adapters/faraday/lib/opentelemetry/adapters/faraday/adapter.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/adapters/faraday/lib/opentelemetry/adapters/faraday/adapter.rb b/adapters/faraday/lib/opentelemetry/adapters/faraday/adapter.rb index 2f4e9ad713..873c60c42e 100644 --- a/adapters/faraday/lib/opentelemetry/adapters/faraday/adapter.rb +++ b/adapters/faraday/lib/opentelemetry/adapters/faraday/adapter.rb @@ -20,7 +20,7 @@ class << self def install(config = {}) @config = config - @propagator = OpenTelemetry.tracer_factory.rack_http_text_format + @propagator = OpenTelemetry.tracer_factory.http_text_format new.install end From 1658d2e37f54ab1060acb17ff43b08b604ac57bd Mon Sep 17 00:00:00 2001 From: David Davis Date: Tue, 19 Nov 2019 17:30:22 -0800 Subject: [PATCH 20/32] Tests: Change Gemfile, gemspec, add Rakefile * Gemfile: opentelemetry-sdk is only required for tests --- adapters/faraday/Gemfile | 4 ++++ adapters/faraday/Rakefile | 14 ++++++++++++++ .../faraday/opentelemetry-adapters-faraday.gemspec | 5 +++++ 3 files changed, 23 insertions(+) create mode 100644 adapters/faraday/Rakefile diff --git a/adapters/faraday/Gemfile b/adapters/faraday/Gemfile index bea08dcb0c..a022b20230 100644 --- a/adapters/faraday/Gemfile +++ b/adapters/faraday/Gemfile @@ -9,3 +9,7 @@ source 'https://rubygems.org' gemspec gem 'opentelemetry-api', path: '../../api' + +group :test do + gem 'opentelemetry-sdk', path: '../../sdk' +end diff --git a/adapters/faraday/Rakefile b/adapters/faraday/Rakefile new file mode 100644 index 0000000000..4919bebd54 --- /dev/null +++ b/adapters/faraday/Rakefile @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +# Copyright 2019 OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +require 'bundler/gem_tasks' +require 'rake/testtask' + +Rake::TestTask.new :test do |t| + t.libs << 'test' + t.libs << 'lib' + t.test_files = FileList['test/**/*_test.rb'] +end \ No newline at end of file diff --git a/adapters/faraday/opentelemetry-adapters-faraday.gemspec b/adapters/faraday/opentelemetry-adapters-faraday.gemspec index eee39b4193..df4a735e24 100644 --- a/adapters/faraday/opentelemetry-adapters-faraday.gemspec +++ b/adapters/faraday/opentelemetry-adapters-faraday.gemspec @@ -27,6 +27,11 @@ Gem::Specification.new do |spec| spec.add_dependency 'opentelemetry-api', '~> 0.0' + spec.add_development_dependency 'appraisal', '~> 2.2.0' spec.add_development_dependency 'bundler', '>= 1.17' spec.add_development_dependency 'faraday', '~> 0.17.0' + spec.add_development_dependency 'minitest', '~> 5.0' + spec.add_development_dependency 'opentelemetry-sdk', '~> 0.0' + spec.add_development_dependency 'simplecov', '~> 0.17.1' + spec.add_development_dependency 'webmock', '~> 3.7.6' end From 21c8ada3a713c93675a509ed2ca512708dd43dc4 Mon Sep 17 00:00:00 2001 From: David Davis Date: Tue, 19 Nov 2019 17:30:48 -0800 Subject: [PATCH 21/32] Add test_helper and first test * note: adapter.install effects aren't localized, so test order matters Update tests to avoid passing name, version to .install * name and version are now determined internally and shouldn't usually be passed in explicitly --- .../opentelemetry/adapters/faraday_test.rb | 35 +++++++++++++++++++ adapters/faraday/test/test_helper.rb | 22 ++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 adapters/faraday/test/opentelemetry/adapters/faraday_test.rb create mode 100644 adapters/faraday/test/test_helper.rb diff --git a/adapters/faraday/test/opentelemetry/adapters/faraday_test.rb b/adapters/faraday/test/opentelemetry/adapters/faraday_test.rb new file mode 100644 index 0000000000..099621b30d --- /dev/null +++ b/adapters/faraday/test/opentelemetry/adapters/faraday_test.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +# Copyright 2019 OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +require 'test_helper' + +require_relative '../../../lib/opentelemetry/adapters/faraday' + +describe OpenTelemetry::Adapters::Faraday do + let(:adapter) { OpenTelemetry::Adapters::Faraday } + let(:exporter) { EXPORTER } + + before do + adapter.install + exporter.reset + end + + describe 'tracing' do + before do + stub_request(:any, 'example.com') + end + + it 'before request' do + _(exporter.finished_spans.size).must_equal 0 + end + + it 'after request' do + ::Faraday.new('http://example.com').get('/') + + _(exporter.finished_spans.size).must_equal 1 + end + end +end diff --git a/adapters/faraday/test/test_helper.rb b/adapters/faraday/test/test_helper.rb new file mode 100644 index 0000000000..347cc5d908 --- /dev/null +++ b/adapters/faraday/test/test_helper.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +# Copyright 2019 OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +require 'faraday' + +require 'opentelemetry/sdk' + +require 'minitest/autorun' +require 'webmock/minitest' + +# global opentelemetry-sdk setup: +sdk = OpenTelemetry::SDK +exporter = sdk::Trace::Export::InMemorySpanExporter.new +span_processor = sdk::Trace::Export::SimpleSpanProcessor.new(exporter) +OpenTelemetry.tracer_factory = sdk::Trace::TracerFactory.new.tap do |factory| + factory.add_span_processor(span_processor) +end + +EXPORTER = exporter \ No newline at end of file From eb243a98d149aa55be8dd915622a6d7dce280bf4 Mon Sep 17 00:00:00 2001 From: David Davis Date: Tue, 19 Nov 2019 17:41:36 -0800 Subject: [PATCH 22/32] Add appraisals for running tests against multiple versions example: $ bundle exec appraisal install $ bundle exec appraisal rake test --- .gitignore | 3 +++ adapters/faraday/Appraisals | 11 +++++++++++ adapters/faraday/gemfiles/faraday_0.13.gemfile | 12 ++++++++++++ adapters/faraday/gemfiles/faraday_0.16.gemfile | 12 ++++++++++++ adapters/faraday/gemfiles/faraday_0.17.gemfile | 12 ++++++++++++ 5 files changed, 50 insertions(+) create mode 100644 adapters/faraday/Appraisals create mode 100644 adapters/faraday/gemfiles/faraday_0.13.gemfile create mode 100644 adapters/faraday/gemfiles/faraday_0.16.gemfile create mode 100644 adapters/faraday/gemfiles/faraday_0.17.gemfile diff --git a/.gitignore b/.gitignore index ec2888c245..d1f826b1b0 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,6 @@ **/coverage **/*.gem **/.bundle + +# Appraisals +adapters/**/*.gemfile.lock diff --git a/adapters/faraday/Appraisals b/adapters/faraday/Appraisals new file mode 100644 index 0000000000..845f66b1e6 --- /dev/null +++ b/adapters/faraday/Appraisals @@ -0,0 +1,11 @@ +appraise "faraday-0.17" do + gem "faraday", "0.17.0" +end + +appraise "faraday-0.16" do + gem "faraday", "0.16.2" +end + +appraise "faraday-0.13" do + gem "faraday", "0.13.1" +end diff --git a/adapters/faraday/gemfiles/faraday_0.13.gemfile b/adapters/faraday/gemfiles/faraday_0.13.gemfile new file mode 100644 index 0000000000..58b4980336 --- /dev/null +++ b/adapters/faraday/gemfiles/faraday_0.13.gemfile @@ -0,0 +1,12 @@ +# This file was generated by Appraisal + +source "https://rubygems.org" + +gem "opentelemetry-api", path: "../../../api" +gem "faraday", "0.13.1" + +group :test do + gem "opentelemetry-sdk", path: "../../../sdk" +end + +gemspec path: "../" diff --git a/adapters/faraday/gemfiles/faraday_0.16.gemfile b/adapters/faraday/gemfiles/faraday_0.16.gemfile new file mode 100644 index 0000000000..d9dd2b2548 --- /dev/null +++ b/adapters/faraday/gemfiles/faraday_0.16.gemfile @@ -0,0 +1,12 @@ +# This file was generated by Appraisal + +source "https://rubygems.org" + +gem "opentelemetry-api", path: "../../../api" +gem "faraday", "0.16.2" + +group :test do + gem "opentelemetry-sdk", path: "../../../sdk" +end + +gemspec path: "../" diff --git a/adapters/faraday/gemfiles/faraday_0.17.gemfile b/adapters/faraday/gemfiles/faraday_0.17.gemfile new file mode 100644 index 0000000000..650108af52 --- /dev/null +++ b/adapters/faraday/gemfiles/faraday_0.17.gemfile @@ -0,0 +1,12 @@ +# This file was generated by Appraisal + +source "https://rubygems.org" + +gem "opentelemetry-api", path: "../../../api" +gem "faraday", "0.17.0" + +group :test do + gem "opentelemetry-sdk", path: "../../../sdk" +end + +gemspec path: "../" From cfcdf5f6aca8c8392ea697653ea8bd4bcfdc7dad Mon Sep 17 00:00:00 2001 From: David Davis Date: Thu, 21 Nov 2019 10:28:48 -0800 Subject: [PATCH 23/32] Add Circle CI config --- .circleci/config.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index e20f49bf9a..8d12d84857 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -40,6 +40,13 @@ commands: - run: name: CI (Jaeger) command: "cd exporters/jaeger && bundle exec rake" + - run: + name: Bundle + CI (Adapters - Faraday) + command: | + cd adapters/faraday + gem install --no-document bundler && bundle install --jobs=3 --retry=3 + bundle exec appraisal install + bundle exec appraisal rake test rake-release: steps: - checkout From dd680bb88f3c3c8670e7561ccd631e5410d023ea Mon Sep 17 00:00:00 2001 From: David Davis Date: Mon, 25 Nov 2019 16:24:05 -0800 Subject: [PATCH 24/32] Add TracerMiddleware test --- .../middlewares/tracer_middleware_test.rb | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 adapters/faraday/test/opentelemetry/adapters/faraday/middlewares/tracer_middleware_test.rb diff --git a/adapters/faraday/test/opentelemetry/adapters/faraday/middlewares/tracer_middleware_test.rb b/adapters/faraday/test/opentelemetry/adapters/faraday/middlewares/tracer_middleware_test.rb new file mode 100644 index 0000000000..016e79acfb --- /dev/null +++ b/adapters/faraday/test/opentelemetry/adapters/faraday/middlewares/tracer_middleware_test.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +# Copyright 2019 OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +require 'test_helper' + +# require Adapter so .install method is found: +require_relative '../../../../../lib/opentelemetry/adapters/faraday/adapter' +require_relative '../../../../../lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware' + +describe OpenTelemetry::Adapters::Faraday::Middlewares::TracerMiddleware do + let(:adapter) { OpenTelemetry::Adapters::Faraday } + let(:exporter) { EXPORTER } + + let(:client) do + ::Faraday.new('http://example.com') do |builder| + builder.adapter(:test) do |stub| + stub.get('/success') { |_| [200, {}, 'OK'] } + stub.get('/failure') { |_| [500, {}, 'OK'] } + stub.get('/not_found') { |_| [404, {}, 'OK'] } + end + end + end + + before do + adapter.install + exporter.reset + end + + describe 'first span' do + let(:span) { exporter.finished_spans.first } + + it 'has http 200 attributes' do + client.get('/success') + + _(span.attributes['component']).must_equal 'http' + _(span.attributes['http.method']).must_equal :get + _(span.attributes['http.status_code']).must_equal 200 + _(span.attributes['http.url']).must_equal 'http://example.com/success' + end + + it 'has http.status_code 404' do + client.get('/not_found') + + _(span.attributes['component']).must_equal 'http' + _(span.attributes['http.method']).must_equal :get + _(span.attributes['http.status_code']).must_equal 404 + _(span.attributes['http.url']).must_equal 'http://example.com/not_found' + end + end +end From 5808423c6c01c00bafdb8c3a9caa0f841f753ad8 Mon Sep 17 00:00:00 2001 From: David Davis Date: Mon, 25 Nov 2019 16:42:38 -0800 Subject: [PATCH 25/32] Add Adapter test --- .../adapters/faraday/adapter_test.rb | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 adapters/faraday/test/opentelemetry/adapters/faraday/adapter_test.rb diff --git a/adapters/faraday/test/opentelemetry/adapters/faraday/adapter_test.rb b/adapters/faraday/test/opentelemetry/adapters/faraday/adapter_test.rb new file mode 100644 index 0000000000..62b3159d46 --- /dev/null +++ b/adapters/faraday/test/opentelemetry/adapters/faraday/adapter_test.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +# Copyright 2019 OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +require 'test_helper' + +require_relative '../../../../lib/opentelemetry/adapters/faraday/adapter' + +describe OpenTelemetry::Adapters::Faraday::Adapter do + let(:adapter) { OpenTelemetry::Adapters::Faraday::Adapter } + + before do + adapter.install + end + + describe 'defaults' do + it 'has propagator' do + _(adapter.propagator).wont_be_nil + end + + it 'has tracer' do + _(adapter.tracer).wont_be_nil + end + end +end From 29346a004b52890bc0edb2e9d6556b7eb10cd381 Mon Sep 17 00:00:00 2001 From: David Davis Date: Wed, 11 Dec 2019 17:41:53 -0800 Subject: [PATCH 26/32] Add/refine tests --- .../faraday/middlewares/tracer_middleware_test.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/adapters/faraday/test/opentelemetry/adapters/faraday/middlewares/tracer_middleware_test.rb b/adapters/faraday/test/opentelemetry/adapters/faraday/middlewares/tracer_middleware_test.rb index 016e79acfb..71ce35512f 100644 --- a/adapters/faraday/test/opentelemetry/adapters/faraday/middlewares/tracer_middleware_test.rb +++ b/adapters/faraday/test/opentelemetry/adapters/faraday/middlewares/tracer_middleware_test.rb @@ -7,7 +7,7 @@ require 'test_helper' # require Adapter so .install method is found: -require_relative '../../../../../lib/opentelemetry/adapters/faraday/adapter' +require_relative '../../../../../lib/opentelemetry/adapters/faraday' require_relative '../../../../../lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware' describe OpenTelemetry::Adapters::Faraday::Middlewares::TracerMiddleware do @@ -49,5 +49,14 @@ _(span.attributes['http.status_code']).must_equal 404 _(span.attributes['http.url']).must_equal 'http://example.com/not_found' end + + it 'has http.status_code 500' do + client.get('/failure') + + _(span.attributes['component']).must_equal 'http' + _(span.attributes['http.method']).must_equal :get + _(span.attributes['http.status_code']).must_equal 500 + _(span.attributes['http.url']).must_equal 'http://example.com/failure' + end end end From bc9e87916bbf8a9ac1ea017652fec8ccb7f0e2a4 Mon Sep 17 00:00:00 2001 From: David Davis Date: Thu, 19 Dec 2019 10:38:29 -0800 Subject: [PATCH 27/32] Allow for easier overriding of TracerMiddleware for per-connection rules * Use case: allow per-connection enabling/disabling of span reporting, possibly based on URL rules --- adapters/faraday/example/faraday.rb | 10 +++++++++ .../opentelemetry/adapters/faraday/adapter.rb | 4 +++- .../faraday/middlewares/tracer_middleware.rb | 8 +++++++ .../adapters/faraday/patches/rack_builder.rb | 2 +- .../middlewares/tracer_middleware_test.rb | 21 +++++++++++++++++-- 5 files changed, 41 insertions(+), 4 deletions(-) diff --git a/adapters/faraday/example/faraday.rb b/adapters/faraday/example/faraday.rb index c66e96eaeb..c278c65a51 100644 --- a/adapters/faraday/example/faraday.rb +++ b/adapters/faraday/example/faraday.rb @@ -15,6 +15,16 @@ ) ) +# Demonstrate disabling span reporting: +# +# require_relative '../lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware' +# class NoOp < OpenTelemetry::Adapters::Faraday::Middlewares::TracerMiddleware +# def disable_span_reporting?(env) +# env.url.to_s =~ /example.com/ +# end +# end +# OpenTelemetry::Adapters::Faraday.install(tracer_middleware: NoOp) + OpenTelemetry::Adapters::Faraday.install conn = Faraday.new('http://example.com') diff --git a/adapters/faraday/lib/opentelemetry/adapters/faraday/adapter.rb b/adapters/faraday/lib/opentelemetry/adapters/faraday/adapter.rb index 873c60c42e..5bd3dc2d16 100644 --- a/adapters/faraday/lib/opentelemetry/adapters/faraday/adapter.rb +++ b/adapters/faraday/lib/opentelemetry/adapters/faraday/adapter.rb @@ -20,6 +20,8 @@ class << self def install(config = {}) @config = config + # allow custom middleware to override default implementation: + @config[:tracer_middleware] ||= Middlewares::TracerMiddleware @propagator = OpenTelemetry.tracer_factory.http_text_format new.install @@ -42,7 +44,7 @@ def install def register_tracer_middleware ::Faraday::Middleware.register_middleware( - open_telemetry: Middlewares::TracerMiddleware + open_telemetry: self.class.config[:tracer_middleware] ) end diff --git a/adapters/faraday/lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware.rb b/adapters/faraday/lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware.rb index 3c1551d09f..e5499a2ccd 100644 --- a/adapters/faraday/lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware.rb +++ b/adapters/faraday/lib/opentelemetry/adapters/faraday/middlewares/tracer_middleware.rb @@ -10,6 +10,8 @@ module Faraday module Middlewares class TracerMiddleware < ::Faraday::Middleware def call(env) + return app.call(env) if disable_span_reporting?(env) + tracer.in_span(env.url.to_s, attributes: { 'component' => 'http', 'http.method' => env.method, @@ -21,6 +23,12 @@ def call(env) end end + # Override implementation (subclass) to determine per-connection + # span reporting rules. + def disable_span_reporting?(_env) + false + end + private attr_reader :app diff --git a/adapters/faraday/lib/opentelemetry/adapters/faraday/patches/rack_builder.rb b/adapters/faraday/lib/opentelemetry/adapters/faraday/patches/rack_builder.rb index 1292ec99a8..65ac6a0938 100644 --- a/adapters/faraday/lib/opentelemetry/adapters/faraday/patches/rack_builder.rb +++ b/adapters/faraday/lib/opentelemetry/adapters/faraday/patches/rack_builder.rb @@ -14,7 +14,7 @@ module Patches module RackBuilder def adapter(*args) use(:open_telemetry) unless @handlers.any? do |handler| - handler.klass == Middlewares::TracerMiddleware + handler.klass == Faraday::Adapter.config[:tracer_middleware] end super diff --git a/adapters/faraday/test/opentelemetry/adapters/faraday/middlewares/tracer_middleware_test.rb b/adapters/faraday/test/opentelemetry/adapters/faraday/middlewares/tracer_middleware_test.rb index 71ce35512f..4556e9d6b3 100644 --- a/adapters/faraday/test/opentelemetry/adapters/faraday/middlewares/tracer_middleware_test.rb +++ b/adapters/faraday/test/opentelemetry/adapters/faraday/middlewares/tracer_middleware_test.rb @@ -13,6 +13,7 @@ describe OpenTelemetry::Adapters::Faraday::Middlewares::TracerMiddleware do let(:adapter) { OpenTelemetry::Adapters::Faraday } let(:exporter) { EXPORTER } + let(:span) { exporter.finished_spans.first } let(:client) do ::Faraday.new('http://example.com') do |builder| @@ -30,8 +31,6 @@ end describe 'first span' do - let(:span) { exporter.finished_spans.first } - it 'has http 200 attributes' do client.get('/success') @@ -59,4 +58,22 @@ _(span.attributes['http.url']).must_equal 'http://example.com/failure' end end + + describe 'overriding span reporting' do + class NoReportMiddleware < OpenTelemetry::Adapters::Faraday::Middlewares::TracerMiddleware + def disable_span_reporting?(_env) + true + end + end + + before do + adapter.install(tracer_middleware: NoReportMiddleware) + end + + it 'does not report' do + client.get('/success') + + _(span).must_be_nil + end + end end From 73f608650954f2396d3a5f754c98cfc756e268c7 Mon Sep 17 00:00:00 2001 From: David Davis Date: Thu, 19 Dec 2019 16:11:17 -0800 Subject: [PATCH 28/32] Fix failing circleci builds * most likely failing due to bundler-2.1.x * see bundler issue #7489 --- .circleci/config.yml | 4 +++- adapters/faraday/opentelemetry-adapters-faraday.gemspec | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 8d12d84857..b5be588e0f 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -44,7 +44,9 @@ commands: name: Bundle + CI (Adapters - Faraday) command: | cd adapters/faraday - gem install --no-document bundler && bundle install --jobs=3 --retry=3 + gem uninstall -aIx bundler + gem install --no-document bundler -v '~> 2.0.2' + bundle install --jobs=3 --retry=3 bundle exec appraisal install bundle exec appraisal rake test rake-release: diff --git a/adapters/faraday/opentelemetry-adapters-faraday.gemspec b/adapters/faraday/opentelemetry-adapters-faraday.gemspec index df4a735e24..02f60aee63 100644 --- a/adapters/faraday/opentelemetry-adapters-faraday.gemspec +++ b/adapters/faraday/opentelemetry-adapters-faraday.gemspec @@ -28,7 +28,7 @@ Gem::Specification.new do |spec| spec.add_dependency 'opentelemetry-api', '~> 0.0' spec.add_development_dependency 'appraisal', '~> 2.2.0' - spec.add_development_dependency 'bundler', '>= 1.17' + spec.add_development_dependency 'bundler', '~> 2.0.2' spec.add_development_dependency 'faraday', '~> 0.17.0' spec.add_development_dependency 'minitest', '~> 5.0' spec.add_development_dependency 'opentelemetry-sdk', '~> 0.0' From fd66e7d921c5eb8b611def36ac41f817e77f0af3 Mon Sep 17 00:00:00 2001 From: David Davis Date: Mon, 30 Dec 2019 12:48:51 -0800 Subject: [PATCH 29/32] Fix failing circleci - jruby * circleci build is failing after jruby:9.2.9-jre * theory: jruby includes (vendored) its own bundler version --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index b5be588e0f..91d2cb543b 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -15,7 +15,7 @@ executors: - image: "circleci/ruby:2.6-stretch" jruby: docker: - - image: "circleci/jruby:latest" + - image: "circleci/jruby:9.2.8-jre" environment: JRUBY_OPTS: "--debug" commands: From 4098dd4a6430c2b46cc659091260ce3bbeb83655 Mon Sep 17 00:00:00 2001 From: David Davis Date: Mon, 6 Jan 2020 09:32:00 -0800 Subject: [PATCH 30/32] Appraisal: add faraday-1.0.x --- adapters/faraday/Appraisals | 4 ++++ adapters/faraday/gemfiles/faraday_1.0.gemfile | 12 ++++++++++++ 2 files changed, 16 insertions(+) create mode 100644 adapters/faraday/gemfiles/faraday_1.0.gemfile diff --git a/adapters/faraday/Appraisals b/adapters/faraday/Appraisals index 845f66b1e6..cfb912cb8b 100644 --- a/adapters/faraday/Appraisals +++ b/adapters/faraday/Appraisals @@ -1,3 +1,7 @@ +appraise "faraday-1.0" do + gem "faraday", "~> 1.0.0" +end + appraise "faraday-0.17" do gem "faraday", "0.17.0" end diff --git a/adapters/faraday/gemfiles/faraday_1.0.gemfile b/adapters/faraday/gemfiles/faraday_1.0.gemfile new file mode 100644 index 0000000000..cc19b3a9a0 --- /dev/null +++ b/adapters/faraday/gemfiles/faraday_1.0.gemfile @@ -0,0 +1,12 @@ +# This file was generated by Appraisal + +source "https://rubygems.org" + +gem "opentelemetry-api", path: "../../../api" +gem "faraday", "~> 1.0.0" + +group :test do + gem "opentelemetry-sdk", path: "../../../sdk" +end + +gemspec path: "../" From 36cfd8ac7bc264864f1053786ea2d188ad05ece1 Mon Sep 17 00:00:00 2001 From: David Davis Date: Thu, 9 Jan 2020 10:37:49 -0800 Subject: [PATCH 31/32] circleci: Remove appraisal support, use latest bundler * TODO: re-enable appraisal support once appraisal issue #162 is solved --- .circleci/config.yml | 8 +++----- adapters/faraday/opentelemetry-adapters-faraday.gemspec | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index c913180722..d695e1d1eb 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -18,7 +18,7 @@ executors: - image: "circleci/ruby:2.7-buster" jruby: docker: - - image: "circleci/jruby:9.2.8-jre" + - image: "circleci/jruby:latest" environment: JRUBY_OPTS: "--debug" commands: @@ -47,11 +47,9 @@ commands: name: Bundle + CI (Adapters - Faraday) command: | cd adapters/faraday - gem uninstall -aIx bundler - gem install --no-document bundler -v '~> 2.0.2' + gem install --no-document bundler bundle install --jobs=3 --retry=3 - bundle exec appraisal install - bundle exec appraisal rake test + bundle exec rake test rake-release: steps: - checkout diff --git a/adapters/faraday/opentelemetry-adapters-faraday.gemspec b/adapters/faraday/opentelemetry-adapters-faraday.gemspec index 02f60aee63..df4a735e24 100644 --- a/adapters/faraday/opentelemetry-adapters-faraday.gemspec +++ b/adapters/faraday/opentelemetry-adapters-faraday.gemspec @@ -28,7 +28,7 @@ Gem::Specification.new do |spec| spec.add_dependency 'opentelemetry-api', '~> 0.0' spec.add_development_dependency 'appraisal', '~> 2.2.0' - spec.add_development_dependency 'bundler', '~> 2.0.2' + spec.add_development_dependency 'bundler', '>= 1.17' spec.add_development_dependency 'faraday', '~> 0.17.0' spec.add_development_dependency 'minitest', '~> 5.0' spec.add_development_dependency 'opentelemetry-sdk', '~> 0.0' From dc05f823b741110e87d3e42ee619fc4e2dffa504 Mon Sep 17 00:00:00 2001 From: David Davis Date: Thu, 9 Jan 2020 13:49:20 -0800 Subject: [PATCH 32/32] circleci: Workaround failing appraisal+bundler(2.1.x)+ruby-2.7 issue * don't run bundle exec appraisal rake test under ruby-2.7 * NOTE: build was failing under jruby:latest (9.3.x) as well --- .circleci/config.yml | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index d695e1d1eb..f4beb9f732 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -21,6 +21,11 @@ executors: - image: "circleci/jruby:latest" environment: JRUBY_OPTS: "--debug" + jruby92: + docker: + - image: "circleci/jruby:9.2.8-jre" + environment: + JRUBY_OPTS: "--debug" commands: rake-test: steps: @@ -43,13 +48,17 @@ commands: - run: name: CI (Jaeger) command: "cd exporters/jaeger && bundle exec rake" + rake-test-appraisal: + steps: - run: name: Bundle + CI (Adapters - Faraday) command: | cd adapters/faraday - gem install --no-document bundler + gem uninstall -aIx bundler + gem install --no-document bundler -v '~> 2.0.2' bundle install --jobs=3 --retry=3 - bundle exec rake test + bundle exec appraisal install + bundle exec appraisal rake test rake-release: steps: - checkout @@ -64,14 +73,17 @@ jobs: executor: ruby24 steps: - rake-test + - rake-test-appraisal test-ruby25: executor: ruby25 steps: - rake-test + - rake-test-appraisal test-ruby26: executor: ruby26 steps: - rake-test + - rake-test-appraisal test-ruby27: executor: ruby27 steps: @@ -80,10 +92,16 @@ jobs: executor: jruby steps: - rake-test + test-jruby92: + executor: jruby92 + steps: + - rake-test + - rake-test-appraisal release: executor: ruby26 steps: - rake-release + - rake-test-appraisal workflows: version: 2 builds: @@ -108,6 +126,10 @@ workflows: filters: tags: only: /^opentelemetry-.+\/v\d.*$/ + - test-jruby92: + filters: + tags: + only: /^opentelemetry-.+\/v\d.*$/ - release: requires: - test-ruby24