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 11 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.measure('all_resources.duration', StatsD.duration(start), tags: statsd_tags << "status:success")
timothysmith0609 marked this conversation as resolved.
Show resolved Hide resolved
@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.measure('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.measure('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.measure('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
37 changes: 30 additions & 7 deletions lib/kubernetes-deploy/statsd.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,41 @@

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
self.default_sample_rate = 1.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this the default behavior?

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 this suffers from the same problem as StatsD.prefix in that it's statically referenced in the underlying library so won't have an effect here (`options[:sample_rate] is used to override if ever we need). I'll remove this line as well.

self.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

def self.measure(key, value = nil, *metric_options, &block)
metric_options.first[:prefix] = PREFIX if metric_options.first.is_a?(Hash)
timothysmith0609 marked this conversation as resolved.
Show resolved Hide resolved
super
end

def self.increment(key, value = 1, *metric_options)
metric_options.first[:prefix] = PREFIX if metric_options.first.is_a?(Hash)
super
end

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

module MeasureMethods
Expand Down Expand Up @@ -49,7 +67,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
12 changes: 11 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,23 @@ 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
KubernetesDeploy::StatsD.build
timothysmith0609 marked this conversation as resolved.
Show resolved Hide resolved
::StatsD.prefix = "test"
::StatsD.default_sample_rate = 2.0
timothysmith0609 marked this conversation as resolved.
Show resolved Hide resolved
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)
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