[collector] Only add service check fields if non-null (#3487)

The `api/v1/check_run` endpoint doesn't accept payloads with
a `message` value of `null`, so avoid that. Do the same thing on the
`tags` and `hostname` fields as it makes sense to be consistent.

Author: olivielpeau

checks/__init__.py

@@ -963,12 +963,17 @@ def create_service_check(check_name, status, tags=None, timestamp=None,
     """
     if check_run_id is None:
         check_run_id = get_next_id('service_check')
-    return {
+    service_check = {
         'id': check_run_id,
         'check': check_name,
         'status': status,
-        'host_name': hostname,
-        'tags': tags,
         'timestamp': float(timestamp or time.time()),
-        'message': message
     }
+    if hostname is not None:
+        service_check['host_name'] = hostname
+    if tags is not None:
+        service_check['tags'] = tags
+    if message is not None:
+        service_check["message"] = message
+
+    return service_check

tests/core/test_common.py

@@ -126,7 +126,24 @@ def test_service_check(self):
         timestamp = time.time()
 
         check = AgentCheck('test', {}, {'checksd_hostname':'foo'})
-        check.service_check(check_name, status, tags, timestamp, host_name)
+        # No "message"/"tags" field
+        check.service_check(check_name, status, timestamp=timestamp, hostname=host_name)
+        self.assertEquals(len(check.service_checks), 1, check.service_checks)
+        val = check.get_service_checks()
+        self.assertEquals(len(val), 1)
+        check_run_id = val[0].get('id', None)
+        self.assertNotEquals(check_run_id, None)
+        self.assertEquals([{
+            'id': check_run_id,
+            'check': check_name,
+            'status': status,
+            'host_name': host_name,
+            'timestamp': timestamp,
+        }], val)
+        self.assertEquals(len(check.service_checks), 0, check.service_checks)
+
+        # With "message" field
+        check.service_check(check_name, status, tags, timestamp, host_name, message='foomessage')
         self.assertEquals(len(check.service_checks), 1, check.service_checks)
         val = check.get_service_checks()
         self.assertEquals(len(val), 1)
@@ -139,10 +156,11 @@ def test_service_check(self):
             'host_name': host_name,
             'tags': tags,
             'timestamp': timestamp,
-            'message': None,
+            'message': 'foomessage',
         }], val)
         self.assertEquals(len(check.service_checks), 0, check.service_checks)
 
+
     def test_no_proxy(self):
         """ Starting with Agent 5.0.0, there should always be a local forwarder
         running and all payloads should go through it. So we should make sure