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.

Sign up for GitHub

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

Add automatic scrubbing for tracing #615

Merged
merged 1 commit into from
Mar 24, 2020
+72 −1
Merged

Add automatic scrubbing for tracing #615

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 16 additions & 1 deletion manifests/init.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,9 @@
# $apm_analyzed_spans
# Hash defining the APM spans to analyze and their rates.
# Optional Hash. Default: undef.
# $apm_obfuscation
# Hash defining obfuscation rules for sensitive data. (Agent 6 and 7 only).
# Optional Hash. Default: undef
# $process_enabled
# String to enable the process/container agent
# Boolean. Default: false
Expand Down Expand Up @@ -300,6 +303,7 @@
String $apm_env = 'none',
Boolean $apm_non_local_traffic = false,
Optional[Hash[String, Float[0, 1]]] $apm_analyzed_spans = undef,
Optional[Hash[String, Data]] $apm_obfuscation = undef,
Boolean $process_enabled = $datadog_agent::params::process_default_enabled,
Boolean $scrub_args = $datadog_agent::params::process_default_scrub_args,
Array $custom_sensitive_words = $datadog_agent::params::process_default_custom_words,
Expand Down Expand Up @@ -527,7 +531,7 @@
}
}

if ($apm_enabled == true) and ($apm_env != 'none') or $apm_analyzed_spans {
if ($apm_enabled == true) and (($apm_env != 'none') or $apm_analyzed_spans or $apm_obfuscation) {
concat::fragment{ 'datadog apm footer':
target => '/etc/dd-agent/datadog.conf',
content => template('datadog_agent/datadog_apm_footer.conf.erb'),
Expand Down Expand Up @@ -606,6 +610,16 @@
$apm_analyzed_span_config = {}
}

if $apm_obfuscation {
$apm_obfuscation_config = {
'apm_config' => {
'obfuscation' => $apm_obfuscation
}
}
} else {
$apm_obfuscation_config = {}
}

if $statsd_forward_host != '' {
if $_statsd_forward_port != '' {
$statsd_forward_config = {
Expand Down Expand Up @@ -634,6 +648,7 @@
$logs_base_config,
$agent_extra_options,
$apm_analyzed_span_config,
$apm_obfuscation_config,
$statsd_forward_config,
$host_config,
$additional_checksd_config)
Expand Down
56 changes: 56 additions & 0 deletions spec/classes/datadog_agent_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1909,6 +1909,62 @@
)
}
end

context 'with apm_enabled set to true and apm_obfuscation specified' do
Copy link
Member

Choose a reason for hiding this comment

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

nit: we're missing a similar test for A5 to test the ini-style file generation (the conf footers), you might feel free to ignore, but just pointing it out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't pass this config to A5 at all, so there should be no change for A5. These settings end up in $agent_config which is only used when generating A6/7 config.

Copy link
Member

Choose a reason for hiding this comment

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

Then why have changes to templates/datadog_apm_footer.conf.erb at all? I guess we can remove that, that's for A5 config.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad, it is used by that template. I'll add a test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually the template is not working since the nested objects aren't output in ini format. I'm just going to remove it and not support this on A5, which I think is reasonable.

let(:params) do
{
apm_enabled: true,
apm_obfuscation: {
elasticsearch: {
enable: true,
keep_values: [
'user_id',
'category_id',
],
},
redis: {
enable: true,
},
memcached: {
enable: true,
},
http: {
remove_query_string: true,
remove_paths_with_digits: true,
},
mongodb: {
enable: true,
keep_values: [
'uid',
'cat_id',
],
},
},
}
end

it {
is_expected.to contain_file(config_yaml_file).with(
'content' => %r{^apm_config:\n},
)
}
it {
is_expected.to contain_file(config_yaml_file).with(
'content' => %r{^apm_config:\n\ \ enabled: true\n},
)
}
it {
is_expected.to contain_file(config_yaml_file).with(
'content' => %r{^\ \ obfuscation:\n},
)
}
it {
is_expected.to contain_file(config_yaml_file).with(
'content' => %r{elasticsearch},
)
}
end

context 'with extra_options and Process enabled' do
let(:params) do
{
Expand Down