Skip to content

feat: unify error handling and logging - #657

Open
disrupted wants to merge 44 commits into
mainfrom
error-handling
Open

feat: unify error handling and logging#657
disrupted wants to merge 44 commits into
mainfrom
error-handling

Conversation

@disrupted

@disrupted disrupted commented Jul 30, 2026

Copy link
Copy Markdown
Member

close #157

before

before

after

after

disrupted and others added 30 commits April 9, 2026 16:37
@disrupted
disrupted changed the base branch from main to update-ruff July 30, 2026 15:08
Base automatically changed from update-ruff to main August 6, 2026 10:51
@disrupted disrupted changed the title refactor: unify error handling and logging feat: unify error handling and logging Aug 6, 2026
@disrupted
disrupted marked this pull request as ready for review August 6, 2026 14:20
@tobbber
tobbber requested a lite review from Copilot August 6, 2026 14:25

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR migrates KPOps’ logging and expected-error handling to a unified, structured approach (via structlog), aiming to produce more helpful, service-scoped error output—especially for external-service failures (Kafka REST Proxy, Kafka Connect, Helm).

Changes:

  • Introduces centralized structured logging utilities (kpops/utils/logging.py) and migrates many modules from logging to structlog.
  • Adds/standardizes external-service exception types (connection vs HTTP response errors) and updates Kafka REST Proxy / Kafka Connect / Helm wrappers accordingly.
  • Updates and adds tests to assert structured log events (using structlog.testing.capture_logs) and adjusts CLI entrypoint to consistently log KpopsExceptions and exit with code 1.

Reviewed changes

Copilot reviewed 68 out of 70 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
uv.lock Adds structlog to the resolved lockfile.
pyproject.toml Adds structlog dependency; switches console script entrypoint to kpops.cli.main:cli.
kpops/utils/logging.py New central structlog configuration + exception logging helpers + context binding.
kpops/core/exception.py Introduces KpopsException base with logged flag + ServiceException.
kpops/component_handlers/utils/exception.py Adds reusable HTTP-response and connection error base classes for services.
kpops/component_handlers/topic/init.py Adds Kafka REST Proxy service name constant.
kpops/component_handlers/topic/exception.py Refactors topic/Kafka REST errors into service-scoped exception hierarchy.
kpops/component_handlers/topic/kafka_rest.py Adds request/response debug hooks, transport→connection errors, structured logs, bound context.
kpops/component_handlers/topic/handler.py Updates topic handler logging to structured fields (topic_name, counts, etc.).
kpops/component_handlers/kafka_connect/init.py Adds Kafka Connect service name constant.
kpops/component_handlers/kafka_connect/exception.py Refactors Kafka Connect errors into service-scoped exception hierarchy.
kpops/component_handlers/kafka_connect/kafka_connect_api.py Adds structured request/response debug logs, transport→connection errors, bound context.
kpops/component_handlers/kafka_connect/kafka_connect_handler.py Migrates handler logging to structured fields and separates “diff header” vs diff body logs.
kpops/component_handlers/kafka_connect/timeout.py Migrates timeout logging to structured form.
kpops/component_handlers/helm_wrapper/init.py Adds Helm service name constant.
kpops/component_handlers/helm_wrapper/exception.py Introduces Helm service exception types (HelmError, etc.).
kpops/component_handlers/helm_wrapper/helm.py Migrates to structlog; wraps operations with bound context; uses HelmError for stderr parsing.
kpops/component_handlers/helm_wrapper/helm_diff.py Migrates HelmDiff logger type to structlog logger and structlog logging.
kpops/component_handlers/helm_wrapper/dry_run_handler.py Migrates dry-run diff logging to structured fields.
kpops/component_handlers/kubernetes/utils.py Migrates K8s utils logging to structured fields; downgrades critical→warning.
kpops/component_handlers/kubernetes/pvc_handler.py Migrates PVC handler logging to structured fields (namespace/app/pvc_names).
kpops/component_handlers/schema_handler/schema_handler.py Migrates schema handler logging to structured fields; refactors message formatting.
kpops/components/base_components/kafka_app.py Migrates base component logger to structlog.
kpops/components/base_components/kafka_connector.py Migrates base component logger to structlog.
kpops/components/base_components/kubernetes_app.py Migrates base component logger to structlog.
kpops/components/base_components/helm_app.py Migrates base component logger to structlog; structured Helm release existence logs.
kpops/components/base_components/base_defaults_component.py Migrates defaults enrichment logs to structured fields.
kpops/components/streams_bootstrap/base.py Migrates streams bootstrap logs to structured fields (component_name, release, etc.).
kpops/components/streams_bootstrap/streams/streams_app.py Migrates validation fallback warning/debug to structured fields.
kpops/components/streams_bootstrap/producer/producer_app.py Migrates validation fallback warning/debug to structured fields.
kpops/components/streams_bootstrap/consumer/consumer_app.py Migrates validation fallback warning/debug to structured fields.
kpops/components/streams_bootstrap_v2/base.py Migrates streams bootstrap v2 logs to structured fields.
kpops/components/streams_bootstrap_v2/streams/streams_app.py Migrates validation fallback warning/debug to structured fields.
kpops/components/streams_bootstrap_v2/producer/producer_app.py Migrates validation fallback warning/debug to structured fields.
kpops/utils/yaml.py Migrates YAML loader debug log to structured fields.
kpops/utils/pydantic.py Switches settings source logger type to structlog logger.
kpops/utils/gen_schema.py Migrates schema generation logs to structured fields.
kpops/utils/docstring.py Removes unused stdlib logger from docstring helpers.
kpops/pipeline/init.py Migrates pipeline generator logger to structlog.
kpops/core/registry.py Migrates registry debug logs to structured fields; removes typer.style formatting usage.
kpops/config/init.py Migrates config logger to structlog.
kpops/api/logs.py Removes legacy stdlib logging formatter/config module.
kpops/api/init.py Uses centralized logging helpers; adds _run_component() wrapper that logs KpopsExceptions.
kpops/cli/main.py Adds cli() entrypoint to catch/log KpopsExceptions and exit(1).
hooks/gen_docs/gen_docs_components.py Migrates doc generation hook logging to structured fields.
tests/conftest.py Switches test logging setup import; adjusts test component copying to allow existing dirs.
tests/utils/test_pydantic.py Updates import path for SerializeAsOptional helpers.
tests/utils/test_logging.py New tests for log_kpops_exception behavior and service-scoped logger naming.
tests/api/test_run_component.py New tests for _run_component() exception logging and contextvars isolation.
tests/cli/test_main.py New tests for CLI exit code and double-logging avoidance.
tests/pipeline/test_generate.py Updates assertions to validate structured log events for pipeline selection/filtering.
tests/kubernetes/test_pvc_handler.py Updates assertions to validate structured PVC deletion logs.
tests/components/test_kubernetes_app.py Removes old log.info patch fixture (no longer used).
tests/components/test_kafka_sink_connector.py Removes old log.info patch fixture (no longer used).
tests/components/test_helm_app.py Updates destroy/reset/clean tests to assert structured logs.
tests/components/streams_bootstrap/test_streams_app.py Updates dry-run and validation fallback tests to assert structured logs.
tests/components/streams_bootstrap/test_producer_app.py Updates validation fallback tests + dry-run diff call assertions.
tests/components/streams_bootstrap/test_consumer_app.py Updates dry-run and validation fallback tests to assert structured logs.
tests/components/streams_bootstrap_v2/test_streams_app.py Updates dry-run and validation fallback tests to assert structured logs.
tests/components/streams_bootstrap_v2/test_producer_app.py Updates validation fallback tests + dry-run diff call assertions.
tests/component_handlers/utils/test_exception.py New tests for shared service exception base behavior (reason extraction, body parsing, etc.).
tests/component_handlers/utils/init.py Package marker for utils tests.
tests/component_handlers/topic/test_topic_handler.py Replaces patched stdlib logger calls with structured-log assertions.
tests/component_handlers/topic/test_kafka_rest.py Updates to pytest_httpx2 request inspection; adds connection/umbrella exception tests; asserts structured logs.
tests/component_handlers/schema_handler/test_schema_handler.py Replaces patched logger assertions with structured-log assertions; updates warning content/fields.
tests/component_handlers/kafka_connect/test_kafka_connect_handler.py Replaces patched logger assertions with structured-log assertions; verifies structured “diff header/body” logging.
tests/component_handlers/kafka_connect/test_kafka_connect_api.py Replaces caplog assertions with structured-log assertions; adds connection/umbrella exception tests.
tests/component_handlers/helm_wrapper/test_helm_wrapper.py Updates exception class expectations; asserts structured warning logs for release-not-found.
tests/component_handlers/helm_wrapper/test_helm_diff.py Migrates helm diff log assertion to structlog capture.
tests/component_handlers/helm_wrapper/test_dry_run_handler.py Migrates dry-run handler tests to structlog capture + structured event assertions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread kpops/utils/logging.py Outdated
Comment on lines +139 to +141
tokens = structlog.contextvars.bind_contextvars(**kw)
yield
structlog.contextvars.reset_contextvars(**tokens)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's wrong. when an exception is raised, it propagates out of the generator at the yield expression. the reset (after yield) only runs for the success path, not the exception path. therefore, the context remains bound after the exception

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@disrupted yeah you're right. But that also means the context stays attached for exceptions that we recover from, right? (for example except TopicNotFoundException in __execute_topic_creation)
Do you think this could lead to context becoming cluttered and stale?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tobbber good point, the context could indeed leak into later exceptions. to fix it, contextvars is now always reset in the finally block. error context is stored directly inside KpopsException as dict.

Comment thread kpops/component_handlers/kafka_connect/timeout.py Outdated
Comment thread tests/conftest.py Outdated
@disrupted
disrupted requested a review from tobbber August 10, 2026 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve error logging

3 participants