Skip to content

Commit

Permalink
Remove deprecated extension functions
Browse files Browse the repository at this point in the history
I want to remove these functions from the Rust extension, but if the
Ruby gem still calls them, that would cause a new error on the next
extension update.

These functions already did nothing anymore in the Rust extension,
so not calling the extension functions loses no functionality. I did not
remove the Ruby methods, because people may still call them and we
should only remove methods in major releases.

Move the deprecation warnings to the Ruby gem and remove calls to the
extension functions.

- Part of appsignal/appsignal-agent#425
- Deprecations logged in
  #667
  • Loading branch information
tombruijn committed Jan 11, 2024
1 parent 0637b71 commit 3afb28f
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 78 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
bump: "patch"
type: "deprecate"
---

Deprecate the `Appsignal.set_host_guage` and `Appsignal.set_process_gauge` helper methods in the Ruby gem. These methods would already log deprecation warnings in the `appsignal.log` file, but now also as a Ruby warning. These methods will be removed in the next major version. These methods already did not report any metrics, and still do not.
24 changes: 0 additions & 24 deletions ext/appsignal_extension.c
Original file line number Diff line number Diff line change
Expand Up @@ -759,28 +759,6 @@ static VALUE set_gauge(VALUE self, VALUE key, VALUE value, VALUE tags) {
return Qnil;
}

static VALUE set_host_gauge(VALUE self, VALUE key, VALUE value) {
Check_Type(key, T_STRING);
Check_Type(value, T_FLOAT);

appsignal_set_host_gauge(
make_appsignal_string(key),
NUM2DBL(value)
);
return Qnil;
}

static VALUE set_process_gauge(VALUE self, VALUE key, VALUE value) {
Check_Type(key, T_STRING);
Check_Type(value, T_FLOAT);

appsignal_set_process_gauge(
make_appsignal_string(key),
NUM2DBL(value)
);
return Qnil;
}

static VALUE increment_counter(VALUE self, VALUE key, VALUE count, VALUE tags) {
appsignal_data_t* tags_data;

Expand Down Expand Up @@ -941,8 +919,6 @@ void Init_appsignal_extension(void) {

// Metrics
rb_define_singleton_method(Extension, "set_gauge", set_gauge, 3);
rb_define_singleton_method(Extension, "set_host_gauge", set_host_gauge, 2);
rb_define_singleton_method(Extension, "set_process_gauge", set_process_gauge, 2);
rb_define_singleton_method(Extension, "increment_counter", increment_counter, 3);
rb_define_singleton_method(Extension, "add_distribution_value", add_distribution_value, 3);
}
14 changes: 0 additions & 14 deletions lib/appsignal/extension/jruby.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,6 @@ def self.lib_extension
attach_function :appsignal_set_gauge,
[:appsignal_string, :double, :pointer],
:void
attach_function :appsignal_set_host_gauge,
[:appsignal_string, :double],
:void
attach_function :appsignal_set_process_gauge,
[:appsignal_string, :double],
:void
attach_function :appsignal_increment_counter,
[:appsignal_string, :double, :pointer],
:void
Expand Down Expand Up @@ -319,14 +313,6 @@ def set_gauge(key, value, tags)
appsignal_set_gauge(make_appsignal_string(key), value, tags.pointer)
end

def set_host_gauge(key, value)
appsignal_set_host_gauge(make_appsignal_string(key), value)
end

def set_process_gauge(key, value)
appsignal_set_process_gauge(make_appsignal_string(key), value)
end

def increment_counter(key, value, tags)
appsignal_increment_counter(make_appsignal_string(key), value, tags.pointer)
end
Expand Down
22 changes: 12 additions & 10 deletions lib/appsignal/helpers/metrics.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,20 @@ def set_gauge(key, value, tags = {})
.warn("Gauge value #{value} for key '#{key}' is too big")
end

def set_host_gauge(key, value)
Appsignal::Extension.set_host_gauge(key.to_s, value.to_f)
rescue RangeError
Appsignal.logger
.warn("Host gauge value #{value} for key '#{key}' is too big")
def set_host_gauge(_key, _value)
Appsignal::Utils::DeprecationMessage.message \
"The `set_host_gauge` method has been deprecated. " \
"Calling this method has no effect. " \
"Please remove method call in the following file to remove " \
"this message.\n#{caller.first}"
end

def set_process_gauge(key, value)
Appsignal::Extension.set_process_gauge(key.to_s, value.to_f)
rescue RangeError
Appsignal.logger
.warn("Process gauge value #{value} for key '#{key}' is too big")
def set_process_gauge(_key, _value)
Appsignal::Utils::DeprecationMessage.message \
"The `set_process_gauge` method has been deprecated. " \
"Calling this method has no effect. " \
"Please remove method call in the following file to remove " \
"this message.\n#{caller.first}"
end

def increment_counter(key, value = 1.0, tags = {})
Expand Down
60 changes: 30 additions & 30 deletions spec/lib/appsignal_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -553,46 +553,46 @@
end

describe ".set_host_gauge" do
it "should call set_host_gauge on the extension with a string key and float" do
expect(Appsignal::Extension).to receive(:set_host_gauge).with("key", 0.1)
Appsignal.set_host_gauge("key", 0.1)
let(:err_stream) { std_stream }
let(:stderr) { err_stream.read }
let(:log_stream) { StringIO.new }
let(:logs) { log_contents(log_stream) }
let(:deprecation_message) do
"The `set_host_gauge` method has been deprecated. " \
"Calling this method has no effect. " \
"Please remove method call in the following file to remove " \
"this message."
end

it "should call set_host_gauge on the extension with a symbol key and int" do
expect(Appsignal::Extension).to receive(:set_host_gauge).with("key", 1.0)
Appsignal.set_host_gauge(:key, 1)
before do
Appsignal.logger = test_logger(log_stream)
capture_std_streams(std_stream, err_stream) { Appsignal.set_host_gauge("key", 0.1) }
end

it "should not raise an exception when out of range" do
expect(Appsignal::Extension).to receive(:set_host_gauge).with("key",
10).and_raise(RangeError)
expect(Appsignal.logger).to receive(:warn)
.with("Host gauge value 10 for key 'key' is too big")
expect do
Appsignal.set_host_gauge("key", 10)
end.to_not raise_error
it "logs a deprecation warning" do
expect(stderr).to include("appsignal WARNING: #{deprecation_message}")
expect(logs).to include(deprecation_message)
end
end

describe ".set_process_gauge" do
it "should call set_process_gauge on the extension with a string key and float" do
expect(Appsignal::Extension).to receive(:set_process_gauge).with("key", 0.1)
Appsignal.set_process_gauge("key", 0.1)
let(:err_stream) { std_stream }
let(:stderr) { err_stream.read }
let(:log_stream) { StringIO.new }
let(:logs) { log_contents(log_stream) }
let(:deprecation_message) do
"The `set_process_gauge` method has been deprecated. " \
"Calling this method has no effect. " \
"Please remove method call in the following file to remove " \
"this message."
end

it "should call set_process_gauge on the extension with a symbol key and int" do
expect(Appsignal::Extension).to receive(:set_process_gauge).with("key", 1.0)
Appsignal.set_process_gauge(:key, 1)
before do
Appsignal.logger = test_logger(log_stream)
capture_std_streams(std_stream, err_stream) { Appsignal.set_process_gauge("key", 0.1) }
end

it "should not raise an exception when out of range" do
expect(Appsignal::Extension).to receive(:set_process_gauge).with("key",
10).and_raise(RangeError)
expect(Appsignal.logger).to receive(:warn)
.with("Process gauge value 10 for key 'key' is too big")
expect do
Appsignal.set_process_gauge("key", 10)
end.to_not raise_error
it "logs a deprecation warning" do
expect(stderr).to include("appsignal WARNING: #{deprecation_message}")
expect(logs).to include(deprecation_message)
end
end

Expand Down

0 comments on commit 3afb28f

Please sign in to comment.