diff --git a/api/src/feeds/impl/feeds_api_impl.py b/api/src/feeds/impl/feeds_api_impl.py index 6a54ca4dc..cc5b5b45b 100644 --- a/api/src/feeds/impl/feeds_api_impl.py +++ b/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: diff --git a/api/src/feeds/impl/models/validation_report_impl.py b/api/src/feeds/impl/models/validation_report_impl.py index 5c6bcb644..d8faccc9a 100644 --- a/api/src/feeds/impl/models/validation_report_impl.py +++ b/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: diff --git a/api/src/middleware/request_context.py b/api/src/middleware/request_context.py index 71e42e073..6103cb7f1 100644 --- a/api/src/middleware/request_context.py +++ b/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: diff --git a/api/src/scripts/load_dataset_on_create.py b/api/src/scripts/load_dataset_on_create.py index 80aede72c..5d97e641e 100644 --- a/api/src/scripts/load_dataset_on_create.py +++ b/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(): diff --git a/api/src/scripts/populate_db.py b/api/src/scripts/populate_db.py index c474061ea..5223e70e6 100644 --- a/api/src/scripts/populate_db.py +++ b/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() diff --git a/api/src/scripts/populate_db_test_data.py b/api/src/scripts/populate_db_test_data.py index 02465f3a8..bd71f6cd7 100644 --- a/api/src/scripts/populate_db_test_data.py +++ b/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] diff --git a/api/src/shared/common/logging_utils.py b/api/src/shared/common/logging_utils.py index b7f36dc31..917454db8 100644 --- a/api/src/shared/common/logging_utils.py +++ b/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 diff --git a/api/src/utils/logger.py b/api/src/utils/logger.py index 0b07de8fe..fb30a2714 100644 --- a/api/src/utils/logger.py +++ b/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") diff --git a/infra/feed-api/main.tf b/infra/feed-api/main.tf index 17f3ca089..7c3018b62 100644 --- a/infra/feed-api/main.tf +++ b/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"