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

Don't hijack statsd #384

Merged
merged 19 commits into from
Dec 12, 2018
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
2 changes: 1 addition & 1 deletion lib/kubernetes-deploy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,5 @@

module KubernetesDeploy
MIN_KUBE_VERSION = '1.9.0'
KubernetesDeploy::StatsD.build
StatsD.build
end
12 changes: 6 additions & 6 deletions lib/kubernetes-deploy/deploy_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -159,25 +159,25 @@ def run!(verify_result: true, allow_protected_ns: false, prune: true)
MSG
@logger.summary.add_paragraph(ColorizedString.new(warning).yellow)
end
::StatsD.event("Deployment of #{@namespace} succeeded",
StatsD.event("Deployment of #{@namespace} succeeded",
"Successfully deployed all #{@namespace} resources to #{@context}",
alert_type: "success", tags: statsd_tags << "status:success")
::StatsD.measure('all_resources.duration', StatsD.duration(start), tags: statsd_tags << "status:success")
StatsD.distribution('all_resources.duration', StatsD.duration(start), tags: statsd_tags << "status:success")
@logger.print_summary(:success)
rescue DeploymentTimeoutError
@logger.print_summary(:timed_out)
::StatsD.event("Deployment of #{@namespace} timed out",
StatsD.event("Deployment of #{@namespace} timed out",
"One or more #{@namespace} resources failed to deploy to #{@context} in time",
alert_type: "error", tags: statsd_tags << "status:timeout")
::StatsD.measure('all_resources.duration', StatsD.duration(start), tags: statsd_tags << "status:timeout")
StatsD.distribution('all_resources.duration', StatsD.duration(start), tags: statsd_tags << "status:timeout")
raise
rescue FatalDeploymentError => error
@logger.summary.add_action(error.message) if error.message != error.class.to_s
@logger.print_summary(:failure)
::StatsD.event("Deployment of #{@namespace} failed",
StatsD.event("Deployment of #{@namespace} failed",
"One or more #{@namespace} resources failed to deploy to #{@context}",
alert_type: "error", tags: statsd_tags << "status:failed")
::StatsD.measure('all_resources.duration', StatsD.duration(start), tags: statsd_tags << "status:failed")
StatsD.distribution('all_resources.duration', StatsD.duration(start), tags: statsd_tags << "status:failed")
raise
end

Expand Down
2 changes: 1 addition & 1 deletion lib/kubernetes-deploy/kubectl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def run(*args, log_failure: nil, use_context: true, use_namespace: true, raise_i
raise(ResourceNotFoundError, err) if raise_if_not_found
else
@logger.debug("Kubectl err: #{err}") unless output_is_sensitive?
::StatsD.increment('kubectl.error', 1, tags: { context: @context, namespace: @namespace, cmd: args[1] })
StatsD.increment('kubectl.error', 1, tags: { context: @context, namespace: @namespace, cmd: args[1] })
end
sleep retry_delay(attempt) unless attempt == attempts
end
Expand Down
2 changes: 1 addition & 1 deletion lib/kubernetes-deploy/kubernetes_resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ def pretty_status

def report_status_to_statsd(watch_time)
unless @statsd_report_done
::StatsD.measure('resource.duration', watch_time, tags: statsd_tags)
StatsD.distribution('resource.duration', watch_time, tags: statsd_tags)
@statsd_report_done = true
end
end
Expand Down
6 changes: 3 additions & 3 deletions lib/kubernetes-deploy/restart_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,14 @@ def perform!(deployments_names = nil)
raise DeploymentTimeoutError
end
raise FatalDeploymentError unless success
::StatsD.distribution('restart.duration', StatsD.duration(start), tags: tags('success', deployments))
StatsD.distribution('restart.duration', StatsD.duration(start), tags: tags('success', deployments))
@logger.print_summary(:success)
rescue DeploymentTimeoutError
::StatsD.distribution('restart.duration', StatsD.duration(start), tags: tags('timeout', deployments))
StatsD.distribution('restart.duration', StatsD.duration(start), tags: tags('timeout', deployments))
@logger.print_summary(:timed_out)
raise
rescue FatalDeploymentError => error
::StatsD.distribution('restart.duration', StatsD.duration(start), tags: tags('failure', deployments))
StatsD.distribution('restart.duration', StatsD.duration(start), tags: tags('failure', deployments))
@logger.summary.add_action(error.message) if error.message != error.class.to_s
@logger.print_summary(:failure)
raise
Expand Down
6 changes: 3 additions & 3 deletions lib/kubernetes-deploy/runner_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,14 @@ def run!(task_template:, entrypoint:, args:, env_vars: [], verify_result: true)
else
record_status_once(pod)
end
::StatsD.distribution('task_runner.duration', StatsD.duration(start), tags: statsd_tags('success'))
StatsD.distribution('task_runner.duration', StatsD.duration(start), tags: statsd_tags('success'))
@logger.print_summary(:success)
rescue DeploymentTimeoutError
::StatsD.distribution('task_runner.duration', StatsD.duration(start), tags: statsd_tags('timeout'))
StatsD.distribution('task_runner.duration', StatsD.duration(start), tags: statsd_tags('timeout'))
@logger.print_summary(:timed_out)
raise
rescue FatalDeploymentError
::StatsD.distribution('task_runner.duration', StatsD.duration(start), tags: statsd_tags('failure'))
StatsD.distribution('task_runner.duration', StatsD.duration(start), tags: statsd_tags('failure'))
@logger.print_summary(:failure)
raise
end
Expand Down
35 changes: 27 additions & 8 deletions lib/kubernetes-deploy/statsd.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,37 @@

module KubernetesDeploy
class StatsD
extend ::StatsD

PREFIX = "KubernetesDeploy"

def self.duration(start_time)
timothysmith0609 marked this conversation as resolved.
Show resolved Hide resolved
(Time.now.utc - start_time).round(1)
end

def self.build
::StatsD.default_sample_rate = 1.0
::StatsD.prefix = "KubernetesDeploy"
timothysmith0609 marked this conversation as resolved.
Show resolved Hide resolved

if ENV['STATSD_DEV'].present?
::StatsD.backend = ::StatsD::Instrument::Backends::LoggerBackend.new(Logger.new($stderr))
self.backend = ::StatsD::Instrument::Backends::LoggerBackend.new(Logger.new($stderr))
elsif ENV['STATSD_ADDR'].present?
statsd_impl = ENV['STATSD_IMPLEMENTATION'].present? ? ENV['STATSD_IMPLEMENTATION'] : "datadog"
::StatsD.backend = ::StatsD::Instrument::Backends::UDPBackend.new(ENV['STATSD_ADDR'], statsd_impl)
self.backend = ::StatsD::Instrument::Backends::UDPBackend.new(ENV['STATSD_ADDR'], statsd_impl)
else
::StatsD.backend = ::StatsD::Instrument::Backends::NullBackend.new
self.backend = ::StatsD::Instrument::Backends::NullBackend.new
end
::StatsD.backend
end

# It is not sufficient to set the prefix field on the KubernetesDeploy::StatsD singleton itself, since its value
# is overridden in the underlying calls to the ::StatsD library, hence the need to pass it in as a custom prefix
# via the metric_options hash. This is done since KubernetesDeploy may be included as a library and should not
# change the global StatsD configuration of the importing application.
def self.increment(key, value = 1, **metric_options)
metric_options[:prefix] = PREFIX
super
end

def self.distribution(key, value = nil, **metric_options, &block)
metric_options[:prefix] = PREFIX
super
end

module MeasureMethods
Expand Down Expand Up @@ -49,7 +63,12 @@ def measure_method(method_name, metric = nil)
dynamic_tags[:error] = error if dynamic_tags.is_a?(Hash)
dynamic_tags << "error:#{error}" if dynamic_tags.is_a?(Array)
end
::StatsD.distribution(metric, KubernetesDeploy::StatsD.duration(start_time), tags: dynamic_tags)

StatsD.distribution(
metric,
KubernetesDeploy::StatsD.duration(start_time),
tags: dynamic_tags
)
end
end

Expand Down
17 changes: 17 additions & 0 deletions test/helpers/statsd_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# frozen_string_literal: true
module StatsDHelper
def capture_statsd_calls
mock_backend = ::StatsD::Instrument::Backends::CaptureBackend.new
old_backend = KubernetesDeploy::StatsD.backend
KubernetesDeploy::StatsD.backend = mock_backend

yield if block_given?

mock_backend.collected_metrics
ensure
if old_backend.is_a?(::StatsD::Instrument::Backends::CaptureBackend)
old_backend.collected_metrics.concat(mock_backend.collected_metrics)
end
KubernetesDeploy::StatsD.backend = old_backend
end
end
1 change: 1 addition & 0 deletions test/integration-serial/serial_deploy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
require 'integration_test_helper'

class SerialDeployTest < KubernetesDeploy::IntegrationTest
include StatsDHelper
# This cannot be run in parallel because it either stubs a constant or operates in a non-exclusive namespace
def test_deploying_to_protected_namespace_with_override_does_not_prune
KubernetesDeploy::DeployTask.stub_const(:PROTECTED_NAMESPACES, [@namespace]) do
Expand Down
1 change: 1 addition & 0 deletions test/integration-serial/serial_task_run_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

class SerialTaskRunTest < KubernetesDeploy::IntegrationTest
include TaskRunnerTestHelper
include StatsDHelper

# Mocha is not thread-safe: https://github.com/freerange/mocha#thread-safety
def test_run_without_verify_result_fails_if_pod_was_not_created
Expand Down
1 change: 1 addition & 0 deletions test/unit/kubernetes-deploy/kubectl_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
require 'test_helper'

class KubectlTest < KubernetesDeploy::TestCase
include StatsDHelper
def setup
super
KubernetesDeploy::Kubectl.any_instance.unstub(:run)
Expand Down
18 changes: 17 additions & 1 deletion test/unit/kubernetes-deploy/statsd_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
require 'test_helper'

class StatsDTest < KubernetesDeploy::TestCase
include StatsDHelper
class TestMeasureClass
extend(KubernetesDeploy::StatsD::MeasureMethods)

Expand Down Expand Up @@ -40,14 +41,29 @@ def test_build_when_statsd_addr_env_present_but_statsd_implementation_is_not

KubernetesDeploy::StatsD.build

assert_equal :datadog, StatsD.backend.implementation
assert_equal(:datadog, KubernetesDeploy::StatsD.backend.implementation)
ensure
ENV['STATSD_ADDR'] = original_addr
ENV['STATSD_IMPLEMENTATION'] = original_impl
ENV['STATSD_DEV'] = original_dev
KubernetesDeploy::StatsD.build
end

def test_kubernetes_statsd_does_not_override_global_config
old_prefix = ::StatsD.prefix
old_sample_rate = ::StatsD.default_sample_rate

::StatsD.prefix = "test"
::StatsD.default_sample_rate = 2.0
timothysmith0609 marked this conversation as resolved.
Show resolved Hide resolved
KubernetesDeploy::StatsD.build
refute_equal(KubernetesDeploy::StatsD.prefix, ::StatsD.prefix)
refute_equal(KubernetesDeploy::StatsD.default_sample_rate, ::StatsD.default_sample_rate)
refute_equal(KubernetesDeploy::StatsD.backend, ::StatsD.backend)
ensure
::StatsD.prefix = old_prefix
::StatsD.default_sample_rate = old_sample_rate
end

def test_measuring_non_existent_method_raises
assert_raises_message(NotImplementedError, "Cannot instrument undefined method bogus_method") do
TestMeasureClass.measure_method(:bogus_method)
Expand Down