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

[activemq_xml] adding integration #521

Merged
merged 5 commits into from
May 14, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
132 changes: 132 additions & 0 deletions manifests/integrations/activemq_xml.pp
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
# Class: datadog_agent::integrations::activemq_xml
#
# This class will install the necessary configuration for the activemq_xml integration
#
# Parameters:
# $password
# The password for the datadog user
# $host
# The host activemq_xml is running on
# $dbname
# The activemq_xml database name
# $port
# The activemq_xml port number
# $username
# The username for the datadog user
# $ssl
# Boolean to enable SSL
# $use_psycopg2
Copy link
Contributor

Choose a reason for hiding this comment

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

docs copy-pasted from postgres integration :) and not updated.

# Boolean to flag connecting to activemq_xml with psycopg2 instead of pg8000.
# Warning, psycopg2 doesn't support ssl mode.
# $collect_function_metrics
# Boolean to enable collecting metrics regarding PL/pgSQL functions from pg_stat_user_functions.
# $collect_count_metrics
# Boolean to enable collecting count metrics, default value is True for backward compatibility but they might be slow,
# suggested value is False.
# $collect_activity_metrics
# Boolean to enable collecting metrics regarding transactions from pg_stat_activity, default value is False.
# Please make sure the user has sufficient privileges to read from pg_stat_activity before enabling this option.
# $collect_database_size_metrics
# Boolean to enable collecting database size metrics. Default value is True but they might be slow with large databases
# $collect_default_database
# Boolean to enable collecting statistics from the default database 'activemq_xml' in the check metrics, default to false
# $tags
# Optional array of tags
# $tables
# Track per relation/table metrics. Array of strings.
# Warning: this can collect lots of metrics per relation
# (10 + 10 per index)
# $tags
# Optional array of tags
# $custom_metrics
# A hash of custom metrics with the following keys - query, metrics,
# relation, descriptors. Refer to this guide for details on those fields:
# https://help.datadoghq.com/hc/en-us/articles/208385813-activemq_xml-custom-metric-collection-explained
#
# Sample Usage:
#
# class { 'datadog_agent::integrations::activemq_xml' :
# host => 'localhost',
# dbname => 'activemq_xml'
# username => 'datadog',
# password => 'some_pass',
# ssl => false,
# custom_metrics => {
# a_custom_query => {
# query => "select tag_column, %s from table",
# relation => false,
# metrics => {
# value_column => ["value_column.datadog.tag", "GAUGE"]
# },
# descriptors => [
# ["tag_column", "tag_column.datadog.tag"]
# ]
# }
# }
# }
#
# Hiera Usage:
#
# datadog_agent::integrations::activemq_xml::instances:
# - host: 'localhost'
# dbname: 'activemq_xml'
# username: 'datadog'
# password: 'some_pass'
# ssl: false
# custom_metrics:
# a_custom_query:
# query: 'select tag_column, %s from table'
# relation: false
# metrics:
# value_column: ["value_column.datadog.tag", "GAUGE"]
# descriptors:
# - ["tag_column", "tag_column.datadog.tag"]
#
class datadog_agent::integrations::activemq_xml(
String $url = 'http://localhost:8161',
Boolean $supress_errors = false,
Optional[String] $username = undef,
Optional[String] $password = undef,
Optional[Array[String]] $detailed_queues = [],
Optional[Array[String]] $detailed_topics = [],
Optional[Array[String]] $detailed_subscribers = [],
Optional[Array] $instances = undef,
) inherits datadog_agent::params {
include datadog_agent

$legacy_dst = "${datadog_agent::conf_dir}/activemq_xml.yaml"
if !$::datadog_agent::agent5_enable {
$dst = "${datadog_agent::conf6_dir}/activemq_xml.d/conf.yaml"
file { $legacy_dst:
ensure => 'absent'
}
} else {
$dst = $legacy_dst
}

if !$instances and $url {
$_instances = [{
'url' => $url,
'username' => $username,
'password' => $password,
'supress_errors' => $supress_errors,
'detailed_queues' => $detailed_queues,
'detailed_topics' => $detailed_topics,
'detailed_subscribers' => $detailed_subscribers,
}]
} elsif !$instances{
$_instances = []
} else {
$_instances = $instances
}

file { $dst:
ensure => file,
owner => $datadog_agent::params::dd_user,
group => $datadog_agent::params::dd_group,
mode => '0600',
content => template('datadog_agent/agent-conf.d/activemq_xml.yaml.erb'),
require => Package[$datadog_agent::params::package_name],
notify => Service[$datadog_agent::params::service_name],
}
}
117 changes: 117 additions & 0 deletions spec/classes/datadog_agent_integrations_activemq_xml_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
require 'spec_helper'

describe 'datadog_agent::integrations::activemq_xml' do
context 'supported agents - v5 and v6' do
agents = { '5' => true, '6' => false }
agents.each do |_, enabled|
let(:pre_condition) { "class {'::datadog_agent': agent5_enable => #{enabled}}" }
let(:facts) {{
operatingsystem: 'Ubuntu',
}}
if enabled
Copy link
Contributor

Choose a reason for hiding this comment

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

if is_agent_5 would be way more readable, since it's what really means. Why are we calling this enabled?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see it's written like this in every integration rspec, so it's probably better to keep it consistent unless we change it everywhere.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated it everywhere 👍

let(:conf_dir) { '/etc/dd-agent/conf.d' }
else
let(:conf_dir) { '/etc/datadog-agent/conf.d' }
end
let(:dd_user) { 'dd-agent' }
let(:dd_group) { 'root' }
let(:dd_package) { 'datadog-agent' }
let(:dd_service) { 'datadog-agent' }
if enabled
let(:conf_file) { "#{conf_dir}/activemq_xml.yaml" }
else
let(:conf_file) { "#{conf_dir}/activemq_xml.d/conf.yaml" }
end

context 'with default parameters' do
it { should compile }
end

context 'with password set' do
let(:params) {{
username: 'foo',
password: 'abc123',
}}

it { should compile.with_all_deps }
it { should contain_file(conf_file).with(
owner: dd_user,
group: dd_group,
mode: '0600',
)}
it { should contain_file(conf_file).that_requires("Package[#{dd_package}]") }
it { should contain_file(conf_file).that_notifies("Service[#{dd_service}]") }
it { should contain_file(conf_file).with_content(/username: foo/) }
it { should contain_file(conf_file).with_content(/password: abc123/) }

context 'with default parameters' do
it {
should contain_file(conf_file)
.with_content(%r{http://localhost:8161})
.with_content(%r{supress_errors: false})
.without_content(%r{detailed_queues:})
.without_content(%r{detailed_topics:})
.without_content(%r{detailed_subscribers:})
}
end

context 'with extra detailed parameters' do
let(:params) {{
supress_errors: true,
detailed_queues: %w(queue1 queue2),
detailed_topics: %w(topic1 topic2),
detailed_subscribers: %w(subscriber1 subscriber2),
}}
it {
should contain_file(conf_file)
.with_content(%r{http://localhost:8161})
.with_content(%r{supress_errors: true})
.with_content(%r{detailed_queues:.*\s+- queue1\s+- queue2})
.with_content(%r{detailed_topics:.*\s+- topic1\s+- topic2})
.with_content(%r{detailed_subscribers:.*\s+- subscriber1\s+- subscriber2})
}
end

context 'with instances set' do
let(:params) {{
instances: [
{
'url' => 'http://localhost:8161',
'username' => 'joe',
'password' => 'hunter1',
'detailed_queues' => %w(queue1 queue2),
'detailed_topics' => %w(topic1 topic2),
'detailed_subscribers' => %w(subscriber1 subscriber2),
},
{
'url' => 'http://remotehost:8162',
'username' => 'moe',
'password' => 'hunter2',
'detailed_queues' => %w(queue3 queue4),
'detailed_topics' => %w(topic3 topic4),
'detailed_subscribers' => %w(subscriber3 subscriber4),
},
],
}}
it {
should contain_file(conf_file)
.with_content(%r{url: http://localhost:8161})
.without_content(%r{supress_errors:})
.with_content(%r{username: joe})
.with_content(%r{password: hunter1})
.with_content(%r{detailed_queues:.*\s+- queue1\s+- queue2})
.with_content(%r{detailed_topics:.*\s+- topic1\s+- topic2})
.with_content(%r{detailed_subscribers:.*\s+- subscriber1\s+- subscriber2})
.with_content(%r{url: http://remotehost:8162})
.without_content(%r{supress_errors:})
.with_content(%r{username: moe})
.with_content(%r{password: hunter2})
.with_content(%r{detailed_queues:.*\s+- queue3\s+- queue4})
.with_content(%r{detailed_topics:.*\s+- topic3\s+- topic4})
.with_content(%r{detailed_subscribers:.*\s+- subscriber3\s+- subscriber4})
}
end
end
end
end
end
38 changes: 38 additions & 0 deletions templates/agent-conf.d/activemq_xml.yaml.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
### MANAGED BY PUPPET

init_config:

instances:
<%- (Array(@_instances)).each do |instance| -%>
- url: <%= instance['url'] %>
<% if !instance['username'].nil? -%>
username: <%= instance['username'] %>
<%- end -%>
<% if !instance['password'].nil? -%>
password: <%= instance['password'] %>
<%- end -%>
<% if !instance['supress_errors'].nil? -%>
supress_errors: <%= instance['supress_errors'] %>
<%- end -%>
<%- if !instance['detailed_queues'].nil? and unless instance['detailed_queues'].empty? -%>
Copy link
Contributor

Choose a reason for hiding this comment

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

Mixing unless and if !. Would be more idiomatic to always use unless.

Copy link
Member Author

@truthbk truthbk May 14, 2019

Choose a reason for hiding this comment

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

You're right this isn't very idiomatic - and it's just rooted at my subpar ruby. I just always struggle with the readability of compound logical statements with unless because you never know if the other side of the or/and statement is also getting negated, and feel it ends up being less readable. I'll change it regardless.

detailed_queues:
<%- Array(instance['detailed_queues'] ).each do |queue| -%>
- <%= queue %>
<%- end -%>
<%- end -%>
<%- end -%>
<%- if !instance['detailed_topics'].nil? and unless instance['detailed_topics'].empty? -%>
detailed_topics:
<%- Array(instance['detailed_topics'] ).each do |topic| -%>
- <%= topic %>
<%- end -%>
<%- end -%>
<%- end -%>
<%- if !instance['detailed_subscribers'].nil? and unless instance['detailed_subscribers'].empty? -%>
detailed_subscribers:
<%- Array(instance['detailed_subscribers'] ).each do |subscriber| -%>
- <%= subscriber %>
<%- end %>
<%- end -%>
<%- end -%>
<% end -%>