-
Notifications
You must be signed in to change notification settings - Fork 397
swap out the existing headers normalization logic with the tag normalizer #5041
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
Closed
Closed
Changes from 15 commits
Commits
Show all changes
68 commits
Select commit
Hold shift + click to select a range
1d8bab2
Add initial attempt at adding process related tags on trace payloads.…
wantsui 58592a3
Add test for multiple calls to the formatter tags
wantsui 7dc9184
Add tests for trace formatter spec to assert that the first span of t…
wantsui cad26a6
it turns out you cannot just pin things to rails 7 due to newer ruby …
wantsui f31440a
Update lib/datadog/core/environment/process.rb
wantsui cfec602
fix string and rename formatted_process_tags_k1_v1 to serialized
wantsui 8dae705
remove unneeded line
wantsui 055586f
remove server type for now until more research is done
wantsui cacb500
Add new tag normalizer logic following the trace agent.
wantsui 7661a3f
lint fix
wantsui 7825940
add missing files from prototype command
wantsui 5de6efd
Add missing constants to ext rbs file
wantsui f5ca84a
jruby fix for the process spec
wantsui 9ad5be5
remove the active record during rails creation because it caused a jr…
wantsui ceaf7d4
Swap out the existing headers normalization logic with the tag normal…
wantsui 0848833
update the comments for the test
wantsui 9fc4009
fix another comment
wantsui a66e635
Bring tag normalization to 1:1 parity with the Trace Agent
wantsui ec1e930
Add changes from code review around comments and add test for the new…
wantsui 4073ab5
Merge branch 'master' into add-process-tags-to-tracing
wantsui 22a3680
Remove the rails gem install from process_spec
wantsui 5784833
Remove 1 sec delay.
wantsui 2b705e3
Update sig/datadog/core/environment/ext.rbs
wantsui e3deb4c
Update lib/datadog/tracing/transport/trace_formatter.rb
wantsui 4747259
Add improvements for long strings.
wantsui 41bc6c0
small improvement to the whitespace removal.
wantsui c3605c0
Add upper bound to regex to avoid the polynomial regex on uncontrolle…
wantsui adfa416
Change untyped to string.
wantsui 0dff545
Use possessive quantifiers in regex instead of limiting the upper bou…
wantsui 7d8da40
Fix types for steep check command
wantsui 31d9796
Remove unneeded Core prefix
wantsui 3672a8a
lint fixes
wantsui 23d9769
restructure folder lookup so it works on the macos ci tests
wantsui 7615906
fixes for local mac development.
wantsui d4c6a91
Add missing trace agent test cases.
wantsui 433b250
Fix lint
wantsui 47efb90
Change methods to private. Also add comments with examples
wantsui a2643a6
Fix basedir logic and adjust tests (and also fix the private change)
wantsui ccd4971
Fix steepcheck error
wantsui be9587d
Add in byte logic to handle emojis with early backoff and allow start…
wantsui 6042830
Move process tags only to the first span and adjust tests
wantsui 4210d74
Add a special character into the test app name to show that it gets n…
wantsui f9af946
Update lib/datadog/core/normalizer.rb
wantsui 381fbe2
Update lib/datadog/core/normalizer.rb
wantsui 138dff8
Fixes for new constant names
wantsui 2449153
Change to byteslice
wantsui 5252259
fix lint.
wantsui 0eaf302
remove process_spec from main rake task
wantsui fbfecfe
Update spec/datadog/core/normalizer_spec.rb
wantsui cc2225f
Update spec/datadog/tracing/transport/trace_formatter_spec.rb
wantsui 62822ab
Merge branch 'master' into add-process-tags-to-tracing
wantsui a77e63b
Remove the unless check and replace with an assertion that the file e…
wantsui 439e81a
Update spec/datadog/core/environment/process_spec.rb
wantsui 9e45ade
fix lint
wantsui 0ab4fef
Rename Normalizer to TagNormalizer.
wantsui 6577d3f
Update lib/datadog/core/environment/process.rb
wantsui a336c66
Add api private comment to the tag normalizer and refactor away the e…
wantsui 0d229de
Fix steep errors on the process rbs file
wantsui e83bc4a
Refactor the utils encode call so it can be used in the tag normalize…
wantsui ce1759f
Update Rakefile
wantsui 8b978c6
Update lib/datadog/core/tag_normalizer.rb
wantsui f4c9d49
Add lint fixes and remove unneeded regex at the end.
wantsui eae4eb9
fix rbs file for deleted variable
wantsui f3f1480
remove unneeded conditional
wantsui b48d20d
Add a log if the process tags cannot be obtained
wantsui 3d33291
Fix regex and reuse the same test cases to show that the leading digi…
wantsui 4e3f8f4
Attempt to retrieve as many non empty string process tags as possible…
wantsui 2f4d1c6
Merge branch 'add-process-tags-to-tracing' into replace-headers-tags-…
wantsui File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require_relative 'ext' | ||
| require_relative '../normalizer' | ||
|
|
||
| module Datadog | ||
| module Core | ||
| module Environment | ||
| # Retrieves process level information | ||
| module Process | ||
| module_function | ||
|
|
||
| def entrypoint_workdir | ||
| File.basename(Dir.pwd) | ||
| end | ||
|
|
||
| def entrypoint_type | ||
| Core::Environment::Ext::PROCESS_TYPE | ||
| end | ||
|
|
||
| def entrypoint_name | ||
| File.basename($0) | ||
| end | ||
|
|
||
| def entrypoint_basedir | ||
| current_basedir = File.expand_path(File.dirname($0)) | ||
| normalized_basedir = current_basedir.tr(File::SEPARATOR, '/') | ||
| normalized_basedir.delete_prefix!('/') | ||
| end | ||
|
|
||
| # Normalize tag key and value using the Datadog Agent's tag normalization logic | ||
| def serialized_kv_helper(key, value) | ||
| key = Core::Normalizer.normalize(key) | ||
| value = Core::Normalizer.normalize(value) | ||
| "#{key}:#{value}" | ||
| end | ||
|
|
||
| # This method returns a key/value part of serialized tags in the format of k1:v1,k2:v2,k3:v3 | ||
| def serialized | ||
| return @serialized if defined?(@serialized) | ||
| tags = [] | ||
| tags << serialized_kv_helper(Core::Environment::Ext::TAG_ENTRYPOINT_WORKDIR, entrypoint_workdir) if entrypoint_workdir | ||
| tags << serialized_kv_helper(Core::Environment::Ext::TAG_ENTRYPOINT_NAME, entrypoint_name) if entrypoint_name | ||
| tags << serialized_kv_helper(Core::Environment::Ext::TAG_ENTRYPOINT_BASEDIR, entrypoint_basedir) if entrypoint_basedir | ||
| tags << serialized_kv_helper(Core::Environment::Ext::TAG_ENTRYPOINT_TYPE, entrypoint_type) if entrypoint_type | ||
| @serialized = tags.join(',').freeze | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module Datadog | ||
| module Core | ||
| module Normalizer | ||
| module_function | ||
|
|
||
| INVALID_TAG_CHARACTERS = %r{[^a-z0-9_\-:./]}.freeze | ||
|
|
||
| # Based on https://docs.datadoghq.com/getting_started/tagging/#defining-tags | ||
| # Currently a reimplementation of the logic in the | ||
| # Datadog::Tracing::Metadata::Ext::HTTP::Headers.to_tag method with some additional items | ||
| # TODO: Swap out the logic in the Datadog Tracing Metadata headers logic | ||
| def self.normalize(original_value) | ||
| return "" if original_value.nil? || original_value.to_s.strip.empty? | ||
|
|
||
| # Removes whitespaces | ||
| normalized_value = original_value.to_s.strip | ||
| # Lower case characters | ||
| normalized_value.downcase! | ||
| # Invalid characters are replaced with an underscore | ||
| normalized_value.gsub!(INVALID_TAG_CHARACTERS, '_') | ||
| # Merge consecutive underscores with a single underscore | ||
| normalized_value.squeeze!('_') | ||
| # Remove leading non-letter characters | ||
| normalized_value.sub!(/\A[^a-z]+/, "") | ||
| # Maximum length is 200 characters | ||
| normalized_value = normalized_value[0...200] if normalized_value.length > 200 | ||
|
|
||
| normalized_value | ||
| end | ||
| end | ||
| end | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| module Datadog | ||
| module Core | ||
| module Environment | ||
| module Process | ||
| @serialized: untyped | ||
|
|
||
| def self?.entrypoint_workdir: () -> untyped | ||
|
|
||
| def self?.entrypoint_type: () -> untyped | ||
|
|
||
| def self?.entrypoint_name: () -> untyped | ||
|
|
||
| def self?.entrypoint_basedir: () -> untyped | ||
| def self?.serialized_kv_helper: (untyped key, untyped value) -> ::String | ||
| def self?.serialized: () -> untyped | ||
| end | ||
| end | ||
| end | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| module Datadog | ||
| module Core | ||
| module Normalizer | ||
| INVALID_TAG_CHARACTERS: ::Regexp | ||
| def self.normalize: (untyped original_value) -> ("" | untyped) | ||
| end | ||
| end | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| require 'spec_helper' | ||
| require 'datadog/core/environment/process' | ||
| require 'open3' | ||
|
|
||
| RSpec.describe Datadog::Core::Environment::Process do | ||
| describe '::entrypoint_workdir' do | ||
| subject(:entrypoint_workdir) { described_class.entrypoint_workdir } | ||
|
|
||
| it { is_expected.to be_a_kind_of(String) } | ||
| end | ||
|
|
||
| describe '::entrypoint_type' do | ||
| subject(:entrypoint_type) { described_class.entrypoint_type } | ||
|
|
||
| it { is_expected.to be_a_kind_of(String) } | ||
| it { is_expected.to eq(Datadog::Core::Environment::Ext::PROCESS_TYPE) } | ||
| end | ||
|
|
||
| describe '::entrypoint_name' do | ||
| subject(:entrypoint_name) { described_class.entrypoint_name } | ||
|
|
||
| it { is_expected.to be_a_kind_of(String) } | ||
| end | ||
|
|
||
| describe '::entrypoint_basedir' do | ||
| subject(:entrypoint_basedir) { described_class.entrypoint_basedir } | ||
|
|
||
| it { is_expected.to be_a_kind_of(String) } | ||
| end | ||
|
|
||
| describe '::serialized' do | ||
| subject(:serialized) { described_class.serialized } | ||
|
|
||
| it { is_expected.to be_a_kind_of(String) } | ||
|
|
||
| it 'returns the same object when called multiple times' do | ||
| # Processes are fixed so no need to recompute this on each call | ||
| first_call = described_class.serialized | ||
| second_call = described_class.serialized | ||
| expect(first_call).to equal(second_call) | ||
| end | ||
| end | ||
|
|
||
| describe 'Scenario: Real applications' do | ||
| context 'when running a real Rails application' do | ||
| it 'detects Rails process information correctly' do | ||
| Dir.mktmpdir do |tmp_dir| | ||
| Dir.chdir(tmp_dir) do | ||
| Bundler.with_unbundled_env do | ||
| skip('rails gem could not be installed') unless system('gem install rails') | ||
| unless system('rails new test_app --minimal --skip-active-record --skip-test --skip-keeps --skip-git --skip-docker') | ||
| skip('rails new command failed') | ||
| end | ||
| end | ||
| end | ||
| File.open("#{tmp_dir}/test_app/Gemfile", 'a') do |file| | ||
| file.puts "gem 'datadog', path: '#{Dir.pwd}', require: false" | ||
| end | ||
| File.write("#{tmp_dir}/test_app/config/initializers/process_initializer.rb", <<-RUBY) | ||
| Rails.application.config.after_initialize do | ||
| require 'datadog/core/environment/process' | ||
| STDERR.puts "entrypoint_workdir:\#{Datadog::Core::Environment::Process.entrypoint_workdir}" | ||
| STDERR.puts "entrypoint_type:\#{Datadog::Core::Environment::Process.entrypoint_type}" | ||
| STDERR.puts "entrypoint_name:\#{Datadog::Core::Environment::Process.entrypoint_name}" | ||
| STDERR.puts "entrypoint_basedir:\#{Datadog::Core::Environment::Process.entrypoint_basedir}" | ||
| STDERR.puts "_dd.tags.process:\#{Datadog::Core::Environment::Process.serialized}" | ||
| STDERR.flush | ||
| Thread.new { sleep 1; Process.kill('TERM', Process.pid)}#{" "} | ||
| end | ||
| RUBY | ||
| Bundler.with_unbundled_env do | ||
| Dir.chdir("#{tmp_dir}/test_app") do | ||
| _, _, _ = Open3.capture3('bundle install') | ||
| _, err, _ = Open3.capture3('bundle exec rails s') | ||
| expect(err).to include('entrypoint_workdir:test_app') | ||
| expect(err).to include('entrypoint_type:script') | ||
| expect(err).to include('entrypoint_name:rails') | ||
| basedir_test = tmp_dir.sub(%r{^/}, '') | ||
| expect(err).to include("entrypoint_basedir:#{basedir_test}/test_app/bin") | ||
| expected_tags = "entrypoint.workdir:test_app,entrypoint.name:rails,entrypoint.basedir:#{basedir_test}/test_app/bin,entrypoint.type:script" | ||
| expect(err).to include("_dd.tags.process:#{expected_tags}") | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| require 'spec_helper' | ||
| require 'datadog/core/normalizer' | ||
|
|
||
| RSpec.describe Datadog::Core::Normalizer do | ||
| describe '.normalize' do | ||
| subject(:normalize) { described_class.normalize(input) } | ||
|
|
||
| context 'keeps normal strings the same' do | ||
| let(:input) { 'regulartag' } | ||
| let(:expected_output) { 'regulartag' } | ||
| it { is_expected.to eq(expected_output) } | ||
| end | ||
|
|
||
| context 'truncates long strings' do | ||
| let(:input) { 'a' * 201 } | ||
| let(:expected_output) { 'a' * 200 } | ||
| it { is_expected.to eq(expected_output) } | ||
| end | ||
|
|
||
| context 'transforms special characters to underscores' do | ||
| let(:input) { 'a&**!' } | ||
| let(:expected_output) { 'a_' } | ||
| it { is_expected.to eq(expected_output) } | ||
| end | ||
|
|
||
| context 'capital letters are lower cased' do | ||
| let(:input) { 'A' * 10 } | ||
| let(:expected_output) { 'a' * 10 } | ||
| it { is_expected.to eq(expected_output) } | ||
| end | ||
|
|
||
| context 'removes whitespaces' do | ||
| let(:input) { ' hi ' } | ||
| let(:expected_output) { 'hi' } | ||
| it { is_expected.to eq(expected_output) } | ||
| end | ||
|
|
||
| context 'characters must start with a letter' do | ||
| let(:input) { '1hi' } | ||
| let(:expected_output) { 'hi' } | ||
| it { is_expected.to eq(expected_output) } | ||
| end | ||
|
|
||
| context 'if none of the characters are valid to start the value, the string is empty' do | ||
| let(:input) { '111111111' } | ||
| let(:expected_output) { '' } | ||
| it { is_expected.to eq(expected_output) } | ||
| end | ||
| end | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When changing the logic -- please update the test description as well! It'd be quite confusing to have a test saying "it replaces each with an underscore" that actually is not true :P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh! good catch, I'll use a better description for this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the description but open to feedback on the wording !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Just a heads up I'm still changing the normalization logic in #5033, so I'll circle back to this PR after the process tags PR to see if I need to update these test assertions.