Skip to content
Open
Show file tree
Hide file tree
Changes from 4 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
10 changes: 6 additions & 4 deletions lib/datadog/core/process_discovery.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# frozen_string_literal: true

require 'datadog/core/process_discovery/tracer_memfd'
require_relative 'process_discovery/tracer_memfd'
require_relative 'environment/process'
require_relative 'environment/container'

module Datadog
module Core
Expand Down Expand Up @@ -42,9 +44,9 @@ def get_metadata(settings)
service_name: settings.service || '',
service_env: settings.env || '',
service_version: settings.version || '',
# TODO: Implement process tags and container id
process_tags: '',
container_id: ''
# Follows Java: https://github.com/DataDog/dd-trace-java/blob/2ebc964340ac530342cc389ba68ff0f5070d5f9f/dd-trace-core/src/main/java/datadog/trace/core/servicediscovery/ServiceDiscovery.java#L37-L38
process_tags: settings.experimental_propagate_process_tags_enabled ? Core::Environment::Process.serialized : '',
container_id: Core::Environment::Container.container_id || ''
}
end
end
Expand Down
128 changes: 98 additions & 30 deletions spec/datadog/core/process_discovery_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,40 +28,68 @@
end

context 'when libdatadog API is available' do
context 'with all settings provided' do
before do
Datadog.configure do |c|
c.service = 'test-service' # Manually set so it isn't set to fallback service name that we don't control
end
before do
Datadog.configure do |c|
c.service = 'dummy-service' # Manually set so it isn't set to fallback service name that we don't control
end
end

after do
Datadog.configuration.reset!
end
after do
Datadog.configuration.reset!
end

context 'with all settings provided' do
let(:settings) do
instance_double(
'Datadog::Core::Configuration::Setting',
service: 'test-service',
env: 'test-env',
version: '1.0.0'
version: '1.0.0',
experimental_propagate_process_tags_enabled: propagate_process_tags
)
end

it 'stores metadata successfully' do
described_class.publish(settings)
context 'when process tags are enabled' do
let(:propagate_process_tags) { true }

expect(content).to include(
{
it 'stores metadata successfully' do
described_class.publish(settings)

expect(content).to include(
'runtime_id' => Datadog::Core::Environment::Identity.id,
'tracer_language' => Datadog::Core::Environment::Identity.lang,
'tracer_version' => Datadog::Core::Environment::Identity.gem_datadog_version_semver2,
'hostname' => Datadog::Core::Environment::Socket.hostname,
'service_name' => 'test-service',
'service_env' => 'test-env',
'service_version' => '1.0.0'
}
)
'service_version' => '1.0.0',
'process_tags' => Datadog::Core::Environment::Process.serialized
)
end
end

context 'when process tags are disabled' do
let(:propagate_process_tags) { false }

it 'does not include process_tags in metadata' do
described_class.publish(settings)

expect(content).to include('process_tags' => '')
end
end

context 'when running in a containerized environment' do
let(:propagate_process_tags) { true }

before do
allow(Datadog::Core::Environment::Container).to receive(:container_id).and_return('container-id-1')
end

it 'includes container_id in metadata' do
described_class.publish(settings)

expect(content).to include('container_id' => 'container-id-1')
end
end
end

Expand All @@ -71,7 +99,8 @@
'Datadog::Core::Configuration::Setting',
service: nil,
env: nil,
version: nil
version: nil,
experimental_propagate_process_tags_enabled: true
)
end

Expand All @@ -81,12 +110,11 @@
# If the string is empty, it should be replaced by None when converting C strings to Rust types.
# Thus not appearing in the content.
expect(content).to include(
{
'runtime_id' => Datadog::Core::Environment::Identity.id,
'tracer_language' => Datadog::Core::Environment::Identity.lang,
'tracer_version' => Datadog::Core::Environment::Identity.gem_datadog_version_semver2,
'hostname' => Datadog::Core::Environment::Socket.hostname
}
'runtime_id' => Datadog::Core::Environment::Identity.id,
'tracer_language' => Datadog::Core::Environment::Identity.lang,
'tracer_version' => Datadog::Core::Environment::Identity.gem_datadog_version_semver2,
'hostname' => Datadog::Core::Environment::Socket.hostname,
'process_tags' => Datadog::Core::Environment::Process.serialized
)
end
end
Expand All @@ -99,6 +127,7 @@
before do
Datadog.configure do |c|
c.service = 'test-service' # Manually set so it isn't set to fallback service name that we don't control
c.experimental_propagate_process_tags_enabled = true
end
end

Expand All @@ -114,16 +143,55 @@
expect_in_fork do
expect(described_class).to have_received(:publish)
expect(content).to include(
{
'runtime_id' => Datadog::Core::Environment::Identity.id,
'tracer_language' => Datadog::Core::Environment::Identity.lang,
'tracer_version' => Datadog::Core::Environment::Identity.gem_datadog_version_semver2,
'hostname' => Datadog::Core::Environment::Socket.hostname,
'service_name' => 'test-service'
}
'runtime_id' => Datadog::Core::Environment::Identity.id,
'tracer_language' => Datadog::Core::Environment::Identity.lang,
'tracer_version' => Datadog::Core::Environment::Identity.gem_datadog_version_semver2,
'hostname' => Datadog::Core::Environment::Socket.hostname,
'service_name' => 'test-service',
'process_tags' => Datadog::Core::Environment::Process.serialized
)
expect(content.fetch('runtime_id')).to_not eq(parent_runtime_id)
end
end
end

describe 'with real configuration', skip: !LibdatadogHelpers.supported? do
before do
described_class.shutdown!
end

after do
Datadog.configuration.reset!
described_class.shutdown!
end

context 'when process tags are enabled' do
it 'includes process tags' do
Datadog.configure do |c|
c.service = 'test-service'
c.experimental_propagate_process_tags_enabled = true
end

expected_tags = Datadog::Core::Environment::Process.serialized

described_class.publish(Datadog.configuration)

expect(content).to include('process_tags' => expected_tags)
expect(expected_tags).not_to be_empty
end
Copy link
Member

Choose a reason for hiding this comment

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

It's a good practice, to leave in it only the part responsible for the assertion. And in RSpec we have tools to do so, here is a refactored way

      before do
        Datadog.configure do |c|
          c.service = 'test-service'
          c.experimental_propagate_process_tags_enabled = true
        end
      end
      
      let(:expected_tags) { Datadog::Core::Environment::Process.serialized }

      it 'includes process tags' do
        expect { described_class.publish(Datadog.configuration) }.to
          change { content }.to hash_including('process_tags' => expected_tags)
      end

also there are some downsides of the existing checks

  1. Expected tags are unknown to the input and they asserted to be verified
  2. No transition check

end

context 'when process tags are disabled' do
it 'excludes process tags' do
Datadog.configure do |c|
c.service = 'test-service'
c.experimental_propagate_process_tags_enabled = false
end

described_class.publish(Datadog.configuration)

expect(content).to include('process_tags' => '')
end
end
end
end
Loading