From 1360d846fd16d0b375de8ce2b810a94aee312c49 Mon Sep 17 00:00:00 2001
From: Curtis Castrapel <ccastrapel@netflix.com>
Date: Wed, 8 Apr 2020 11:50:42 -0700
Subject: [PATCH] Improve error logging for a couple of use cases

---
 lemur/common/defaults.py          | 17 +++++++++++++----
 lemur/plugins/lemur_aws/plugin.py | 22 ++++++++++++----------
 lemur/sources/service.py          | 16 ++++++++++------
 3 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/lemur/common/defaults.py b/lemur/common/defaults.py
index d563dbd0..b9c88e49 100644
--- a/lemur/common/defaults.py
+++ b/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)[
-            0
-        ].value.strip()
+        subject_oid = cert.subject.get_attributes_for_oid(x509.OID_COMMON_NAME)
+        if len(subject_oid) > 0:
+            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):
diff --git a/lemur/plugins/lemur_aws/plugin.py b/lemur/plugins/lemur_aws/plugin.py
index 7bb7a3a2..8692348a 100644
--- a/lemur/plugins/lemur_aws/plugin.py
+++ b/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(
-                "Describing classic load balancers in {0}-{1}".format(
-                    account_number, region
-                )
-            )
+            current_app.logger.info({
+                "message": "Describing classic load balancers",
+                "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(
-                "Describing advanced load balancers in {0}-{1}".format(
-                    account_number, region
-                )
-            )
+            current_app.logger.info({
+                "message": "Describing advanced load balancers",
+                "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))
diff --git a/lemur/sources/service.py b/lemur/sources/service.py
index f4783313..e0f0aacf 100644
--- a/lemur/sources/service.py
+++ b/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(
-                "Certificate Not Found. Name: {0} Endpoint: {1}".format(
-                    certificate_name, endpoint["name"]
-                )
-            )
+            current_app.logger.error({
+                "message": "Certificate Not Found",
+                "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")