feat: Updates based on rubocop

open-telemetry · Mar 1, 2023 · 4ca8c88 · 4ca8c88
1 parent 7cf3e31
commit 4ca8c88
Show file tree

Hide file tree

Showing 8 changed files with 49 additions and 75 deletions.
diff --git a/...entation/elasticsearch/lib/opentelemetry/instrumentation/elasticsearch/instrumentation.rb b/...entation/elasticsearch/lib/opentelemetry/instrumentation/elasticsearch/instrumentation.rb
@@ -11,8 +11,8 @@ module Elasticsearch
       class Instrumentation < OpenTelemetry::Instrumentation::Base
         MINIMUM_VERSION = Gem::Version.new('8.0.0')
 
-        install do |_config|
-          convert_config(_config)
+        install do |config|
+          convert_config(config)
           require_dependencies
           patch
         end
@@ -39,9 +39,9 @@ def patch
         private
 
         def convert_config(config)
-          if field_names = config[:sanitize_field_names]
-            config[:sanitize_field_names] = field_names.map { |p| WildcardPattern.new(p) }
-          end
+          return unless (field_names = config[:sanitize_field_names])
+
+          config[:sanitize_field_names] = field_names.map { |p| WildcardPattern.new(p) }
         end
 
         def gem_version

diff --git a/...mentation/elasticsearch/lib/opentelemetry/instrumentation/elasticsearch/patches/client.rb b/...mentation/elasticsearch/lib/opentelemetry/instrumentation/elasticsearch/patches/client.rb
@@ -13,24 +13,25 @@ module Patches
 
         # Module to prepend to Elastic::Transport::Client for instrumentation
         module Client
+          # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
           def perform_request(method, path, *args, &block)
             config = Elasticsearch::Instrumentation.instance.config
             attributes = {
               'db.system' => 'elasticsearch',
-              # TODO should we set db.name?
-              #'db.name' => database_name,
+              # TODO: should we set db.name?
+              # 'db.name' => database_name,
               'db.operation' => method,
-              # TODO is this true for all elasticsearch client requests?
+              # TODO: is this true for all elasticsearch client requests?
               'net.transport' => IP_TCP,
               'elasticsearch.method' => method
-              # TODO set other elasticsearch custom attributes, based on spec
+              # TODO: set other elasticsearch custom attributes, based on spec
             }
             attributes['peer.service'] = config[:peer_service] if config[:peer_service]
             attributes['elasticsearch.params'] = args&.[](0).to_json if args&.[](0)
 
             # We can only be sure of the host info if there is one host.
             # Otherwise, the host will be selected further down the call stack.
-            if host = @hosts.size == 1 && @hosts[0]
+            if (host = @hosts.size == 1 && @hosts[0])
               attributes['net.peer.name'] = host[:host]
               attributes['net.peer.port'] = host[:port]
             end
@@ -40,9 +41,7 @@ def perform_request(method, path, *args, &block)
             obfuscate = config[:db_statement] == :obfuscate
             unless omit
               body = Sanitizer.sanitize(body, config[:sanitize_field_names]) if obfuscate
-              if body && !body.is_a?(String)
-                body = body.to_json
-              end
+              body = body.to_json if body && !body.is_a?(String)
               attributes['db.statement'] = body
             end
 
@@ -51,6 +50,7 @@ def perform_request(method, path, *args, &block)
               super
             end
           end
+          # rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
 
           def tracer
             Elasticsearch::Instrumentation.instance.tracer

diff --git a/...ntation/elasticsearch/lib/opentelemetry/instrumentation/elasticsearch/patches/deep_dup.rb b/...ntation/elasticsearch/lib/opentelemetry/instrumentation/elasticsearch/patches/deep_dup.rb
@@ -8,8 +8,6 @@ module OpenTelemetry
   module Instrumentation
     module Elasticsearch
       module Patches
-        # @api private
-        #
         # Makes a deep copy of an Array or Hash
         # NB: Not guaranteed to work well with complex objects, only simple Hash,
         # Array, String, Number, etc.

diff --git a/...tation/elasticsearch/lib/opentelemetry/instrumentation/elasticsearch/patches/sanitizer.rb b/...tation/elasticsearch/lib/opentelemetry/instrumentation/elasticsearch/patches/sanitizer.rb
@@ -8,10 +8,9 @@ module OpenTelemetry
   module Instrumentation
     module Elasticsearch
       module Patches
+        # Replaces values in a hash, given a set of keys to match on.
         class Sanitizer
-
           class << self
-
             FILTERED = '?'
             DEFAULT_KEY_PATTERNS =
               %w[password passwd pwd secret *key *token* *session* *credit* *card* *auth* set-cookie].map! do |p|

diff --git a/...ntation/elasticsearch/lib/opentelemetry/instrumentation/elasticsearch/wildcard_pattern.rb b/...ntation/elasticsearch/lib/opentelemetry/instrumentation/elasticsearch/wildcard_pattern.rb
@@ -7,6 +7,7 @@
 module OpenTelemetry
   module Instrumentation
     module Elasticsearch
+      # Object representing a user-supplied key pattern that behaves as a regex
       class WildcardPattern
         def initialize(str)
           @pattern = convert(str)
@@ -18,7 +19,7 @@ def match?(other)
           !!@pattern.match(other)
         end
 
-        alias :match :match?
+        alias match match?
 
         private
 
@@ -36,12 +37,11 @@ def convert(str)
             end
 
           Regexp.new(
-            '\A' + parts.join + '\Z',
+            '\A' + parts.join + '\Z', # rubocop:disable Style/StringConcatenation
             case_sensitive ? nil : Regexp::IGNORECASE
           )
         end
       end
     end
   end
 end
-
diff --git a/instrumentation/elasticsearch/test/instrumentation/elasticsearch/instrumentation_test.rb b/instrumentation/elasticsearch/test/instrumentation/elasticsearch/instrumentation_test.rb
@@ -1,8 +1,8 @@
-# # frozen_string_literal: true
+# frozen_string_literal: true
+
+# Copyright The OpenTelemetry Authors
 #
-# # Copyright The OpenTelemetry Authors
-# #
-# # SPDX-License-Identifier: Apache-2.0
+# SPDX-License-Identifier: Apache-2.0
 
 require 'test_helper'
 

diff --git a/instrumentation/elasticsearch/test/instrumentation/elasticsearch/patches/client_test.rb b/instrumentation/elasticsearch/test/instrumentation/elasticsearch/patches/client_test.rb
@@ -17,7 +17,7 @@
   let(:config) { {} }
   let(:client) do
     Elasticsearch::Client.new(log: false).tap do |client|
-      client.instance_variable_set(:"@verified", true)
+      client.instance_variable_set(:@verified, true)
     end
   end
 
@@ -48,18 +48,15 @@
       _(exporter.finished_spans.size).must_equal(1)
       _(span.name).must_equal 'GET _search'
       _(span.attributes['db.statement']).must_be_nil
-      _(span.attributes['db.system']).must_equal "elasticsearch"
-      _(span.attributes['db.operation']).must_equal "GET"
+      _(span.attributes['db.system']).must_equal 'elasticsearch'
+      _(span.attributes['db.operation']).must_equal 'GET'
       _(span.attributes['elasticsearch.method']).must_equal 'GET'
       _(span.attributes['net.transport']).must_equal 'ip_tcp'
 
       _(span.attributes['net.peer.name']).must_equal 'localhost'
       _(span.attributes['net.peer.port']).must_equal 9200
-      #_(span.attributes['elasticsearch.url']).must_equal 'http://localhost:9200/_search?q=test'
 
-      _(span.attributes['elasticsearch.params']).must_equal "{\"q\":\"test\"}"
-      #_(span.attributes['elasticsearch.id']).must_equal # doc id
-      #_(span.attributes['elasticsearch.target']).must_equal '_search'
+      _(span.attributes['elasticsearch.params']).must_equal "{\"q\":\"test\"}" # rubocop:disable Style/StringLiterals
       assert_requested(
         :get,
         'http://localhost:9200/_search?q=test'
@@ -84,18 +81,15 @@
       _(exporter.finished_spans.size).must_equal(1)
       _(span.name).must_equal 'POST _bulk'
       _(span.attributes['db.statement']).must_be_nil
-      _(span.attributes['db.system']).must_equal "elasticsearch"
-      _(span.attributes['db.operation']).must_equal "POST"
+      _(span.attributes['db.system']).must_equal 'elasticsearch'
+      _(span.attributes['db.operation']).must_equal 'POST'
       _(span.attributes['elasticsearch.method']).must_equal 'POST'
       _(span.attributes['net.transport']).must_equal 'ip_tcp'
 
       _(span.attributes['net.peer.name']).must_equal 'localhost'
       _(span.attributes['net.peer.port']).must_equal 9200
-      #_(span.attributes['elasticsearch.url']).must_equal 'http://localhost:9200/_search?q=test'
 
       _(span.attributes['elasticsearch.params']).must_equal '{}'
-      #_(span.attributes['elasticsearch.id']).must_equal # doc id
-      #_(span.attributes['elasticsearch.target']).must_equal '_search'
       assert_requested(
         :post,
         'http://localhost:9200/_bulk'
@@ -122,18 +116,15 @@
       _(span.attributes['db.statement']).must_equal(
         "{\"index\":{\"_index\":\"users\"}}\n{\"name\":\"Emily\",\"password\":\"top_secret\"}\n"
       )
-      _(span.attributes['db.system']).must_equal "elasticsearch"
-      _(span.attributes['db.operation']).must_equal "POST"
+      _(span.attributes['db.system']).must_equal 'elasticsearch'
+      _(span.attributes['db.operation']).must_equal 'POST'
       _(span.attributes['elasticsearch.method']).must_equal 'POST'
       _(span.attributes['net.transport']).must_equal 'ip_tcp'
 
       _(span.attributes['net.peer.name']).must_equal 'localhost'
       _(span.attributes['net.peer.port']).must_equal 9200
-      #_(span.attributes['elasticsearch.url']).must_equal 'http://localhost:9200/_search?q=test'
 
       _(span.attributes['elasticsearch.params']).must_equal '{}'
-      #_(span.attributes['elasticsearch.id']).must_equal # doc id
-      #_(span.attributes['elasticsearch.target']).must_equal '_search'
       assert_requested(
         :post,
         'http://localhost:9200/_bulk'
@@ -145,28 +136,23 @@
     it 'captures the span attributes' do
       client.bulk(
         refresh: true,
-        body: [{
-          index: { _index: 'users', data: { name: 'Fernando' } }
-         }]
+        body: [{ index: { _index: 'users', data: { name: 'Fernando' } } }]
       )
 
       _(exporter.finished_spans.size).must_equal(1)
       _(span.name).must_equal 'POST _bulk'
       _(span.attributes['db.statement']).must_equal(
         "{\"index\":{\"_index\":\"users\"}}\n{\"name\":\"Fernando\"}\n"
       )
-      _(span.attributes['db.system']).must_equal "elasticsearch"
-      _(span.attributes['db.operation']).must_equal "POST"
+      _(span.attributes['db.system']).must_equal 'elasticsearch'
+      _(span.attributes['db.operation']).must_equal 'POST'
       _(span.attributes['elasticsearch.method']).must_equal 'POST'
       _(span.attributes['net.transport']).must_equal 'ip_tcp'
 
       _(span.attributes['net.peer.name']).must_equal 'localhost'
       _(span.attributes['net.peer.port']).must_equal 9200
-      #_(span.attributes['elasticsearch.url']).must_equal 'http://localhost:9200/_search?q=test'
 
-      _(span.attributes['elasticsearch.params']).must_equal "{\"refresh\":true}"
-      #_(span.attributes['elasticsearch.id']).must_equal # doc id
-      #_(span.attributes['elasticsearch.target']).must_equal '_search'
+      _(span.attributes['elasticsearch.params']).must_equal "{\"refresh\":true}" # rubocop:disable Style/StringLiterals
       assert_requested(
         :post,
         'http://localhost:9200/_bulk?refresh=true'
@@ -185,20 +171,17 @@
       _(exporter.finished_spans.size).must_equal(1)
       _(span.name).must_equal 'PUT users/_doc/1'
       _(span.attributes['db.statement']).must_equal(
-        "{\"name\":\"Emily\",\"password\":\"?\"}"
+        "{\"name\":\"Emily\",\"password\":\"?\"}" # rubocop:disable Style/StringLiterals
       )
-      _(span.attributes['db.system']).must_equal "elasticsearch"
-      _(span.attributes['db.operation']).must_equal "PUT"
+      _(span.attributes['db.system']).must_equal 'elasticsearch'
+      _(span.attributes['db.operation']).must_equal 'PUT'
       _(span.attributes['elasticsearch.method']).must_equal 'PUT'
       _(span.attributes['net.transport']).must_equal 'ip_tcp'
 
       _(span.attributes['net.peer.name']).must_equal 'localhost'
       _(span.attributes['net.peer.port']).must_equal 9200
-      #_(span.attributes['elasticsearch.url']).must_equal 'http://localhost:9200/_search?q=test'
 
       _(span.attributes['elasticsearch.params']).must_equal '{}'
-      #_(span.attributes['elasticsearch.id']).must_equal # doc id
-      #_(span.attributes['elasticsearch.target']).must_equal '_search'
       assert_requested(
         :put,
         'http://localhost:9200/users/_doc/1'
@@ -218,20 +201,17 @@
       _(exporter.finished_spans.size).must_equal(1)
       _(span.name).must_equal 'PUT users/_doc/1'
       _(span.attributes['db.statement']).must_equal(
-        "{\"name\":\"Emily\",\"abcde\":\"?\"}"
+        "{\"name\":\"Emily\",\"abcde\":\"?\"}" # rubocop:disable Style/StringLiterals
       )
-      _(span.attributes['db.system']).must_equal "elasticsearch"
-      _(span.attributes['db.operation']).must_equal "PUT"
+      _(span.attributes['db.system']).must_equal 'elasticsearch'
+      _(span.attributes['db.operation']).must_equal 'PUT'
       _(span.attributes['elasticsearch.method']).must_equal 'PUT'
       _(span.attributes['net.transport']).must_equal 'ip_tcp'
 
       _(span.attributes['net.peer.name']).must_equal 'localhost'
       _(span.attributes['net.peer.port']).must_equal 9200
-      #_(span.attributes['elasticsearch.url']).must_equal 'http://localhost:9200/_search?q=test'
 
       _(span.attributes['elasticsearch.params']).must_equal '{}'
-      #_(span.attributes['elasticsearch.id']).must_equal # doc id
-      #_(span.attributes['elasticsearch.target']).must_equal '_search'
       assert_requested(
         :put,
         'http://localhost:9200/users/_doc/1'
@@ -248,24 +228,21 @@
     it 'adds an error event to the span' do
       begin
         client.search q: 'test'
-      rescue
+      rescue StandardError # rubocop:disable Lint/SuppressedException
       end
 
       _(exporter.finished_spans.size).must_equal(1)
       _(span.name).must_equal 'GET _search'
       _(span.attributes['db.statement']).must_be_nil
-      _(span.attributes['db.system']).must_equal "elasticsearch"
-      _(span.attributes['db.operation']).must_equal "GET"
+      _(span.attributes['db.system']).must_equal 'elasticsearch'
+      _(span.attributes['db.operation']).must_equal 'GET'
       _(span.attributes['elasticsearch.method']).must_equal 'GET'
       _(span.attributes['net.transport']).must_equal 'ip_tcp'
 
       _(span.attributes['net.peer.name']).must_equal 'localhost'
       _(span.attributes['net.peer.port']).must_equal 9200
-      #_(span.attributes['elasticsearch.url']).must_equal 'http://localhost:9200/_search?q=test'
 
-      _(span.attributes['elasticsearch.params']).must_equal "{\"q\":\"test\"}"
-      #_(span.attributes['elasticsearch.id']).must_equal # doc id
-      #_(span.attributes['elasticsearch.target']).must_equal '_search'
+      _(span.attributes['elasticsearch.params']).must_equal "{\"q\":\"test\"}" # rubocop:disable Style/StringLiterals
 
       _(span.status.code).must_equal(
         OpenTelemetry::Trace::Status::ERROR

diff --git a/instrumentation/elasticsearch/test/instrumentation/elasticsearch/patches/sanitizer_test.rb b/instrumentation/elasticsearch/test/instrumentation/elasticsearch/patches/sanitizer_test.rb
@@ -13,12 +13,12 @@
   let(:sanitizer) { OpenTelemetry::Instrumentation::Elasticsearch::Patches::Sanitizer }
 
   describe '#sanitize with default key patterns' do
-    let(:obj) {
+    let(:obj) do
       {
         query: 'a query',
         password: 'top secret'
       }
-    }
+    end
 
     it 'sanitizes default key patterns' do
       _(sanitizer.sanitize(obj)).must_equal(
@@ -33,12 +33,12 @@
   describe '#sanitize with custom key patterns' do
     let(:key_patterns) { [/.*sensitive.*/] }
 
-    let(:obj) {
+    let(:obj) do
       {
         query: 'a query',
         some_sensitive_field: 'sensitive data'
       }
-    }
+    end
 
     it 'sanitizes custom key patterns' do
       _(sanitizer.sanitize(obj, key_patterns)).must_equal(
@@ -53,12 +53,12 @@
   describe '#sanitize with no matching key patterns' do
     let(:key_patterns) { [/.*sensitive.*/] }
 
-    let(:obj) {
+    let(:obj) do
       {
         query: 'a query',
         a_normal_field: 'normal data'
       }
-    }
+    end
 
     it 'does not sanitize fields' do
       _(sanitizer.sanitize(obj, key_patterns)).must_equal(