Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add and Replace SLASH and ROOT constants #2256

Merged
merged 2 commits into from
Oct 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,6 @@ module InstanceIdentification

DEFAULT = 'default'.freeze
UNKNOWN = 'unknown'.freeze
SLASH = '/'.freeze
LOCALHOST = 'localhost'.freeze

def adapter_from_config(config)
Expand Down Expand Up @@ -288,7 +287,7 @@ def supported_adapter?(config)
private

def postgres_unix_domain_socket_case?(host, adapter)
adapter == :postgres && host && host.start_with?(SLASH)
adapter == :postgres && host && host.start_with?(NewRelic::SLASH)
end

def mysql_default_case?(config, adapter)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,8 @@ def port_path_or_id_from_address(address)
UNKNOWN
end

SLASH = '/'.freeze

def unix_domain_socket?(host)
host.start_with?(SLASH)
host.start_with?(NewRelic::SLASH)
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,12 @@ module Roda
module TransactionNamer
extend self

ROOT = '/'.freeze
REGEX_MULTIPLE_SLASHES = %r{^[/^]*(.*?)[/$?]*$}.freeze

def transaction_name(request)
path = request.path || ::NewRelic::Agent::UNKNOWN_METRIC
name = path.gsub(REGEX_MULTIPLE_SLASHES, '\1') # remove any rogue slashes
name = ROOT if name.empty?
name = NewRelic::ROOT if name.empty?
name = "#{request.request_method} #{name}" if request.respond_to?(:request_method)

name
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,12 @@ def initial_transaction_name(request)
transaction_name(::NewRelic::Agent::UNKNOWN_METRIC, request)
end

ROOT = '/'.freeze

def transaction_name(route_text, request)
verb = http_verb(request)

route_text = route_text.source if route_text.is_a?(Regexp)
name = route_text.gsub(%r{^[/^\\A]*(.*?)[/\$\?\\z]*$}, '\1')
name = ROOT if name.empty?
name = NewRelic::ROOT if name.empty?
name = "#{verb} #{name}" unless verb.nil?
name
rescue => e
Expand Down
4 changes: 2 additions & 2 deletions lib/new_relic/agent/messaging.rb
Original file line number Diff line number Diff line change
Expand Up @@ -329,9 +329,9 @@ def segment_parameters_enabled?

def transaction_name(library, destination_type, destination_name)
transaction_name = Transaction::MESSAGE_PREFIX + library
transaction_name << Transaction::MessageBrokerSegment::SLASH
transaction_name << NewRelic::SLASH
transaction_name << Transaction::MessageBrokerSegment::TYPES[destination_type]
transaction_name << Transaction::MessageBrokerSegment::SLASH
transaction_name << NewRelic::SLASH

case destination_type
when :queue
Expand Down
2 changes: 1 addition & 1 deletion lib/new_relic/agent/rules_engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
module NewRelic
module Agent
class RulesEngine
SEGMENT_SEPARATOR = '/'.freeze
SEGMENT_SEPARATOR = NewRelic::SLASH
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assigned the SLASH to SEGMENT_SEPARATOR because I felt, the SEGMENT_SEPARATOR is the best name for the context for SLASH here. Please let me know if you think otherwise.

Copy link
Contributor

@fallwith fallwith Oct 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having one constant reference another gives us the benefit of using a more context-specific name while still minimizing resources and not* maintaining two copies of the same frozen string. I really like your approach here, @chahmedejaz.

LEADING_SLASH_REGEX = %r{^/}.freeze

include Enumerable
Expand Down
3 changes: 1 addition & 2 deletions lib/new_relic/agent/transaction/message_broker_segment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ class MessageBrokerSegment < Segment
PRODUCE = 'Produce'.freeze
QUEUE = 'Queue'.freeze
PURGE = 'Purge'.freeze
SLASH = '/'.freeze
TEMP = 'Temp'.freeze
TOPIC = 'Topic'.freeze
UNKNOWN = 'Unknown'.freeze
Expand Down Expand Up @@ -73,7 +72,7 @@ def name
return @name if @name

@name = METRIC_PREFIX + library
@name << SLASH << TYPES[destination_type] << SLASH << ACTIONS[action] << SLASH
@name << NewRelic::SLASH << TYPES[destination_type] << NewRelic::SLASH << ACTIONS[action] << NewRelic::SLASH

if destination_type == :temporary_queue || destination_type == :temporary_topic
@name << TEMP
Expand Down
4 changes: 1 addition & 3 deletions lib/new_relic/agent/transaction/request_attributes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,10 @@ def referer_from_request(request)
# rails construct the PATH_INFO enviroment variable improperly and we're generally
# being defensive.

ROOT_PATH = '/'.freeze

def path_from_request(request)
path = attribute_from_request(request, :path) || ''
path = HTTPClients::URIUtil.strip_query_string(path)
path.empty? ? ROOT_PATH : path
path.empty? ? NewRelic::ROOT : path
end

def content_length_from_request(request)
Expand Down
4 changes: 1 addition & 3 deletions lib/new_relic/agent/utilization/gcp.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,8 @@ def prepare_response(response)
body
end

SLASH = '/'.freeze

def trim_leading(value)
value.split(SLASH).last
value.split(NewRelic::SLASH).last
end
end
end
Expand Down
3 changes: 3 additions & 0 deletions lib/new_relic/constants.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,7 @@ module NewRelic

CONNECT_RETRY_PERIODS = [15, 15, 30, 60, 120, 300]
MAX_RETRY_PERIOD = 300

SLASH = '/'
ROOT = SLASH
end
Loading