Merge pull request #2946 from Netflix/enhanced_error_loggin

Improve error logging for a couple of use cases

lemur/common/defaults.py

@@ -2,6 +2,7 @@ import re
 import unicodedata
 
 from cryptography import x509
+from cryptography.hazmat.primitives.serialization import Encoding
 from flask import current_app
 
 from lemur.common.utils import is_selfsigned
@@ -71,12 +72,20 @@ def common_name(cert):
     :return: Common name or None
     """
     try:
-        return cert.subject.get_attributes_for_oid(x509.OID_COMMON_NAME)[
+        subject_oid = cert.subject.get_attributes_for_oid(x509.OID_COMMON_NAME)
-            0
+        if len(subject_oid) > 0:
-        ].value.strip()
+            return subject_oid[0].value.strip()
+        return None
     except Exception as e:
         sentry.captureException()
-        current_app.logger.error("Unable to get common name! {0}".format(e))
+        current_app.logger.error(
+            {
+                "message": "Unable to get common name",
+                "error": e,
+                "public_key": cert.public_bytes(Encoding.PEM).decode("utf-8")
+            },
+            exc_info=True
+        )
 
 
 def organization(cert):

lemur/plugins/lemur_aws/plugin.py

@@ -216,22 +216,24 @@ class AWSSourcePlugin(SourcePlugin):
 
         for region in regions:
             elbs = elb.get_all_elbs(account_number=account_number, region=region)
-            current_app.logger.info(
+            current_app.logger.info({
-                "Describing classic load balancers in {0}-{1}".format(
+                "message": "Describing classic load balancers",
-                    account_number, region
+                "account_number": account_number,
-                )
+                "region": region,
-            )
+                "number_of_load_balancers": len(elbs)
+            })
 
             for e in elbs:
                 endpoints.extend(get_elb_endpoints(account_number, region, e))
 
             # fetch advanced ELBs
             elbs_v2 = elb.get_all_elbs_v2(account_number=account_number, region=region)
-            current_app.logger.info(
+            current_app.logger.info({
-                "Describing advanced load balancers in {0}-{1}".format(
+                "message": "Describing advanced load balancers",
-                    account_number, region
+                "account_number": account_number,
-                )
+                "region": region,
-            )
+                "number_of_load_balancers": len(elbs_v2)
+            })
 
             for e in elbs_v2:
                 endpoints.extend(get_elb_endpoints_v2(account_number, region, e))

lemur/sources/service.py

@@ -123,15 +123,19 @@ def sync_endpoints(source):
                                               "acct": s.get_option("accountNumber", source.options)})
 
         if not endpoint["certificate"]:
-            current_app.logger.error(
+            current_app.logger.error({
-                "Certificate Not Found. Name: {0} Endpoint: {1}".format(
+                "message": "Certificate Not Found",
-                    certificate_name, endpoint["name"]
+                "certificate_name": certificate_name,
-                )
+                "endpoint_name": endpoint["name"],
-            )
+                "dns_name": endpoint.get("dnsname"),
+                "account": s.get_option("accountNumber", source.options),
+            })
+
             metrics.send("endpoint.certificate.not.found",
                          "counter", 1,
                          metric_tags={"cert": certificate_name, "endpoint": endpoint["name"],
-                                      "acct": s.get_option("accountNumber", source.options)})
+                                      "acct": s.get_option("accountNumber", source.options),
+                                      "dnsname": endpoint.get("dnsname")})
             continue
 
         policy = endpoint.pop("policy")