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

Backport (1.x): Add limits to #log_deprecation #3780

Merged
merged 1 commit into from
Jul 11, 2024
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
21 changes: 2 additions & 19 deletions lib/datadog/core.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# frozen_string_literal: true

require_relative 'core/deprecations'
require_relative 'core/extensions'

# We must load core extensions to make certain global APIs
Expand All @@ -9,25 +10,7 @@ module Datadog
# products. It is a dependency of each product. Contrast with Datadog::Kit
# for higher-level features.
module Core
class << self
# Records the occurrence of a deprecated operation in this library.
#
# Currently, these operations are logged to `Datadog.logger` at `warn` level.
#
# `disallowed_next_major` adds a message informing that the deprecated operation
# won't be allowed in the next major release.
#
# @yieldreturn [String] a String with the lazily evaluated deprecation message.
# @param [Boolean] disallowed_next_major whether this deprecation will be enforced in the next major release.
def log_deprecation(disallowed_next_major: true)
Datadog.logger.warn do
message = yield
message += ' This will be enforced in the next major release.' if disallowed_next_major
message
end
nil
end
end
extend Core::Deprecations
end

extend Core::Extensions
Expand Down
58 changes: 58 additions & 0 deletions lib/datadog/core/deprecations.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# frozen_string_literal: true

module Datadog
module Core
# Contains behavior for handling deprecated functions in the codebase.
module Deprecations
# Records the occurrence of a deprecated operation in this library.
#
# Currently, these operations are logged to `Datadog.logger` at `warn` level.
#
# `disallowed_next_major` adds a message informing that the deprecated operation
# won't be allowed in the next major release.
#
# @yieldreturn [String] a String with the lazily evaluated deprecation message.
# @param [Boolean] disallowed_next_major whether this deprecation will be enforced in the next major release.
# @param [Object] key A unique key for the deprecation. Only the first message with the same key will be logged.
def log_deprecation(disallowed_next_major: true, key: nil)
return unless log_deprecation?(key)

Datadog.logger.warn do
message = yield
message += ' This will be enforced in the next major release.' if disallowed_next_major

Choose a reason for hiding this comment

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

Static Analysis Violation (ruby-best-practices/concat-strings)

⚪ Notice - Best Practices - View in Datadog

Suggested change
message += ' This will be enforced in the next major release.' if disallowed_next_major
message << ' This will be enforced in the next major release.' if disallowed_next_major
Avoid appending to string using += (read more)

The rule to avoid slow string concatenation in Ruby is essential for writing efficient and fast-performing code. String concatenation using the += operator is slower because it creates a new string object every time it's used. This can lead to performance issues, especially in loops or large programs where numerous string concatenations might be happening.

Instead, the << operator, also known as the append operator, should be used for string concatenation in Ruby. The << operator modifies the original string, avoiding the creation of multiple unnecessary string objects. This results in faster execution time and lower memory usage, which is especially beneficial in larger applications or systems with limited resources.

Therefore, good coding practice in Ruby suggests using << for string concatenation instead of +=. For instance, output << "<p>#{text}</p>" is more efficient than output += "<p>#{text}</p>". Following this rule will help you write cleaner, faster, and more resource-efficient Ruby code.

Leave feedback in #static-analysis

message
end

# Track the deprecation being logged.
deprecation_logged!(key)

nil
end

private

# Determines whether a deprecation message should be logged.
#
# Internal use only.
def log_deprecation?(key)
return true if key.nil?

# Only allow a deprecation to be logged once.
!logged_deprecations.key?(key)
end

def deprecation_logged!(key)
return if key.nil?

logged_deprecations[key] += 1
end

# Tracks what deprecation warnings have already been logged
#
# Internal use only.
def logged_deprecations
@logged_deprecations ||= Hash.new(0)
end
end
end
end
1 change: 1 addition & 0 deletions sig/datadog/core.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ module Datadog
module Core
end

extend Core::Deprecations
extend Core::Configuration
extend Tracing::Contrib::Extensions::Helpers
end
22 changes: 22 additions & 0 deletions sig/datadog/core/deprecations.rbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
module Datadog
module Core
module Deprecations
interface _Hashing
def hash: () -> ::Integer
def eql?: (untyped) -> bool
def nil?: () -> bool
end

type key = _Hashing

@logged_deprecations: ::Hash[key, ::Integer]
def log_deprecation: (?disallowed_next_major: bool, ?key: key?) { () -> String } -> void

private
def log_deprecation?: (key key) -> bool

def deprecation_logged!: (key key) -> void
def logged_deprecations: () -> ::Hash[key, ::Integer]
end
end
end
91 changes: 91 additions & 0 deletions spec/datadog/core/deprecations_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
require 'spec_helper'

require 'datadog/core/deprecations'

RSpec.describe Datadog::Core::Deprecations do
context 'when extended' do
subject(:test_class) { Class.new { extend Datadog::Core::Deprecations } }

describe '.log_deprecation' do
subject(:log_deprecation) { call_log_deprecation }

let(:options) { {} }
let(:message) { 'Longer allowed.' }

def call_log_deprecation
test_class.log_deprecation(**options) { message }
end

context 'by default' do
it 'warns with enforcement message' do
expect(Datadog.logger).to receive(:warn) do |&block|
expect(block.call).to eq('Longer allowed. This will be enforced in the next major release.')
end
log_deprecation
end

it 'does not limit messages' do
expect(Datadog.logger).to receive(:warn).twice
2.times { call_log_deprecation }
end
end

context 'with disallowed_next_major:' do
let(:options) { { disallowed_next_major: disallowed_next_major } }

context 'true' do
let(:disallowed_next_major) { true }

it 'warns with enforcement message' do
expect(Datadog.logger).to receive(:warn) do |&block|
expect(block.call).to eq('Longer allowed. This will be enforced in the next major release.')
end
log_deprecation
end
end

context 'false' do
let(:disallowed_next_major) { false }

it 'warns with enforcement message' do
expect(Datadog.logger).to receive(:warn) do |&block|
expect(block.call).to eq('Longer allowed.')
end
log_deprecation
end
end
end

context 'with key:' do
let(:options) { { key: key } }

context 'nil' do
let(:key) { nil }

it 'does not limit messages' do
expect(Datadog.logger).to receive(:warn).twice
2.times { call_log_deprecation }
end
end

context 'Symbol' do
let(:key) { :deprecated_setting }

it 'limits messages' do
expect(Datadog.logger).to receive(:warn).once
2.times { call_log_deprecation }
end
end

context 'String' do
let(:key) { 'deprecated_setting' }

it 'limits messages' do
expect(Datadog.logger).to receive(:warn).once
2.times { call_log_deprecation }
end
end
end
end
end
end
37 changes: 1 addition & 36 deletions spec/datadog/core_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,42 +3,7 @@
require 'datadog/core'

RSpec.describe Datadog::Core do
describe '.log_deprecation' do
subject(:log_deprecation) { described_class.log_deprecation(**options) { message } }
let(:options) { {} }
let(:message) { 'Longer allowed.' }

context 'by default' do
it 'warns with enforcement message' do
expect(Datadog.logger).to receive(:warn) do |&block|
expect(block.call).to eq('Longer allowed. This will be enforced in the next major release.')
end
log_deprecation
end
end

context 'with disallowed_next_major true' do
let(:options) { { disallowed_next_major: true } }

it 'warns with enforcement message' do
expect(Datadog.logger).to receive(:warn) do |&block|
expect(block.call).to eq('Longer allowed. This will be enforced in the next major release.')
end
log_deprecation
end
end

context 'with disallowed_next_major false' do
let(:options) { { disallowed_next_major: false } }

it 'warns with enforcement message' do
expect(Datadog.logger).to receive(:warn) do |&block|
expect(block.call).to eq('Longer allowed.')
end
log_deprecation
end
end
end
it { expect(described_class).to be_a_kind_of(Datadog::Core::Deprecations) }
end

RSpec.describe Datadog do
Expand Down
Loading