Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions webhook_server/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@

# Global variables
ALLOWED_IPS: tuple[ipaddress._BaseNetwork, ...] = ()
LOGGER = get_logger_with_params(name="main")
LOGGER = get_logger_with_params()

_lifespan_http_client: httpx.AsyncClient | None = None

Expand Down Expand Up @@ -210,7 +210,7 @@ async def process_webhook(request: Request, background_tasks: BackgroundTasks) -

# Create repository-specific logger
repository_name = hook_data["repository"]["name"]
logger = get_logger_with_params(name="main", repository_name=repository_name)
logger = get_logger_with_params(repository_name=repository_name)
logger.info(f"{log_context} Processing webhook for repository: {repository_name}")

async def process_with_error_handling(_api: GithubWebhook, _logger: logging.Logger) -> None:
Expand Down
10 changes: 5 additions & 5 deletions webhook_server/tests/test_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,14 @@ def test_extract_key_from_dict_complex_nested(self) -> None:

def test_get_logger_with_params_default(self) -> None:
"""Test logger creation with default parameters."""
unique_name = "test_helpers_logger"
logger = get_logger_with_params(name=unique_name)
logger = get_logger_with_params()
assert isinstance(logger, logging.Logger)
assert logger.name == unique_name
# Logger name is now based on log file path to ensure single handler instance
assert logger.name.startswith("webhook-server-")

def test_get_logger_with_params_with_repository(self) -> None:
"""Test logger creation with repository name."""
logger = get_logger_with_params(name="test", repository_name="test-repo")
logger = get_logger_with_params(repository_name="test-repo")
assert isinstance(logger, logging.Logger)
# The logger should have repository-specific formatting

Expand Down Expand Up @@ -226,7 +226,7 @@ def test_get_logger_with_params_log_file_path(self, tmp_path, monkeypatch):
mock_config = MockConfig.return_value
mock_config.get_value.side_effect = lambda value, **kwargs: "test.log" if value == "log-file" else "INFO"
mock_config.data_dir = str(tmp_path)
logger = get_logger_with_params(name="test_logger", repository_name="repo")
logger = get_logger_with_params(repository_name="repo")
assert isinstance(logger, logging.Logger)
log_dir = tmp_path / "logs"
assert log_dir.exists()
Expand Down
2 changes: 1 addition & 1 deletion webhook_server/utils/app_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
CLOUDFLARE_IPS_URL: str = "https://api.cloudflare.com/client/v4/ips"

# Logger for utilities
LOGGER = get_logger_with_params(name="app_utils")
LOGGER = get_logger_with_params()


async def get_github_allowlist(http_client: httpx.AsyncClient) -> list[str]:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from webhook_server.utils.helpers import get_api_with_highest_rate_limit, get_logger_with_params
from webhook_server.utils.webhook import create_webhook

LOGGER = get_logger_with_params(name="repository-and-webhook-settings")
LOGGER = get_logger_with_params()


def get_repository_api(repository: str) -> tuple[str, github.Github | None, str]:
Expand Down
2 changes: 1 addition & 1 deletion webhook_server/utils/github_repository_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
"required_conversation_resolution": True,
}

LOGGER = get_logger_with_params(name="github-repository-settings")
LOGGER = get_logger_with_params()


def _get_github_repo_api(github_api: github.Github, repository: int | str) -> Repository | None:
Expand Down
45 changes: 37 additions & 8 deletions webhook_server/utils/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,40 @@


def get_logger_with_params(
name: str,
repository_name: str = "",
mask_sensitive: bool = True,
) -> Logger:
mask_sensitive_patterns: list[str] = [
# Passwords and secrets
"container_repository_password",
"-p",
"password",
"secret",
# Tokens and API keys
"token",
"apikey",
"secret",
"api_key",
"github_token",
"GITHUB_TOKEN",
"pypi",
# Authentication credentials
"username",
"login",
"-u",
"-p",
"--username",
"--password",
"--creds",
# Private keys and sensitive IDs
"private_key",
"private-key",
"webhook_secret",
"webhook-secret",
"github-app-id",
# Slack webhooks (contain sensitive URLs)
"slack-webhook-url",
"slack_webhook_url",
"webhook-url",
"webhook_url",
]

_config = Config(repository=repository_name)
Expand All @@ -49,8 +72,14 @@ def get_logger_with_params(

log_file = os.path.join(log_file_path, log_file)

# CRITICAL FIX: Use a fixed logger name for the same log file to ensure
# only ONE RotatingFileHandler instance manages the file rotation.
# Multiple handlers writing to the same file causes rotation to fail.
# The original 'name' parameter is preserved in log records via the logger name.
logger_cache_key = f"webhook-server-{log_file or 'console'}"

return get_logger(
name=name,
name=logger_cache_key,
filename=log_file,
level=log_level,
file_max_bytes=1024 * 1024 * 10,
Expand All @@ -74,7 +103,7 @@ def extract_key_from_dict(key: Any, _dict: dict[Any, Any]) -> Any:


def get_github_repo_api(github_app_api: github.Github, repository: int | str) -> Repository:
logger = get_logger_with_params(name="helpers")
logger = get_logger_with_params()
logger.debug(f"Get GitHub API for repository {repository}")

return github_app_api.get_repo(repository)
Expand All @@ -97,7 +126,7 @@ async def run_command(
Returns:
tuple: True, out if command succeeded, False, err otherwise.
"""
logger = get_logger_with_params(name="helpers")
logger = get_logger_with_params()
out_decoded: str = ""
err_decoded: str = ""
kwargs["stdout"] = subprocess.PIPE
Expand Down Expand Up @@ -158,7 +187,7 @@ def get_api_with_highest_rate_limit(config: Config, repository_name: str = "") -
Returns:
tuple: API, token, api_user
"""
logger = get_logger_with_params(name="helpers")
logger = get_logger_with_params()

api: github.Github | None = None
token: str | None = None
Expand Down Expand Up @@ -204,7 +233,7 @@ def get_api_with_highest_rate_limit(config: Config, repository_name: str = "") -


def log_rate_limit(rate_limit: RateLimitOverview, api_user: str) -> None:
logger = get_logger_with_params(name="helpers")
logger = get_logger_with_params()

rate_limit_str: str
time_for_limit_reset: int = (rate_limit.rate.reset - datetime.datetime.now(tz=datetime.timezone.utc)).seconds
Expand Down
2 changes: 1 addition & 1 deletion webhook_server/utils/webhook.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
get_logger_with_params,
)

LOGGER = get_logger_with_params(name="webhook")
LOGGER = get_logger_with_params()


def process_github_webhook(
Expand Down