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

Change profiling HTTP request runtime and type fields #1166

Merged
merged 5 commits into from
Sep 9, 2020
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
6 changes: 4 additions & 2 deletions lib/ddtrace/ext/profiling.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,14 @@ module HTTP
FORM_FIELD_TAG_LANGUAGE = 'language'.freeze
FORM_FIELD_TAG_PROFILER_VERSION = 'profiler_version'.freeze
FORM_FIELD_TAG_RUNTIME = 'runtime'.freeze
FORM_FIELD_TAG_RUNTIME_ENGINE = 'runtime_engine'.freeze
FORM_FIELD_TAG_RUNTIME_PLATFORM = 'runtime_platform'.freeze
FORM_FIELD_TAG_RUNTIME_VERSION = 'runtime_version'.freeze
FORM_FIELD_TAG_SERVICE = 'service'.freeze
FORM_FIELD_TAG_VERSION = 'version'.freeze
FORM_FIELD_TAGS = 'tags'.freeze
FORM_FIELD_TYPES = 'types'.freeze
FORM_FIELD_TYPE_CPU_TIME = 'cpu_time'.freeze
FORM_FIELD_TYPES = 'types[0]'.freeze
FORM_FIELD_TYPES_AUTO = 'auto'.freeze

HEADER_CONTENT_TYPE = 'Content-Type'.freeze
HEADER_CONTENT_TYPE_OCTET_STREAM = 'application/octet-stream'.freeze
Expand Down
2 changes: 2 additions & 0 deletions lib/ddtrace/ext/runtime.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ module Ext
module Runtime
# Identity
LANG = 'ruby'.freeze
LANG_ENGINE = RUBY_ENGINE
LANG_INTERPRETER = (RUBY_ENGINE + '-' + RUBY_PLATFORM).freeze
LANG_PLATFORM = RUBY_PLATFORM
LANG_VERSION = RUBY_VERSION
TRACER_VERSION = Datadog::VERSION::STRING

Expand Down
6 changes: 4 additions & 2 deletions lib/ddtrace/profiling/flush.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ module Profiling
:version,
:host,
:language,
:runtime,
:runtime_engine,
:runtime_platform,
:runtime_version,
:profiler_version
) do
Expand All @@ -27,7 +28,8 @@ def initialize(*args)
self.version = version || Datadog.configuration.version
self.host = host || Datadog::Runtime::Socket.hostname
self.language = language || Datadog::Runtime::Identity.lang
self.runtime = runtime || Datadog::Runtime::Identity.lang_interpreter
self.runtime_engine = runtime_engine || Datadog::Runtime::Identity.lang_engine
self.runtime_platform = runtime_platform || Datadog::Runtime::Identity.lang_platform
Copy link
Member

Choose a reason for hiding this comment

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

We might want to report these in the tracer payload as well. Nothing to do in this PR, just a reminder for our future selves.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good idea. In the tracer payload I think we send them as "interpreter" which is both terms combined.

self.runtime_version = runtime_version || Datadog::Runtime::Identity.lang_version
self.profiler_version = profiler_version || Datadog::Runtime::Identity.tracer_version
end
Expand Down
20 changes: 7 additions & 13 deletions lib/ddtrace/profiling/transport/http/api/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@ module API
class Endpoint < Datadog::Transport::HTTP::API::Endpoint
include Datadog::Ext::Profiling::Transport::HTTP

TYPE_MAPPINGS = {
cpu_time_ns: 'ruby-cpu'.freeze
}.freeze

attr_reader \
:encoder

Expand Down Expand Up @@ -46,19 +42,22 @@ def build_form(env)
FORM_FIELD_RECORDING_START => flush.start.utc.iso8601,
FORM_FIELD_RECORDING_END => flush.finish.utc.iso8601,
FORM_FIELD_TAGS => [
"#{FORM_FIELD_TAG_RUNTIME}:#{flush.runtime}",
"#{FORM_FIELD_TAG_RUNTIME}:#{flush.language}",
"#{FORM_FIELD_TAG_RUNTIME_ENGINE}:#{flush.runtime_engine}",
"#{FORM_FIELD_TAG_RUNTIME_PLATFORM}:#{flush.runtime_platform}",
"#{FORM_FIELD_TAG_RUNTIME_VERSION}:#{flush.runtime_version}",
"#{FORM_FIELD_TAG_PROFILER_VERSION}:#{flush.profiler_version}",
# NOTE: Redundant w/ 'runtime'; may want to remove this later.
"#{FORM_FIELD_TAG_LANGUAGE}:#{flush.language}",
"#{FORM_FIELD_TAG_HOST}:#{flush.host}"
],
FORM_FIELD_DATA => pprof_file,
FORM_FIELD_RUNTIME => flush.runtime,
FORM_FIELD_RUNTIME => flush.language,
FORM_FIELD_FORMAT => FORM_FIELD_FORMAT_PPROF
}

# Add types
form['types[0]'] = types.join(',')
form[FORM_FIELD_TYPES] = types.join(',')

# Optional fields
form[FORM_FIELD_TAGS] << "#{FORM_FIELD_TAG_SERVICE}:#{flush.service}" unless flush.service.nil?
Expand All @@ -71,11 +70,6 @@ def build_form(env)
def build_pprof(flush)
pprof = encoder.encode(flush)

# Convert types
types = pprof.types.map do |type|
TYPE_MAPPINGS.key?(type) ? TYPE_MAPPINGS[type] : type.to_s
end

# Wrap pprof as a gzipped file
gzipped_data = Datadog::Utils::Compression.gzip(pprof.data)
pprof_file = Datadog::Vendor::Multipart::Post::UploadIO.new(
Expand All @@ -84,7 +78,7 @@ def build_pprof(flush)
PPROF_DEFAULT_FILENAME
)

[pprof_file, types]
[pprof_file, [FORM_FIELD_TYPES_AUTO]]
end
end
end
Expand Down
8 changes: 8 additions & 0 deletions lib/ddtrace/runtime/identity.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,18 @@ def lang
Ext::Runtime::LANG
end

def lang_engine
Ext::Runtime::LANG_ENGINE
end

def lang_interpreter
Ext::Runtime::LANG_INTERPRETER
end

def lang_platform
Ext::Runtime::LANG_PLATFORM
end

def lang_version
Ext::Runtime::LANG_VERSION
end
Expand Down
81 changes: 81 additions & 0 deletions spec/ddtrace/profiling/flush_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
RSpec.describe Datadog::Profiling::Flush do
describe '#new' do
context 'given no arguments' do
subject(:identifier) { described_class.new }

it do
is_expected.to have_attributes(
start: nil,
finish: nil,
event_groups: nil,
event_count: nil,
runtime_id: Datadog::Runtime::Identity.id,
service: Datadog.configuration.service,
env: Datadog.configuration.env,
version: Datadog.configuration.version,
host: Datadog::Runtime::Socket.hostname,
language: Datadog::Runtime::Identity.lang,
runtime_engine: Datadog::Runtime::Identity.lang_engine,
runtime_platform: Datadog::Runtime::Identity.lang_platform,
runtime_version: Datadog::Runtime::Identity.lang_version,
profiler_version: Datadog::Runtime::Identity.tracer_version
)
end
end

context 'given full arguments' do
subject(:identifier) do
described_class.new(
start,
finish,
event_groups,
event_count,
runtime_id,
service,
env,
version,
host,
language,
runtime_engine,
runtime_platform,
runtime_version,
profiler_version
)
end

let(:start) { double('start') }
let(:finish) { double('finish') }
let(:event_groups) { double('event_groups') }
let(:event_count) { double('event_count') }
let(:runtime_id) { double('runtime_id') }
let(:service) { double('service') }
let(:env) { double('env') }
let(:version) { double('version') }
let(:host) { double('host') }
let(:language) { double('language') }
let(:runtime_engine) { double('runtime_engine') }
let(:runtime_platform) { double('runtime_platform') }
let(:runtime_version) { double('runtime_version') }
let(:profiler_version) { double('profiler_version') }

it do
is_expected.to have_attributes(
start: start,
finish: finish,
event_groups: event_groups,
event_count: event_count,
runtime_id: runtime_id,
service: service,
env: env,
version: version,
host: host,
language: language,
runtime_engine: runtime_engine,
runtime_platform: runtime_platform,
runtime_version: runtime_version,
profiler_version: profiler_version
)
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -87,16 +87,18 @@
'recording-start' => kind_of(String),
'recording-end' => kind_of(String),
'data[0]' => kind_of(String),
'types[0]' => /ruby-cpu(,wall_time_ns)?/, # TODO: Replace
'runtime' => Datadog::Ext::Runtime::LANG_INTERPRETER,
'types[0]' => /auto/,
'runtime' => Datadog::Ext::Runtime::LANG,
'format' => Datadog::Ext::Profiling::Transport::HTTP::FORM_FIELD_FORMAT_PPROF
)

# rubocop:disable Metrics/LineLength
tags = body["#{Datadog::Ext::Profiling::Transport::HTTP::FORM_FIELD_TAGS}[]"].list
expect(tags).to be_a_kind_of(Array)
expect(tags).to include(
/#{Datadog::Ext::Profiling::Transport::HTTP::FORM_FIELD_TAG_RUNTIME}:#{Datadog::Ext::Runtime::LANG_INTERPRETER}/,
/#{Datadog::Ext::Profiling::Transport::HTTP::FORM_FIELD_TAG_RUNTIME}:#{Datadog::Ext::Runtime::LANG}/,
/#{Datadog::Ext::Profiling::Transport::HTTP::FORM_FIELD_TAG_RUNTIME_ENGINE}:#{Datadog::Ext::Runtime::LANG_ENGINE}/,
/#{Datadog::Ext::Profiling::Transport::HTTP::FORM_FIELD_TAG_RUNTIME_PLATFORM}:#{Datadog::Ext::Runtime::LANG_PLATFORM}/,
/#{Datadog::Ext::Profiling::Transport::HTTP::FORM_FIELD_TAG_RUNTIME_VERSION}:#{Datadog::Ext::Runtime::LANG_VERSION}/,
/#{Datadog::Ext::Profiling::Transport::HTTP::FORM_FIELD_TAG_PROFILER_VERSION}:#{Datadog::Ext::Runtime::TRACER_VERSION}/,
/#{Datadog::Ext::Profiling::Transport::HTTP::FORM_FIELD_TAG_LANGUAGE}:#{Datadog::Ext::Runtime::LANG}/
Expand Down
8 changes: 5 additions & 3 deletions spec/ddtrace/profiling/transport/http/api/endpoint_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,12 @@
Datadog::Ext::Profiling::Transport::HTTP::FORM_FIELD_FORMAT => Datadog::Ext::Profiling::Transport::HTTP::FORM_FIELD_FORMAT_PPROF,
Datadog::Ext::Profiling::Transport::HTTP::FORM_FIELD_RECORDING_START => flush.start.utc.iso8601,
Datadog::Ext::Profiling::Transport::HTTP::FORM_FIELD_RECORDING_END => flush.finish.utc.iso8601,
Datadog::Ext::Profiling::Transport::HTTP::FORM_FIELD_RUNTIME => flush.runtime,
Datadog::Ext::Profiling::Transport::HTTP::FORM_FIELD_RUNTIME => flush.language,
Datadog::Ext::Profiling::Transport::HTTP::FORM_FIELD_RUNTIME_ID => flush.runtime_id,
Datadog::Ext::Profiling::Transport::HTTP::FORM_FIELD_TAGS => array_including(
"#{Datadog::Ext::Profiling::Transport::HTTP::FORM_FIELD_TAG_RUNTIME}:#{flush.runtime}",
"#{Datadog::Ext::Profiling::Transport::HTTP::FORM_FIELD_TAG_RUNTIME}:#{flush.language}",
"#{Datadog::Ext::Profiling::Transport::HTTP::FORM_FIELD_TAG_RUNTIME_ENGINE}:#{flush.runtime_engine}",
"#{Datadog::Ext::Profiling::Transport::HTTP::FORM_FIELD_TAG_RUNTIME_PLATFORM}:#{flush.runtime_platform}",
"#{Datadog::Ext::Profiling::Transport::HTTP::FORM_FIELD_TAG_RUNTIME_VERSION}:#{flush.runtime_version}",
"#{Datadog::Ext::Profiling::Transport::HTTP::FORM_FIELD_TAG_PROFILER_VERSION}:#{flush.profiler_version}",
"#{Datadog::Ext::Profiling::Transport::HTTP::FORM_FIELD_TAG_LANGUAGE}:#{flush.language}",
Expand Down Expand Up @@ -111,7 +113,7 @@
it 'includes env tags' do
call
expect(env.form).to include(
'types[0]' => 'wall_time_ns,ruby-cpu'
Datadog::Ext::Profiling::Transport::HTTP::FORM_FIELD_TYPES => Datadog::Ext::Profiling::Transport::HTTP::FORM_FIELD_TYPES_AUTO
)
end
end
Expand Down
30 changes: 30 additions & 0 deletions spec/ddtrace/runtime/identity_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,34 @@
end
end
end

describe '::lang' do
subject(:lang) { described_class.lang }
it { is_expected.to eq(Datadog::Ext::Runtime::LANG) }
end

describe '::lang_engine' do
subject(:lang_engine) { described_class.lang_engine }
it { is_expected.to eq(Datadog::Ext::Runtime::LANG_ENGINE) }
end

describe '::lang_interpreter' do
subject(:lang_interpreter) { described_class.lang_interpreter }
it { is_expected.to eq(Datadog::Ext::Runtime::LANG_INTERPRETER) }
end

describe '::lang_platform' do
subject(:lang_platform) { described_class.lang_platform }
it { is_expected.to eq(Datadog::Ext::Runtime::LANG_PLATFORM) }
end

describe '::lang_version' do
subject(:lang_version) { described_class.lang_version }
it { is_expected.to eq(Datadog::Ext::Runtime::LANG_VERSION) }
end

describe '::tracer_version' do
subject(:tracer_version) { described_class.tracer_version }
it { is_expected.to eq(Datadog::Ext::Runtime::TRACER_VERSION) }
end
end