fix: logging API roles (#1244)

Author: davidgamez

api/src/feeds/impl/feeds_api_impl.py

@@ -35,7 +35,6 @@
     InternalHTTPException,
     gbfs_feed_not_found,
 )
-from shared.common.logging_utils import Logger
 from shared.database.database import Database, with_db_session
 from shared.database_gen.sqlacodegen_models import (
     Feed as FeedOrm,
@@ -50,6 +49,7 @@
 from shared.feed_filters.gtfs_feed_filter import LocationFilter
 from shared.feed_filters.gtfs_rt_feed_filter import GtfsRtFeedFilter, EntityTypeFilter
 from utils.date_utils import valid_iso_date
+from utils.logger import get_logger
 
 T = TypeVar("T", bound="Feed")
 
@@ -64,7 +64,7 @@ class FeedsApiImpl(BaseFeedsApi):
     APIFeedType = Union[FeedOrm, GtfsFeed, GtfsRTFeed]
 
     def __init__(self) -> None:
-        self.logger = Logger("FeedsApiImpl").get_logger()
+        self.logger = get_logger("FeedsApiImpl")
 
     @with_db_session
     def get_feed(self, id: str, db_session: Session) -> Feed:

api/src/feeds/impl/models/validation_report_impl.py

@@ -1,6 +1,6 @@
 from shared.database_gen.sqlacodegen_models import Validationreport
 from feeds_gen.models.validation_report import ValidationReport
-from shared.common.logging_utils import Logger
+from utils.logger import get_logger
 
 DATETIME_FORMAT = "%Y-%m-%dT%H:%M:%S"
 
@@ -18,7 +18,7 @@ class Config:
 
     @classmethod
     def _get_logger(cls):
-        return Logger(ValidationReportImpl.__class__.__module__).get_logger()
+        return get_logger(ValidationReportImpl.__class__.__module__)
 
     @classmethod
     def from_orm(cls, validation_report: Validationreport | None) -> ValidationReport | None:

api/src/middleware/request_context.py

@@ -3,7 +3,6 @@
 
 import requests
 from google.auth import jwt
-from requests.exceptions import RequestException
 from starlette.datastructures import Headers
 from starlette.types import Scope
 
@@ -33,8 +32,10 @@ def resolve_google_public_keys(self):
                 )
                 response.raise_for_status()
                 self.google_public_keys = response.json()
-            except RequestException as e:
-                print(f"Error fetching Google's public keys: {e}")
+                if not self.google_public_keys:
+                    logging.error("Fetched Google's public keys, but the result is empty or invalid.")
+            except requests.exceptions.RequestException as e:
+                logging.error(f"Failed to fetch Google's public keys: {e}")
 
     def decode_jwt(self, token: str):
         """
@@ -47,10 +48,11 @@ def decode_jwt(self, token: str):
             token = token.replace("Bearer ", "")
             self.resolve_google_public_keys()
             if not self.google_public_keys:
+                logging.error("Cannot decode JWT: Google's public keys are not available.")
                 return None
             return jwt.decode(token, self.google_public_keys, audience=get_config(PROJECT_ID))
         except Exception as e:
-            logging.warning("Error decoding JWT: %s", e)
+            logging.error("Error decoding JWT: %s", e)
             return None
 
     def _extract_from_headers(self, headers: dict, scope: Scope) -> None:

api/src/scripts/load_dataset_on_create.py

@@ -10,13 +10,13 @@
 from google.cloud.pubsub_v1.futures import Future
 
 from shared.database_gen.sqlacodegen_models import Feed
-from shared.common.logging_utils import Logger
+from utils.logger import get_logger
 
 # Lazy create so we won't try to connect to google cloud when the file is imported.
 pubsub_client = None
 
 lock = threading.Lock()
-logger = Logger("load_dataset_on_create").get_logger()
+logger = get_logger("load_dataset_on_create")
 
 
 def get_pubsub_client():

api/src/scripts/populate_db.py

@@ -9,13 +9,13 @@
 from dotenv import load_dotenv
 from sqlalchemy import text
 
-from shared.common.logging_utils import Logger
 from shared.database.database import Database
 from shared.database.database import configure_polymorphic_mappers
 from shared.database_gen.sqlacodegen_models import Feed, Gtfsrealtimefeed, Gtfsfeed, Gbfsfeed
 from shared.database_gen.sqlacodegen_models import (
     t_feedsearch,
 )
+from utils.logger import get_logger
 
 if TYPE_CHECKING:
     from sqlalchemy.orm import Session
@@ -50,7 +50,7 @@ def __init__(self, filepaths):
         Specify a list of files to load the csv data from.
         Can also be a single string with a file name.
         """
-        self.logger = Logger(self.__class__.__name__).get_logger()
+        self.logger = get_logger(self.__class__.__name__)
         self.logger.setLevel(logging.INFO)
         self.db = Database(echo_sql=False)
         self.df = pandas.DataFrame()

api/src/scripts/populate_db_test_data.py

@@ -20,9 +20,10 @@
     Gbfsfeed,
 )
 from scripts.populate_db import set_up_configs, DatabasePopulateHelper
-from shared.common.logging_utils import Logger
 from typing import TYPE_CHECKING
 
+from utils.logger import get_logger
+
 if TYPE_CHECKING:
     from sqlalchemy.orm import Session
 
@@ -38,7 +39,7 @@ def __init__(self, filepaths):
         Specify a list of files to load the json data from.
         Can also be a single string with a file name.
         """
-        self.logger = Logger(self.__class__.__module__).get_logger()
+        self.logger = get_logger(self.__class__.__module__)
 
         if not isinstance(filepaths, list):
             self.filepaths = [filepaths]

api/src/shared/common/logging_utils.py

@@ -7,29 +7,3 @@ def get_env_logging_level():
     Get the logging level from the environment via OS variable LOGGING_LEVEL. Returns INFO if not set.
     """
     return logging.getLevelName(os.getenv("LOGGING_LEVEL", "INFO"))
-
-
-class Logger:
-    """
-    Util class for logging information, errors or warnings
-    """
-
-    def __init__(self, name):
-        """
-        Initialize the logger
-        """
-        formatter = logging.Formatter("%(asctime)s %(levelname)s %(name)s %(message)s")
-
-        console_handler = logging.StreamHandler()
-        console_handler.setFormatter(formatter)
-
-        self.logger = logging.getLogger(name)
-        self.logger.addHandler(console_handler)
-        self.logger.setLevel(get_env_logging_level())
-
-    def get_logger(self):
-        """
-        Get the logger instance
-        :return: the logger instance
-        """
-        return self.logger

api/src/utils/logger.py

@@ -62,23 +62,26 @@ class GoogleCloudLogFilter(CloudLoggingFilter):
     """
 
     def filter(self, record: logging.LogRecord) -> bool:
-        request_context = get_request_context()
-        http_request = get_http_request(record)
-        if http_request:
-            record.http_request = asdict(http_request)
-        span_id = request_context.get("span_id")
-        trace = get_trace(request_context)
-        record.trace = trace
-        record.span_id = span_id
-
-        record._log_fields = {
-            "logging.googleapis.com/trace": trace,
-            "logging.googleapis.com/spanId": span_id,
-            "logging.googleapis.com/httpRequest": asdict(http_request) if http_request else None,
-            "logging.googleapis.com/trace_sampled": True,
-        }
-        super().filter(record)
-
+        try:
+            request_context = get_request_context()
+            http_request = get_http_request(record)
+            if http_request:
+                record.http_request = asdict(http_request)
+            span_id = request_context.get("span_id")
+            trace = get_trace(request_context)
+            record.trace = trace
+            record.span_id = span_id
+
+            record._log_fields = {
+                "logging.googleapis.com/trace": trace,
+                "logging.googleapis.com/spanId": span_id,
+                "logging.googleapis.com/httpRequest": asdict(http_request) if http_request else None,
+                "logging.googleapis.com/trace_sampled": True,
+            }
+            super().filter(record)
+        except Exception as e:
+            # Using print to avoid a recursive call the log filter
+            print(f"Error in GoogleCloudLogFilter: {e}")
         return True
 
 
@@ -116,10 +119,12 @@ def is_local_env():
 
 
 def global_logging_setup():
+    logging.debug("Starting cloud up logging")
     if is_local_env():
+        logging.debug("Setting local up logging")
         logging.basicConfig(level=get_env_logging_level())
         return
-
+    logging.debug("Setting cloud up logging")
     # Send warnings through logging
     logging.captureWarnings(True)
     # Replace sys.stderr
@@ -152,4 +157,4 @@ def global_logging_setup():
     ]:
         get_logger(name)
 
-    logging.info("Setting cloud up logging completed")
+    logging.debug("Setting cloud up logging completed")

infra/feed-api/main.tf

@@ -32,6 +32,23 @@ locals {
   #  DEV and QA use the vpc connector
   vpc_connector_name = lower(var.environment) == "dev" ? "vpc-connector-qa" : "vpc-connector-${lower(var.environment)}"
   vpc_connector_project = lower(var.environment) == "dev" ? "mobility-feeds-qa" : var.project_id
+  service_account_roles = [
+    # Cloud Logging: allows writing logs to GCP
+    "roles/logging.logWriter",
+    # Cloud Trace: allows writing trace and span data
+    "roles/cloudtrace.agent",
+    # Cloud Monitoring: allows publishing custom metrics
+    "roles/monitoring.metricWriter",
+    # Serverless VPC Access: required to use a VPC connector
+    "roles/vpcaccess.user"
+  ]
+    service_account_role_bindings = {
+    for role in local.service_account_roles :
+    "${role}" => {
+      role    = role
+      project = var.project_id
+    }
+  }
 }
 
 data "google_vpc_access_connector" "vpc_connector" {
@@ -56,7 +73,7 @@ resource "google_cloud_run_v2_service" "mobility-feed-api" {
     service_account = google_service_account.containers_service_account.email
     vpc_access {
       connector = data.google_vpc_access_connector.vpc_connector.id
-      egress = "PRIVATE_RANGES_ONLY"
+      egress = "ALL_TRAFFIC"
     }
     containers {
       image = "${var.gcp_region}-docker.pkg.dev/${var.project_id}/${var.docker_repository_name}/${var.feed_api_service}:${var.feed_api_image_version}"
@@ -108,6 +125,14 @@ resource "google_secret_manager_secret_iam_member" "policy" {
   member =  "serviceAccount:${google_service_account.containers_service_account.email}"
 }
 
+resource "google_project_iam_member" "containers_service_account_roles" {
+  for_each = local.service_account_role_bindings
+
+  project = each.value.project
+  role    = each.value.role
+  member  = "serviceAccount:${google_service_account.containers_service_account.email}"
+}
+
 output "feed_api_uri" {
   value       = google_cloud_run_v2_service.mobility-feed-api.uri
   description = "Main URI of the Feed API"