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

feat: support classic ingest keys #238

Merged
merged 4 commits into from
Mar 7, 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
2 changes: 1 addition & 1 deletion honeycomb-beeline.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ Gem::Specification.new do |spec|
spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) }
spec.require_paths = ["lib"]

spec.add_dependency "libhoney", ">= 1.14.2"
spec.add_dependency "libhoney", ">= 2.3.0"

spec.add_development_dependency "appraisal"
spec.add_development_dependency "bump"
Expand Down
6 changes: 1 addition & 5 deletions lib/honeycomb/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def initialize(configuration:)
@libhoney.add_field "service.name", configuration.service_name
@context = Context.new

@context.classic = classic_write_key?(configuration.write_key)
@context.classic = configuration.classic?

@additional_trace_options = {
presend_hook: configuration.presend_hook,
Expand Down Expand Up @@ -128,9 +128,5 @@ def add_exception_data(span, exception)
span.add_field("error_backtrace_limit", error_backtrace_limit)
span.add_field("error_backtrace_total_length", exception.backtrace.length)
end

def classic_write_key?(write_key)
Copy link
Member Author

@robbkidd robbkidd Mar 6, 2024

Choose a reason for hiding this comment

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

This was a private method and its only use was to set the classic instance variable on @context.

@context.classic is set now by asking the configuration object directly about its classicness.

write_key.nil? || write_key.length == 32
end
end
end
2 changes: 1 addition & 1 deletion lib/honeycomb/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def initialize
end

def classic?
@write_key.nil? || @write_key.length == 32
Libhoney.classic_api_key?(@write_key)
end

def service_name
Expand Down
40 changes: 38 additions & 2 deletions spec/honeycomb/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,41 @@
expect(configuration.write_key).to eq write_key
end

describe "#classic?" do
Copy link
Member Author

Choose a reason for hiding this comment

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

Spec the classicness of the various key formats once here.

it "is true when no key is set" do
configuration.write_key = nil
expect(configuration.classic?).to be true
end

it "is true with a classic key" do
configuration.write_key = "c1a551c000d68f9ed1e96432ac1a3380"
expect(configuration.classic?).to be true
end

it "is true with a classic v3 ingest key" do
configuration.write_key = "hcaic_1234567890123456789012345678901234567890123456789012345678"
expect(configuration.classic?).to be true
end

it "is false with an E&S key" do
configuration.write_key = "1234567890123456789012"
expect(configuration.classic?).to be false
end

it "is false with an E&S v3 ingest key" do
configuration.write_key = "hcaik_1234567890123456789012345678901234567890123456789012345678"
expect(configuration.classic?).to be false
end

it "is false with a v3 key id only" do
configuration.write_key = "hcxik_12345678901234567890123456"
expect(configuration.classic?).to be false
end
end

describe "#service_name" do
it "returns the default unknown-service:<process_name> when not set" do
expect(configuration.instance_variable_get(:@service_name)).to be_nil
# rspec is the expected process name because *this test suite* is rspec
expect(configuration.service_name).to eq "unknown_service:rspec"
end
Expand All @@ -61,10 +94,12 @@

context "with a classic write key" do
before do
configuration.write_key = "c1a551c000d68f9ed1e96432ac1a3380"
allow(configuration).to receive(:classic?).and_return(true)
Copy link
Member Author

Choose a reason for hiding this comment

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

Stub the classic check within "classic" test scopes instead of repeating the matrix of key types all over again.

end

it "returns the value of dataset when service_name isn't set" do
expect(configuration.instance_variable_get(:@service_name)).to be_nil

configuration.dataset = "a_dataset"
expect(configuration.service_name).to eq "a_dataset"
end
Expand Down Expand Up @@ -105,11 +140,12 @@

context "with a classic write key" do
before do
configuration.write_key = "c1a551c000d68f9ed1e96432ac1a3380"
allow(configuration).to receive(:classic?).and_return(true)
end

it "returns whatever dataset has been set" do
expect(configuration.dataset).to be_nil

configuration.dataset = "a_dataset"
expect(configuration.dataset).to eq "a_dataset"
end
Expand Down
Loading