Skip to content

Replace external hclog-slog adapter with an in-tree one in the Go SDK - #70918

Merged
potiuk merged 3 commits into
apache:mainfrom
jason810496:ci/go-sdk/drop-go-hclog-slog
Aug 3, 2026
Merged

Replace external hclog-slog adapter with an in-tree one in the Go SDK#70918
potiuk merged 3 commits into
apache:mainfrom
jason810496:ci/go-sdk/drop-go-hclog-slog

Conversation

@jason810496

Copy link
Copy Markdown
Member

Why

In plugin mode, bundlev1server installs an slog default logger that forwards through an hclog.Logger so the go-plugin host can parse the plugin's stderr. That bridge came from the external github.com/evanphx/go-hclog-slog module, which ships no license file and no tagged release (it is pinned to a bare commit). That makes it awkward to depend on and to reason about.

The adapter is small and we already maintain its mirror image in-tree — pkg/logging/shclog presents an slog Logger as an hclog. This adds the inverse and drops the external module.

What

  • Add pkg/logging/hclogslog: a small slog.Handler that forwards records to an hclog.Logger, translating levels and flattening grouped attributes with dotted keys (100% unit-test covered).
  • Point bundlev1server at the in-tree package (the call site — hclogslog.Adapt(hcLogger) — is unchanged).
  • go mod tidy drops github.com/evanphx/go-hclog-slog from go-sdk/go.mod / go-sdk/go.sum; re-tidy kubernetes-tests/lang_sdk/go_example to drop its stale indirect entry.

Plugin logging behaviour is identical.


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 4.8)

Generated-by: Claude Code (Opus 4.8) following the guidelines

The plugin-mode bundle server routes its slog default logger through hclog so
the go-plugin host can parse the plugin's stderr. That bridge came from the
external github.com/evanphx/go-hclog-slog module, which ships no license and no
tagged release. Replace it with a small in-tree slog.Handler in
pkg/logging/hclogslog (the inverse of the existing pkg/logging/shclog), keeping
the plugin logging behaviour identical while removing the external module. The
lang-SDK Go example module, which resolves the SDK via a replace directive, is
re-tidied to drop the now-unused indirect entry.
An empty-keyed attribute bound with slog's With and an empty-keyed
attribute on the record itself both fell back to a positional key that
each scope numbered from zero. At the same group prefix they produced
the same numeric key, so hclog emitted a duplicate JSON key and the
plugin host's decoder silently kept only the last value, dropping the
bound one. Carry the bound attributes' position count into the record so
their synthesized keys no longer overlap.
@jason810496
jason810496 marked this pull request as ready for review August 3, 2026 06:15
@potiuk

potiuk commented Aug 3, 2026

Copy link
Copy Markdown
Member

good calls!

@potiuk potiuk added the backport-to-v3-3-test Backport to v3-3-test label Aug 3, 2026
@potiuk
potiuk merged commit 3cfbbe6 into apache:main Aug 3, 2026
184 checks passed
@potiuk potiuk added this to the Airflow 3.3.1 milestone Aug 3, 2026
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Backport failed to create: v3-3-test. View the failure log Run details

Note: As of Merging PRs targeted for Airflow 3.X
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.

In matter of doubt please ask in #release-management Slack channel.

Status Branch Result
v3-3-test Commit Link

You can attempt to backport this manually by running:

cherry_picker 3cfbbe6 v3-3-test

This should apply the commit to the v3-3-test branch and leave the commit in conflict state marking
the files that need manual conflict resolution.

After you have resolved the conflicts, you can continue the backport process by running:

cherry_picker --continue

If you don't have cherry-picker installed, see the installation guide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants