Skip to content

Commit

Permalink
Use opts instead of method args
Browse files Browse the repository at this point in the history
  • Loading branch information
estolfo committed Jul 4, 2023
1 parent 45655cc commit d1ed119
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 36 deletions.
21 changes: 6 additions & 15 deletions lib/elastic/transport/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -173,21 +173,21 @@ def initialize(arguments = {}, &block)

# Performs a request through delegation to {#transport}.
#
def perform_request(method, path, params = {}, body = nil, headers = nil, path_templates = nil, endpoint = nil)
def perform_request(method, path, params = {}, body = nil, headers = nil, opts = {})
method = @send_get_body_as if 'GET' == method && body
validate_ca_fingerprints if @ca_fingerprint
if @otel
span_name = endpoint || method
span_name = opts[:endpoint] || method
@otel.tracer.in_span(span_name) do |span|
span['http.request.method'] = method
if endpoint && path_templates
path_params(endpoint, path_templates, path)&.each do |k, v|
if opts[:endpoint] && opts[:path_templates]
@otel.path_params(opts[:endpoint], opts[:path_templates], path)&.each do |k, v|
span["db.elasticsearch.path_parts.#{k}"] = v
end
if body_as_json = @otel.process_body(body, endpoint)
if body_as_json = @otel.process_body(body, opts[:endpoint])
span['db.statement'] = body_as_json
end
span['db.operation'] = endpoint
span['db.operation'] = opts[:endpoint]
end
transport.perform_request(method, path, params || {}, body, headers)
end
Expand All @@ -198,15 +198,6 @@ def perform_request(method, path, params = {}, body = nil, headers = nil, path_t

private

def path_params(endpoint, path_templates, path)
return unless endpoint && path_templates
matching_regexp = @otel.path_regexps(endpoint, path_templates).find do |r|
path.match?(r)
end

path.match(matching_regexp)&.named_captures if matching_regexp
end

def validate_ca_fingerprints
transport.connections.connections.each do |connection|
unless connection.host[:scheme] == 'https'
Expand Down
27 changes: 18 additions & 9 deletions lib/elastic/transport/opentelemetry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,24 @@ def initialize
end
attr_accessor :tracer

def path_params(endpoint, path_templates, path)
return unless endpoint && path_templates
matching_regexp = path_regexps(endpoint, path_templates).find do |r|
path.match?(r)
end

path.match(matching_regexp)&.named_captures if matching_regexp
end

def process_body(body, endpoint)
unless @body_strategy == 'omit' || !SEARCH_ENDPOINTS.include?(endpoint)
if @body_strategy == 'sanitize'
body = Sanitizer.sanitize(body, @sanitize_keys)
end
body.to_json unless body&.is_a?(String)
end
end

def path_regexps(endpoint, path_templates)
return ENDPOINT_PATH_REGEXPS[endpoint] if ENDPOINT_PATH_REGEXPS.key?(endpoint)

Expand All @@ -58,15 +76,6 @@ def path_regexps(endpoint, path_templates)
ENDPOINT_PATH_REGEXPS[endpoint]
end

def process_body(body, endpoint)
unless @body_strategy == 'omit' || !SEARCH_ENDPOINTS.include?(endpoint)
if @body_strategy == 'sanitize'
body = Sanitizer.sanitize(body, @sanitize_keys)
end
body.to_json unless body&.is_a?(String)
end
end

# Replaces values in a hash, given a set of keys to match on.
class Sanitizer
class << self
Expand Down
30 changes: 18 additions & 12 deletions spec/elastic/transport/opentelemetry_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,23 +35,23 @@
context 'when path parameters' do
before do
client.perform_request(
'DELETE', '/users', nil, nil, nil, ["/{index}"],
'delete'
'DELETE', '/users', nil, nil, nil, path_templates: ["/{index}"],
endpoint: 'delete'
)
rescue
end
after do
client.perform_request(
'DELETE', '/users', nil, nil, nil, ["/{index}"],
'delete'
'DELETE', '/users', nil, nil, nil, path_templates: ["/{index}"],
endpoint: 'delete'
)
rescue
end

it 'creates a span with path parameters' do
client.perform_request(
'POST', '/users/_create/abc', nil, { name: 'otel-test' }, nil, ["/{index}/_create/{id}"],
'create'
'POST', '/users/_create/abc', nil, { name: 'otel-test' }, nil,
path_templates: ["/{index}/_create/{id}"], endpoint: 'create'
)

span = exporter.finished_spans.find { |s| s.name == 'create' }
Expand Down Expand Up @@ -97,7 +97,8 @@
end

it 'creates a span and omits db.statement' do
client.perform_request('GET', '/_search', nil, body, nil, ["/_search", "/{index}/_search"], 'search')
client.perform_request('GET', '/_search', nil, body, nil,
path_templates: ["/_search", "/{index}/_search"], endpoint: 'search')

expect(span.name).to eql('search')
expect(span.attributes['db.operation']).to eq('search')
Expand All @@ -121,7 +122,8 @@
end

it 'sanitizes the body' do
client.perform_request('GET', '/_search', nil, body, nil, ["/_search", "/{index}/_search"], 'search')
client.perform_request('GET', '/_search', nil, body, nil,
path_templates: ["/_search", "/{index}/_search"], endpoint: 'search')

expect(span.attributes['db.statement']).to eq(sanitized_body.to_json)
end
Expand Down Expand Up @@ -150,7 +152,8 @@
end

it 'sanitizes the body' do
client.perform_request('GET', '/_search', nil, body, nil, ["/_search", "/{index}/_search"], 'search')
client.perform_request('GET', '/_search', nil, body, nil,
path_templates: ["/_search", "/{index}/_search"], endpoint: 'search')

expect(span.attributes['db.statement']).to eq(sanitized_body.to_json)
end
Expand All @@ -164,7 +167,8 @@

it 'does not capture db.statement' do
client.perform_request(
'POST', '_all/_delete_by_query', nil, body, nil, ["/{index}/_delete_by_query"], 'delete_by_query'
'POST', '_all/_delete_by_query', nil, body, nil,
path_templates: ["/{index}/_delete_by_query"], endpoint: 'delete_by_query'
)

expect(span.attributes['db.statement']).to be_nil
Expand All @@ -182,7 +186,8 @@
end

it 'instruments' do
client.perform_request('GET', '/_search', nil, nil, nil, ["/_search", "/{index}/_search"], 'search')
client.perform_request('GET', '/_search', nil, nil, nil,
path_templates: ["/_search", "/{index}/_search"], endpoint: 'search')
expect(span.name).to eq('search')
end
end
Expand All @@ -196,7 +201,8 @@
end

it 'instruments' do
client.perform_request('GET', '/_search', nil, nil, nil, ["/_search", "/{index}/_search"], 'search')
client.perform_request('GET', '/_search', nil, nil, nil,
path_templates: ["/_search", "/{index}/_search"], endpoint: 'search')
expect(span).to be_nil
end
end
Expand Down

0 comments on commit d1ed119

Please sign in to comment.