Skip to content

feat: config validation improvements, wire DebugLogInterceptor#74

Merged
ankurs merged 3 commits intomainfrom
feat/config-validation-debug-interceptor
Apr 9, 2026
Merged

feat: config validation improvements, wire DebugLogInterceptor#74
ankurs merged 3 commits intomainfrom
feat/config-validation-debug-interceptor

Conversation

@ankurs
Copy link
Copy Markdown
Member

@ankurs ankurs commented Apr 9, 2026

Summary

Config validation (ROADMAP 4.1):

  • TLS file existence check via os.Stat when both cert and key are set
  • OTLP endpoint host:port format validation via net.SplitHostPort
  • Log level validation against known levels (error, warn, warning, info, debug)
  • Timeout vs shutdown duration sanity warning

DebugLogInterceptor wiring:

  • Add DisableDebugLogInterceptor bool and DebugLogHeaderName string config fields
  • Wire to interceptors in processConfig()
  • Update getCustomHeaderMatcher to variadic headers — forwards debug log header from HTTP to gRPC metadata alongside trace header

Goroutine leak detection:

  • Add goleak.VerifyTestMain with documented ignores for third-party singletons

Depends on: go-coldbrew/interceptors#38 (DebugLogInterceptor)

Test plan

  • 4 new config validation tests (TLS files, OTLP format, log level, timeout sanity)
  • All existing core tests pass
  • make test and make lint clean (tested with local replace directive for interceptors)

Summary by CodeRabbit

Release Notes

  • New Features

    • Added configuration options to disable debug logging and customize the debug log header name.
    • Enhanced startup validation with warnings for missing TLS files, invalid OTLP endpoint format, unrecognized log levels, and timeout conflicts.
  • Improvements

    • Improved HTTP to gRPC header forwarding to support multiple custom headers.
  • Dependencies

    • Updated core dependencies to latest versions.

Config validation (4.1):
- TLS file existence check via os.Stat
- OTLP endpoint host:port format validation
- Log level validation against known levels
- Timeout vs shutdown duration sanity check

DebugLogInterceptor wiring:
- Add DisableDebugLogInterceptor and DebugLogHeaderName config fields
- Wire to interceptors in processConfig
- Update getCustomHeaderMatcher to variadic headers for HTTP support

Also adds goleak.VerifyTestMain for goroutine leak detection.
Copilot AI review requested due to automatic review settings April 9, 2026 05:35
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 9, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8253bf4e-92a6-440c-aa6f-e3cc336df329

📥 Commits

Reviewing files that changed from the base of the PR and between 22416eb and 01f2c6a.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (6)
  • config/config.go
  • config/config_test.go
  • core.go
  • core_test.go
  • go.mod
  • goleak_test.go

📝 Walkthrough

Walkthrough

This PR extends the configuration system with new interceptor and logging controls, adds comprehensive startup validation for TLS files, OTLP endpoint format, log levels, and gRPC timeout settings. It updates header matching logic to support multiple headers and adds goleak dependency for goroutine leak detection in tests.

Changes

Cohort / File(s) Summary
Configuration Fields & Validation
config/config.go, config/config_test.go
Added DisableDebugLogInterceptor and DebugLogHeaderName config fields; extended Validate() to check TLS file existence via os.Stat, validate OTLP endpoint format using net.SplitHostPort, verify log level against recognized values, and warn when gRPC server timeout exceeds shutdown duration. Comprehensive test coverage added for all new validation paths.
Core Interceptor & Header Matching
core.go, core_test.go
Updated HTTP→gRPC header matcher to support variadic headers and exact-match multiple header names; configured debug log interceptor initialization based on new config fields; added test coverage for multi-header matching behavior.
Dependencies & Test Infrastructure
go.mod, goleak_test.go
Updated minor/patch versions for go-coldbrew packages (errors, interceptors, log, tracing) and added new direct dependency go.uber.org/goleak v1.3.0; added TestMain in core package to configure goroutine leak detection with ignores for known background goroutines from gRPC, OpenTelemetry, Sentry, and rollbar-go.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • vestor
  • fajran
  • kevinjom
  • svetha-cvl

Poem

🐰 twitches whiskers with delight
Headers multiply, validation shines bright,
Debug logs leap where they should go,
Goleak watches goroutines flow—
Config wisdom flows just right! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the two main objectives: configuration validation improvements and DebugLogInterceptor wiring, matching the primary changes across config and core files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/config-validation-debug-interceptor

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 improves startup-time configuration validation, wires the new DebugLogInterceptor controls into the core interceptor setup, and adds goroutine leak detection for the core package test suite.

Changes:

  • Added additional Config.Validate() checks (TLS file existence, OTLP host:port formatting, log level validation, and timeout vs shutdown sanity warning) plus new unit tests.
  • Wired DisableDebugLogInterceptor / DebugLogHeaderName into processConfig() and forwarded the debug header from HTTP gateway to gRPC metadata via an updated header matcher.
  • Added goleak.VerifyTestMain for core tests and introduced go.uber.org/goleak as a dependency.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
goleak_test.go Adds a TestMain to verify goroutine leaks with documented ignores.
go.mod Adds the go.uber.org/goleak dependency.
core.go Wires DebugLogInterceptor config and forwards debug header via grpc-gateway header matcher.
config/config.go Adds new config fields and validation warnings for common misconfigurations.
config/config_test.go Adds tests covering the new config validation behavior.

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

Comment thread go.mod
Comment thread config/config.go Outdated
Comment thread core.go
Comment thread core.go
Address review: use os.IsNotExist for "not found" vs general "could not
be accessed" with error details. Add TestGetCustomHeaderMatcher_MultipleHeaders
for variadic header matching.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.


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

Comment thread config/config.go
@ankurs ankurs merged commit de96de5 into main Apr 9, 2026
7 checks passed
@ankurs ankurs deleted the feat/config-validation-debug-interceptor branch April 9, 2026 07:39
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.

2 participants