Upgrade rubocop for a few gems (#1452)

plantfansam · fbogsany · web-flow · commit 4347e0f47bb0 · 2023-05-30T10:52:06.000-04:00
* upgrade rubocop

* 1.51

* upgrade rubocop

* upgrade rubocop on zipkin

* update rubocop on b3 propagator

* upgrade rubocop on jaeger

* assert_same instead of equal?

* don't use explicit blocks

* d'oh

---------

Co-authored-by: Francis Bogsanyi &lt;francis.bogsanyi@shopify.com&gt;
diff --git a/exporter/zipkin/.rubocop.yml b/exporter/zipkin/.rubocop.yml
@@ -1,3 +1,5 @@
+inherit_from: .rubocop_todo.yml
+
 AllCops:
   TargetRubyVersion: "2.6.0"
   Exclude:
@@ -15,13 +17,15 @@ Metrics/ModuleLength:
 Metrics/MethodLength:
   Max: 25
 Metrics/PerceivedComplexity:
-  Max: 11
+  Enabled: false
 Metrics/CyclomaticComplexity:
-  Max: 8
+  Enabled: false
 Metrics/ParameterLists:
   Enabled: false
 Naming/FileName:
   Exclude:
     - "lib/opentelemetry-exporter-zipkin.rb"
+Style/ExplicitBlockArgument:
+  Enabled: false
 Style/ModuleFunction:
   Enabled: false
diff --git a/exporter/zipkin/.rubocop_todo.yml b/exporter/zipkin/.rubocop_todo.yml
@@ -0,0 +1,38 @@
+# This configuration was generated by
+# `rubocop --auto-gen-config`
+# on 2023-05-22 22:55:26 UTC using RuboCop version 1.51.0.
+# The point is for the user to remove these configuration records
+# one by one as the offenses are removed from the code base.
+# Note that changes in the inspected code, or installation of new
+# versions of RuboCop, may require this file to be generated again.
+
+# Offense count: 4
+# Configuration parameters: AllowedMethods.
+# AllowedMethods: enums
+Lint/ConstantDefinitionInBlock:
+  Exclude:
+    - 'test/opentelemetry/exporters/zipkin/exporter_test.rb'
+    - 'test/opentelemetry/exporters/zipkin/transformer_test.rb'
+
+# Offense count: 1
+# Configuration parameters: AllowedMethods, AllowedPatterns.
+Metrics/CyclomaticComplexity:
+  Max: 9
+
+# Offense count: 2
+# Configuration parameters: AllowedMethods, AllowedPatterns.
+Metrics/PerceivedComplexity:
+  Max: 14
+
+# Offense count: 1
+# This cop supports unsafe autocorrection (--autocorrect-all).
+# Configuration parameters: Mode.
+Style/StringConcatenation:
+  Exclude:
+    - 'lib/opentelemetry/exporter/zipkin.rb'
+
+# Offense count: 1
+# This cop supports unsafe autocorrection (--autocorrect-all).
+Style/ZeroLengthPredicate:
+  Exclude:
+    - 'lib/opentelemetry/exporter/zipkin/transformer.rb'
diff --git a/exporter/zipkin/lib/opentelemetry/exporter/zipkin/exporter.rb b/exporter/zipkin/lib/opentelemetry/exporter/zipkin/exporter.rb
@@ -115,7 +115,7 @@ def valid_headers?(headers)
           false
         end
 
-        def send_spans(zipkin_spans, timeout: nil) # rubocop:disable Metrics/CyclomaticComplexity, Metrics/MethodLength
+        def send_spans(zipkin_spans, timeout: nil) # rubocop:disable Metrics/MethodLength
           retry_count = 0
           timeout ||= @timeout
           start_time = OpenTelemetry::Common::Utilities.timeout_timestamp
@@ -174,7 +174,7 @@ def handle_redirect(location)
           # TODO: figure out destination and reinitialize @http and @path
         end
 
-        def backoff?(retry_after: nil, retry_count:, reason:)
+        def backoff?(retry_count:, reason:, retry_after: nil)
           return false if retry_count > RETRY_COUNT
 
           # TODO: metric exporter
diff --git a/exporter/zipkin/lib/opentelemetry/exporter/zipkin/transformer.rb b/exporter/zipkin/lib/opentelemetry/exporter/zipkin/transformer.rb
@@ -91,7 +91,7 @@ def add_status_tags(span_data, tags)
           end
         end
 
-        def add_conditional_tags(zipkin_span, span_data, tags, service_name) # rubocop:disable Metrics/CyclomaticComplexity
+        def add_conditional_tags(zipkin_span, span_data, tags, service_name)
           dropped_attributes_count = span_data.total_recorded_attributes - span_data.attributes&.size.to_i
           dropped_events_count = span_data.total_recorded_events - span_data.events&.size.to_i
           dropped_links_count = span_data.total_recorded_links - span_data.links&.size.to_i
diff --git a/exporter/zipkin/opentelemetry-exporter-zipkin.gemspec b/exporter/zipkin/opentelemetry-exporter-zipkin.gemspec
@@ -36,7 +36,7 @@ Gem::Specification.new do |spec|
   spec.add_development_dependency 'minitest', '~> 5.0'
   spec.add_development_dependency 'opentelemetry-test-helpers'
   spec.add_development_dependency 'rake', '~> 12.0'
-  spec.add_development_dependency 'rubocop', '~> 0.73.0'
+  spec.add_development_dependency 'rubocop', '~> 1.51.0'
   spec.add_development_dependency 'simplecov', '~> 0.17'
   spec.add_development_dependency 'webmock', '~> 3.7.6'
   spec.add_development_dependency 'yard', '~> 0.9'
diff --git a/metrics_api/.rubocop.yml b/metrics_api/.rubocop.yml
@@ -1,3 +1,5 @@
+inherit_from: .rubocop_todo.yml
+
 AllCops:
   TargetRubyVersion: "2.6.0"
 
diff --git a/metrics_api/.rubocop_todo.yml b/metrics_api/.rubocop_todo.yml
@@ -0,0 +1,26 @@
+# This configuration was generated by
+# `rubocop --auto-gen-config`
+# on 2023-05-22 22:49:35 UTC using RuboCop version 1.49.0.
+# The point is for the user to remove these configuration records
+# one by one as the offenses are removed from the code base.
+# Note that changes in the inspected code, or installation of new
+# versions of RuboCop, may require this file to be generated again.
+
+# Offense count: 4
+# Configuration parameters: AllowedMethods.
+# AllowedMethods: enums
+Lint/ConstantDefinitionInBlock:
+  Exclude:
+    - 'test/opentelemetry/metrics/meter_test.rb'
+
+# Offense count: 1
+Lint/MissingSuper:
+  Exclude:
+    - 'lib/opentelemetry/internal/proxy_meter_provider.rb'
+
+# Offense count: 3
+# This cop supports unsafe autocorrection (--autocorrect-all).
+# Configuration parameters: Mode.
+Style/StringConcatenation:
+  Exclude:
+    - 'test/opentelemetry/metrics/meter_test.rb'
diff --git a/metrics_api/lib/opentelemetry/metrics/meter.rb b/metrics_api/lib/opentelemetry/metrics/meter.rb
@@ -41,15 +41,15 @@ def create_up_down_counter(name, unit: nil, description: nil)
         create_instrument(:up_down_counter, name, unit, description, nil) { UP_DOWN_COUNTER }
       end
 
-      def create_observable_counter(name, unit: nil, description: nil, callback:)
+      def create_observable_counter(name, callback:, unit: nil, description: nil)
         create_instrument(:observable_counter, name, unit, description, callback) { OBSERVABLE_COUNTER }
       end
 
-      def create_observable_gauge(name, unit: nil, description: nil, callback:)
+      def create_observable_gauge(name, callback:, unit: nil, description: nil)
         create_instrument(:observable_gauge, name, unit, description, callback) { OBSERVABLE_GAUGE }
       end
 
-      def create_observable_up_down_counter(name, unit: nil, description: nil, callback:)
+      def create_observable_up_down_counter(name, callback:, unit: nil, description: nil)
         create_instrument(:observable_up_down_counter, name, unit, description, callback) { OBSERVABLE_UP_DOWN_COUNTER }
       end
 
diff --git a/metrics_api/opentelemetry-metrics-api.gemspec b/metrics_api/opentelemetry-metrics-api.gemspec
@@ -32,7 +32,7 @@ Gem::Specification.new do |spec|
   spec.add_development_dependency 'minitest', '~> 5.0'
   spec.add_development_dependency 'opentelemetry-test-helpers', '~> 0.3.0'
   spec.add_development_dependency 'rake', '~> 12.0'
-  spec.add_development_dependency 'rubocop', '~> 0.73.0'
+  spec.add_development_dependency 'rubocop', '~> 1.51.0'
   spec.add_development_dependency 'simplecov', '~> 0.17'
   spec.add_development_dependency 'yard', '~> 0.9'
   spec.add_development_dependency 'yard-doctest', '~> 0.1.6'
diff --git a/metrics_api/test/opentelemetry/opentelemetry_test.rb b/metrics_api/test/opentelemetry/opentelemetry_test.rb
@@ -26,7 +26,7 @@
 
       OpenTelemetry.meter_provider = new_meter_provider
 
-      assert(OpenTelemetry.meter_provider.object_id == new_meter_provider.object_id)
+      assert_same(OpenTelemetry.meter_provider, new_meter_provider)
     end
 
     describe 'when global MeterProvider is an instance of Internal::ProxyMeterProvider' do
@@ -37,8 +37,8 @@
         OpenTelemetry::TestHelpers.with_test_logger do |log_stream|
           OpenTelemetry.meter_provider = new_meter_provider
 
-          assert(proxy_meter_provider.instance_variable_get(:@delegate).object_id == new_meter_provider.object_id)
-          assert(OpenTelemetry.meter_provider.object_id == new_meter_provider.object_id)
+          assert_same(proxy_meter_provider.instance_variable_get(:@delegate), new_meter_provider)
+          assert_same(OpenTelemetry.meter_provider, new_meter_provider)
           assert(log_stream.string.match?(/Upgrading default proxy meter provider to #{new_meter_provider.class}/i))
         end
       end
diff --git a/propagator/b3/opentelemetry-propagator-b3.gemspec b/propagator/b3/opentelemetry-propagator-b3.gemspec
@@ -31,7 +31,7 @@ Gem::Specification.new do |spec|
   spec.add_development_dependency 'bundler', '>= 1.17'
   spec.add_development_dependency 'minitest', '~> 5.0'
   spec.add_development_dependency 'rake', '~> 12.0'
-  spec.add_development_dependency 'rubocop', '~> 0.73.0'
+  spec.add_development_dependency 'rubocop', '~> 1.51.0'
   spec.add_development_dependency 'simplecov', '~> 0.17.1'
   spec.add_development_dependency 'yard', '~> 0.9'
   spec.add_development_dependency 'yard-doctest', '~> 0.1.6'
diff --git a/propagator/jaeger/.rubocop.yml b/propagator/jaeger/.rubocop.yml
@@ -1,3 +1,5 @@
+inherit_from: .rubocop_todo.yml
+
 AllCops:
   TargetRubyVersion: '2.6.0'
 Lint/UnusedMethodArgument:
diff --git a/propagator/jaeger/.rubocop_todo.yml b/propagator/jaeger/.rubocop_todo.yml
@@ -0,0 +1,26 @@
+# This configuration was generated by
+# `rubocop --auto-gen-config`
+# on 2023-05-22 23:03:15 UTC using RuboCop version 1.51.0.
+# The point is for the user to remove these configuration records
+# one by one as the offenses are removed from the code base.
+# Note that changes in the inspected code, or installation of new
+# versions of RuboCop, may require this file to be generated again.
+
+# Offense count: 1
+Lint/MixedRegexpCaptureTypes:
+  Exclude:
+    - 'lib/opentelemetry/propagator/jaeger/text_map_propagator.rb'
+
+# Offense count: 1
+# This cop supports unsafe autocorrection (--autocorrect-all).
+Style/SlicingWithRange:
+  Exclude:
+    - 'lib/opentelemetry/propagator/jaeger/text_map_propagator.rb'
+
+# Offense count: 2
+# This cop supports unsafe autocorrection (--autocorrect-all).
+# Configuration parameters: Mode.
+Style/StringConcatenation:
+  Exclude:
+    - 'lib/opentelemetry/propagator/jaeger/text_map_propagator.rb'
+    - 'test/text_map_propagator_test.rb'
diff --git a/propagator/jaeger/opentelemetry-propagator-jaeger.gemspec b/propagator/jaeger/opentelemetry-propagator-jaeger.gemspec
@@ -32,7 +32,7 @@ Gem::Specification.new do |spec|
   spec.add_development_dependency 'minitest', '~> 5.0'
   spec.add_development_dependency 'opentelemetry-sdk', '~> 1.2'
   spec.add_development_dependency 'rake', '~> 12.0'
-  spec.add_development_dependency 'rubocop', '~> 0.73.0'
+  spec.add_development_dependency 'rubocop', '~> 1.51.0'
   spec.add_development_dependency 'simplecov', '~> 0.17.1'
   spec.add_development_dependency 'yard', '~> 0.9'
   spec.add_development_dependency 'yard-doctest', '~> 0.1.6'
diff --git a/sdk/.rubocop.yml b/sdk/.rubocop.yml
@@ -1,3 +1,5 @@
+inherit_from: .rubocop_todo.yml
+
 AllCops:
   TargetRubyVersion: "2.6.0"
 
@@ -14,7 +16,7 @@ Metrics/ParameterLists:
 Naming/FileName:
   Exclude:
     - "lib/opentelemetry-sdk.rb"
-Style/ModuleFunction:
+Style/ExplicitBlockArgument:
   Enabled: false
-Style/BracesAroundHashParameters:
+Style/ModuleFunction:
   Enabled: false
diff --git a/sdk/.rubocop_todo.yml b/sdk/.rubocop_todo.yml
@@ -0,0 +1,48 @@
+# This configuration was generated by
+# `rubocop --auto-gen-config`
+# on 2023-05-22 22:52:42 UTC using RuboCop version 1.51.0.
+# The point is for the user to remove these configuration records
+# one by one as the offenses are removed from the code base.
+# Note that changes in the inspected code, or installation of new
+# versions of RuboCop, may require this file to be generated again.
+
+# Offense count: 21
+# Configuration parameters: AllowedMethods.
+# AllowedMethods: enums
+Lint/ConstantDefinitionInBlock:
+  Exclude:
+    - 'test/opentelemetry/sdk/configurator_test.rb'
+    - 'test/opentelemetry/sdk/resources/resource_test.rb'
+    - 'test/opentelemetry/sdk/trace/export/batch_span_processor_test.rb'
+    - 'test/opentelemetry/sdk/trace/samplers/result_test.rb'
+    - 'test/opentelemetry/sdk/trace/samplers_test.rb'
+    - 'test/opentelemetry/sdk/trace/span_test.rb'
+    - 'test/opentelemetry/sdk/trace/tracer_test.rb'
+
+# Offense count: 1
+Lint/FloatComparison:
+  Exclude:
+    - 'lib/opentelemetry/sdk/trace/samplers/trace_id_ratio_based.rb'
+
+# Offense count: 2
+Lint/MissingSuper:
+  Exclude:
+    - 'lib/opentelemetry/sdk/trace/tracer.rb'
+    - 'lib/opentelemetry/sdk/trace/tracer_provider.rb'
+
+# Offense count: 2
+# Configuration parameters: AllowedMethods, AllowedPatterns.
+Metrics/CyclomaticComplexity:
+  Max: 9
+
+# Offense count: 1
+# Configuration parameters: AllowedMethods, AllowedPatterns.
+Metrics/PerceivedComplexity:
+  Max: 9
+
+# Offense count: 1
+# Configuration parameters: AllowedMethods.
+# AllowedMethods: respond_to_missing?
+Style/OptionalBooleanParameter:
+  Exclude:
+    - 'test/opentelemetry/sdk/trace/export/batch_span_processor_test.rb'
diff --git a/sdk/lib/opentelemetry/sdk/forwarding_logger.rb b/sdk/lib/opentelemetry/sdk/forwarding_logger.rb
@@ -10,7 +10,7 @@ module SDK
     # with an ERROR log level, only OpenTelemetry logs at the ERROR level or higher
     # will be emitted.
     class ForwardingLogger
-      def initialize(logger, level:) # rubocop:disable Metrics/CyclomaticComplexity
+      def initialize(logger, level:)
         @logger = logger
 
         if level.is_a?(Integer)
diff --git a/sdk/lib/opentelemetry/sdk/resources/resource.rb b/sdk/lib/opentelemetry/sdk/resources/resource.rb
@@ -11,7 +11,7 @@ module Resources
       # for which telemetry (metrics or traces) is reported.
       class Resource
         class << self
-          private :new # rubocop:disable Style/AccessModifierDeclarations
+          private :new
 
           # Returns a newly created {Resource} with the specified attributes
           #
diff --git a/sdk/lib/opentelemetry/sdk/trace/export/console_span_exporter.rb b/sdk/lib/opentelemetry/sdk/trace/export/console_span_exporter.rb
@@ -4,8 +4,6 @@
 #
 # SPDX-License-Identifier: Apache-2.0
 
-require 'pp'
-
 module OpenTelemetry
   module SDK
     module Trace
diff --git a/sdk/lib/opentelemetry/sdk/trace/samplers/result.rb b/sdk/lib/opentelemetry/sdk/trace/samplers/result.rb
@@ -40,7 +40,7 @@ class Result
           #   an empty Tracestate here, the Tracestate will be cleared, so
           #   samplers SHOULD normally return the passed-in Tracestate if they
           #   do not intend to change it.
-          def initialize(decision:, attributes: nil, tracestate:)
+          def initialize(decision:, tracestate:, attributes: nil)
             @decision = decision
             @attributes = attributes.freeze || EMPTY_HASH
             @tracestate = tracestate
diff --git a/sdk/lib/opentelemetry/sdk/trace/tracer_provider.rb b/sdk/lib/opentelemetry/sdk/trace/tracer_provider.rb
@@ -164,7 +164,7 @@ def internal_start_span(name, kind, attributes, links, start_timestamp, parent_c
 
         private
 
-        def sampler_from_environment(default_sampler) # rubocop:disable Metrics/CyclomaticComplexity
+        def sampler_from_environment(default_sampler)
           case ENV['OTEL_TRACES_SAMPLER']
           when 'always_on' then Samplers::ALWAYS_ON
           when 'always_off' then Samplers::ALWAYS_OFF
diff --git a/sdk/opentelemetry-sdk.gemspec b/sdk/opentelemetry-sdk.gemspec
@@ -43,7 +43,7 @@ Gem::Specification.new do |spec|
   spec.add_development_dependency 'opentelemetry-test-helpers'
   spec.add_development_dependency 'pry'
   spec.add_development_dependency 'rake', '~> 12.0'
-  spec.add_development_dependency 'rubocop', '~> 0.73.0'
+  spec.add_development_dependency 'rubocop', '~> 1.51.0'
   spec.add_development_dependency 'simplecov', '~> 0.17'
   spec.add_development_dependency 'yard', '~> 0.9'
   spec.add_development_dependency 'yard-doctest', '~> 0.1.6'
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
@@ -19,8 +19,7 @@ def initialize(status_codes: nil)
       @failed_batches = []
     end
 
-    attr_reader :batches
-    attr_reader :failed_batches
+    attr_reader :batches, :failed_batches
 
     def export(batch, timeout: nil)
       # If status codes is empty, its a success for less verbose testing

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
	`1`	`+inherit_from: .rubocop_todo.yml`
	`2`	`+`
`1`	`3`	`AllCops:`
`2`	`4`	`TargetRubyVersion: "2.6.0"`
`3`	`5`