Skip to content

Strengthen TOML parse-error guarantees and clarify tracing key precedence#6640

Merged
lpcox merged 3 commits into
mainfrom
copilot/go-fan-rev-burntsushi-toml
May 28, 2026
Merged

Strengthen TOML parse-error guarantees and clarify tracing key precedence#6640
lpcox merged 3 commits into
mainfrom
copilot/go-fan-rev-burntsushi-toml

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 28, 2026

This update tightens BurntSushi/toml integration in internal/config by ensuring structured parse errors remain unwrap-able and by making dual tracing-key behavior explicit. It also documents an intentional TOML key naming exception in guard policy config.

  • Parse error contract hardening

    • TestLoadFromFile_InvalidTOML now asserts the returned error unwraps to toml.ParseError and carries non-zero line/column position metadata.
    • This protects against future wrapping regressions that would drop structured parse context.
  • Tracing key precedence visibility

    • LoadFromFile now emits a warning when both [gateway.tracing] and [gateway.opentelemetry] are present.
    • Existing behavior is preserved: opentelemetry continues to override legacy tracing.
  • Precedence behavior coverage

    • Added TestLoadFromFile_BothTracingAndOpenTelemetry_OpenTelemetryTakesPrecedence to verify merged result uses opentelemetry values and clears the temporary alias field.
  • Guard policy key documentation

    • Added a focused comment on GuardPolicy noting hyphenated TOML keys (e.g. allow-only) are intentional to match policy schema field names.
if cfg.Gateway.Opentelemetry != nil {
    if cfg.Gateway.Tracing != nil {
        logConfig.Print("Warning: both [gateway.tracing] and [gateway.opentelemetry] are set; [gateway.opentelemetry] takes precedence")
    }
    cfg.Gateway.Tracing = cfg.Gateway.Opentelemetry
    cfg.Gateway.Opentelemetry = nil
}

Copilot AI linked an issue May 28, 2026 that may be closed by this pull request
4 tasks
Copilot AI changed the title [WIP] Review Go module BurntSushi/toml for usage in gh-aw Strengthen TOML parse-error guarantees and clarify tracing key precedence May 28, 2026
Copilot finished work on behalf of lpcox May 28, 2026 16:14
Copilot AI requested a review from lpcox May 28, 2026 16:14
@lpcox lpcox marked this pull request as ready for review May 28, 2026 16:16
Copilot AI review requested due to automatic review settings May 28, 2026 16:16
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

Tightens TOML integration in internal/config by asserting that parse errors expose structured toml.ParseError data, makes the precedence between [gateway.tracing] and [gateway.opentelemetry] explicit with a warning log and test, and documents intentional hyphenated TOML key names on GuardPolicy.

Changes:

  • Emit a warning when both tracing and opentelemetry gateway sections are set; opentelemetry still wins.
  • Strengthen invalid-TOML test to verify errors.As(toml.ParseError) with non-zero line/column.
  • Add precedence test covering combined tracing + opentelemetry config and document hyphenated keys on GuardPolicy.
Show a summary per file
File Description
internal/config/config_core.go Logs warning when both tracing and opentelemetry are configured.
internal/config/config_core_test.go Adds parse-error structural assertions and a precedence test.
internal/config/guard_policy.go Comment clarifying intentional hyphenated TOML keys.

Copilot's findings

Tip

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

  • Files reviewed: 3/3 changed files
  • Comments generated: 0

@lpcox lpcox merged commit ca6b82b into main May 28, 2026
29 checks passed
@lpcox lpcox deleted the copilot/go-fan-rev-burntsushi-toml branch May 28, 2026 16:20
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.

[go-fan] Go Module Review: BurntSushi/toml

3 participants