Add datadog component #7158

nunofgs · 2017-04-17T23:28:41Z

Description:

Adds a datadog component. Note that while it is possible to use the existing statsd component to report to Datadog, its true power comes from using tags which requires a datadog-specific library.

I am not affiliated with Datadog in any way. Just a happy user :)

This all started when I saw a tweet by @andre.rb. Since I was already using Datadog, I decided to build a component for HA.

Here's what I've been able to achieve using this component:

With the data in Datadog I'm able to also set up alerts to email/Slack when, for example, a statistical anomaly is detected on a sensor.

Datadog also supports events, which I'm grabbing from the logbook:

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#2452

Example entry for `configuration.yaml`:

datadog:
  host: 127.0.0.1

Note that you need a datadog-agent running. More information about that here.

Checklist:

Documentation added/updated in home-assistant.github.io
Local tests with tox run successfully. Your PR cannot be merged unless tests pass
New dependencies have been added to the REQUIREMENTS variable (example).
New dependencies are only imported inside functions that use them (example).
New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.

mention-bot · 2017-04-17T23:28:43Z

@nunofgs, thanks for your PR! By analyzing the history of the files in this pull request, we identified @postlund, @MartinHjelmare and @balloob to be potential reviewers.

homeassistant · 2017-04-17T23:28:44Z

Hi @nunofgs,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

houndci-bot · 2017-04-17T23:29:44Z

tests/components/test_datadog.py

+            for attribute, value in attributes.items():
+                mock_client.gauge.assert_has_calls([
+                    mock.call("ha.sensor.%s" % attribute, value, sample_rate=1,
+                        tags=["entity:%s" % state.entity_id])


continuation line under-indented for visual indent

houndci-bot · 2017-04-17T23:29:44Z

tests/components/test_datadog.py

+
+        for in_, out in valid.items():
+            state = mock.MagicMock(domain="sensor", entity_id="sensor.foo.bar",
+                state=in_, attributes=attributes)


continuation line under-indented for visual indent

houndci-bot · 2017-04-17T23:29:44Z

tests/components/test_datadog.py

+import unittest
+import voluptuous as vol
+
+class TestDatadog(unittest.TestCase):


expected 2 blank lines, found 1

houndci-bot · 2017-04-17T23:29:44Z

homeassistant/components/logbook.py

@@ -17,7 +17,7 @@
 from homeassistant.components import sun
 from homeassistant.components.frontend import register_built_in_panel
 from homeassistant.components.http import HomeAssistantView
-from homeassistant.const import (EVENT_HOMEASSISTANT_START,
+from homeassistant.const import (EVENT_HOMEASSISTANT_START, EVENT_LOGBOOK_ENTRY,


line too long (80 > 79 characters)

houndci-bot · 2017-04-17T23:29:44Z

homeassistant/components/datadog.py

+
+def setup(hass, config):
+    """Setup the Datadog component."""
+    from datadog import initialize, statsd, DogStatsd


'datadog.DogStatsd' imported but unused

homeassistant · 2017-04-17T23:30:27Z

Hi @nunofgs,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

houndci-bot · 2017-04-17T23:30:35Z

tests/components/test_datadog.py

+
+from tests.common import get_test_home_assistant
+
+class TestDatadog(unittest.TestCase):


expected 2 blank lines, found 1

houndci-bot · 2017-04-17T23:30:35Z

homeassistant/components/datadog.py

+        try:
+            value = state_helper.state_as_number(state)
+        except ValueError:
+            _LOGGER.debug('Error sending %s: %s (tags: %s)', metric, state.state, tags)


line too long (87 > 79 characters)

MartinHjelmare

The type change in the state helper needs some thought. Simplest solution would be to move that to the data dog module. As it is now the change will create an error for current users of Influxdb component.

Otherwise looks good. Some style comments. Use python 3 string formatting consistently. Haven't looked at the tests.

MartinHjelmare · 2017-04-18T06:16:15Z

homeassistant/helpers/state.py

-    if state.state in (STATE_ON, STATE_LOCKED, STATE_ABOVE_HORIZON,
-                       STATE_OPEN):
+    if state.state in (STATE_ON, STATE_LOCKED, STATE_ABOVE_HORIZON, STATE_OPEN,
+                       STATE_HOME, STATE_PLAYING):


This change of state type will create a type mismatch error for Influxdb schema.

I personally feel that, in the context of "assigning" a value to a state, STATE_HOME and STATE_PLAYING naturally fall under a "1". If anything, we should be adding more states and mapping them to 1 and 0.

However, I understand the schema issues this may bring. In fact, it's probably an issue for other components as well (logentries for example). By changing the converted state this PR is introducing a BC and possibly breaking alerts/graphs that people are using.

I'll revert this change but I really think an improved way of converting to a number should be developed, as Datadog (and possibly others) only handle numbers (not strings).

I agree, but we should take more devs opinion and it should go in another PR that targets that topic specifically. It will probably need some discussion on how to handle breakage. We've used migration scripts in the past eg for influxdb. You're welcome to start a PR or RFC issue for discussion.

Yeah, I would prefer it in a new PR too. Also agree that more number representations is better in the long run.

MartinHjelmare · 2017-04-18T06:17:48Z

homeassistant/components/datadog.py

+import voluptuous as vol
+
+from homeassistant.const import (
+    CONF_HOST,


Please use the whole line width until 79 characters.

MartinHjelmare · 2017-04-18T06:21:57Z

homeassistant/components/datadog.py

+
+        statsd.event(
+            title="Home Assistant",
+            text="%%%%%% \n **%s** %s \n %%%%%%" % (name, message),


Use python 3 string formatting, ie 'hello {}'.format('world').

houndci-bot · 2017-04-18T22:17:26Z

homeassistant/components/datadog.py

+            tags=[
+                "entity:{}".format(event.data.get('entity_id'),
+                "domain:{}".format(event.data.get('domain')
+            ]


SyntaxError: invalid syntax

houndci-bot · 2017-04-18T22:17:26Z

homeassistant/components/datadog.py

@@ -0,0 +1,118 @@
+"""


TokenError: EOF in multi-line statement

MartinHjelmare

I've looked over the tests now too. Use assert_setup_component to make sure config is validated when setting up the component.

Aside from that I think it looks good. Please don't rebase/squash your branch/commits during the review process, but just push the new commits. It's a lot easier to see what changed that way. If you want, you can then squash everything when we're ready to merge, although since github now has the squash and merge button, you don't have to do that either.

MartinHjelmare · 2017-04-22T08:28:51Z

tests/components/test_datadog.py

+            }
+        }
+
+        with self.assertRaises(vol.Invalid):


I think you should test this using setup_component and use the helper context manager assert_setup_component found in tests/common.py. See eg: https://github.com/home-assistant/home-assistant/blob/dev/tests/components/alarm_control_panel/test_mqtt.py#L31-L40

Also use assert_setup_component for the other tests when you setup the component, to make sure the config is validated.

Good call. I grabbed this from the statsd component, which I used for the basis of my component.

Would you accept a commit that improves the test there as well?

Yes, but not in this PR.

houndci-bot · 2017-04-26T21:31:39Z

tests/components/test_datadog.py

+
+    def test_invalid_config(self):
+        """Test invalid configuration."""
+        with assert_setup_component(0) as config:


local variable 'config' is assigned to but never used

houndci-bot · 2017-04-26T21:31:39Z

tests/components/test_datadog.py

+)
+from homeassistant.components import datadog
+from homeassistant.setup import setup_component
+import homeassistant.components.datadog as datadog


redefinition of unused 'datadog' from line 13

houndci-bot · 2017-04-26T21:31:39Z

tests/components/test_datadog.py

+from unittest import mock
+import unittest
+
+import voluptuous as vol


'voluptuous as vol' imported but unused

MartinHjelmare · 2017-05-03T12:47:16Z

I think you should use assert_setup_component for all the tests, where you set up the component to make sure config is validated.

nunofgs · 2017-05-05T21:53:15Z

@MartinHjelmare updated.

MartinHjelmare · 2017-05-05T22:17:07Z

Sorry if I'm not making sense. I meant that you should use the helper assert_setup_component as a context manager for all the tests when calling setup_component.

MartinHjelmare · 2017-05-05T22:28:48Z

Nevermind, I think this is fine. 👍

homeassistant added the cla-needed label Apr 17, 2017

balloobbot added component core platform requirement and removed cla-needed labels Apr 17, 2017

houndci-bot reviewed Apr 17, 2017

View reviewed changes

nunofgs force-pushed the enhancement/add-datadog-component branch from 1c95a26 to 6d82ac1 Compare April 17, 2017 23:30

homeassistant added the cla-needed label Apr 17, 2017

balloobbot removed platform cla-needed labels Apr 17, 2017

houndci-bot reviewed Apr 17, 2017

View reviewed changes

homeassistant added the cla-signed label Apr 17, 2017

nunofgs force-pushed the enhancement/add-datadog-component branch from 6d82ac1 to aa53cf2 Compare April 17, 2017 23:33

homeassistant added the cla-signed label Apr 17, 2017

nunofgs mentioned this pull request Apr 17, 2017

Add datadog component documentation home-assistant/home-assistant.io#2452

Merged

MartinHjelmare requested changes Apr 18, 2017

View reviewed changes

nunofgs force-pushed the enhancement/add-datadog-component branch from aa53cf2 to ea0f084 Compare April 18, 2017 22:17

homeassistant added the cla-signed label Apr 18, 2017

houndci-bot reviewed Apr 18, 2017

View reviewed changes

nunofgs force-pushed the enhancement/add-datadog-component branch from ea0f084 to d05f83b Compare April 18, 2017 22:24

homeassistant added the cla-signed label Apr 18, 2017

nunofgs force-pushed the enhancement/add-datadog-component branch from d05f83b to 4935630 Compare April 19, 2017 00:06

homeassistant added the cla-signed label Apr 19, 2017

MartinHjelmare requested changes Apr 22, 2017

View reviewed changes

houndci-bot reviewed Apr 26, 2017

View reviewed changes

nunofgs force-pushed the enhancement/add-datadog-component branch from df68897 to f36196f Compare April 26, 2017 21:33

homeassistant added the cla-signed label Apr 26, 2017

MartinHjelmare self-assigned this Apr 26, 2017

nunofgs added 2 commits May 5, 2017 22:19

Add datadog component

1593794

Improve test_invalid_config datadog test

44b197a

nunofgs force-pushed the enhancement/add-datadog-component branch from f36196f to f443185 Compare May 5, 2017 21:19

homeassistant added the cla-signed label May 5, 2017

Use assert_setup_component for test setup

aae2ad2

nunofgs force-pushed the enhancement/add-datadog-component branch from f443185 to aae2ad2 Compare May 5, 2017 21:34

homeassistant added the cla-signed label May 5, 2017

MartinHjelmare approved these changes May 5, 2017

View reviewed changes

MartinHjelmare merged commit 20ded1b into home-assistant:dev May 5, 2017

jnewland mentioned this pull request May 6, 2017

Datadog jnewland/ha-config#32

Merged

balloob mentioned this pull request May 18, 2017

0.45 #7648

Merged

home-assistant locked and limited conversation to collaborators Aug 12, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add datadog component #7158

Add datadog component #7158

nunofgs commented Apr 17, 2017 •

edited

Loading

mention-bot commented Apr 17, 2017

homeassistant commented Apr 17, 2017

houndci-bot Apr 17, 2017

houndci-bot Apr 17, 2017

houndci-bot Apr 17, 2017

houndci-bot Apr 17, 2017

houndci-bot Apr 17, 2017

homeassistant commented Apr 17, 2017

houndci-bot Apr 17, 2017

houndci-bot Apr 17, 2017

MartinHjelmare left a comment

MartinHjelmare Apr 18, 2017

nunofgs Apr 18, 2017

MartinHjelmare Apr 18, 2017

balloob Apr 20, 2017

MartinHjelmare Apr 18, 2017

MartinHjelmare Apr 18, 2017

houndci-bot Apr 18, 2017

houndci-bot Apr 18, 2017

MartinHjelmare left a comment •

edited

Loading

MartinHjelmare Apr 22, 2017

nunofgs Apr 26, 2017

MartinHjelmare Apr 26, 2017

houndci-bot Apr 26, 2017

houndci-bot Apr 26, 2017

houndci-bot Apr 26, 2017

MartinHjelmare commented May 3, 2017

nunofgs commented May 5, 2017

MartinHjelmare commented May 5, 2017

MartinHjelmare commented May 5, 2017


		from tests.common import get_test_home_assistant

		class TestDatadog(unittest.TestCase):

Add datadog component #7158

Add datadog component #7158

Conversation

nunofgs commented Apr 17, 2017 • edited Loading

Description:

Example entry for configuration.yaml:

Checklist:

mention-bot commented Apr 17, 2017

homeassistant commented Apr 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

homeassistant commented Apr 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MartinHjelmare left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MartinHjelmare left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MartinHjelmare commented May 3, 2017

nunofgs commented May 5, 2017

MartinHjelmare commented May 5, 2017

MartinHjelmare commented May 5, 2017

nunofgs commented Apr 17, 2017 •

edited

Loading

Example entry for `configuration.yaml`:

MartinHjelmare left a comment •

edited

Loading