Skip to content

Remove automatic cross-compatibility for hooks/middleware#1316

Merged
brandur merged 1 commit into
masterfrom
brandur-remove-hook-middleware-compatibility
Jul 22, 2026
Merged

Remove automatic cross-compatibility for hooks/middleware#1316
brandur merged 1 commit into
masterfrom
brandur-remove-hook-middleware-compatibility

Conversation

@brandur

@brandur brandur commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

This one's related to #1284. In that pull request, we made it so that
any hooks sent to Config.Hooks that implemented middleware or any
middleware sent to Config.Middleware that implement hooks were
automatically extracted and use for all types.

I've been thinking about this for the last couple of days, and in
retrospect, I think this automatic compatibility might be a little
unnecessarily aggressive. The case in favor is that when adding
additional hook-related functionality to something like otelriver, the
user can just leave in their existing instance in middleware and not
change anything to get the existing functionality. (Without the automatic
compatibility they'd have to move it to Config.Plugins instead.) The
case against is that it may result in some surprising behavior if you
only intended to add something as either a hook or middleware.

We haven't shipped a release containing #1284 yet, so we can still
change this if we want to. I didn't ship a strict revert because #1284
also brings in Config.Plugins and some useful internal changes to
support everything.

This one's related to #1284. In that pull request, we made it so that
any hooks sent to `Config.Hooks` that implemented middleware or any
middleware sent to `Config.Middleware` that implement hooks were
automatically extracted and use for all types.

I've been thinking about this for the last couple of days, and in
retrospect, I think this automatic compatibility might be a little
unnecessarily aggressive. The case in favor is that when adding
additional hook-related functionality to something like `otelriver`, the
user can just leave in their existing instance in middleware and not
change anything to get the existing functionality. (Without the automatic
compatibility they'd have to move it to `Config.Plugins` instead.) The
case against is that it may result in some surprising behavior if you
only intended to add something as either a hook or middleware.

We haven't shipped a release containing #1284 yet, so we can still
change this if we want to. I didn't ship a strict revert because #1284
also brings in `Config.Plugins` and some useful internal changes to
support everything.
@brandur
brandur marked this pull request as ready for review July 22, 2026 04:51
@brandur

brandur commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Thanks!

Okay, I couldn't fully decide whether to merge this one or not (can make arguments both directions), so I sent it through Codex and asked it to be as objective as possible:

Final recommendation

Merge the branch.

The automatic approach is defensible only if the firm long-term direction is that Hooks and Middleware are deprecated aliases for Plugins, and seamless activation of new
integration features is considered more important than explicit configuration.

The current branch instead keeps all three fields supported, as documented in client.go:242 and CHANGELOG.md:16. Under that strategy, role-specific semantics are more coherent,
safer, more expressive, and more compatible with previously released behavior.

I would coordinate the merge with an otelriver migration example showing existing users that hybrid functionality now belongs in Config.Plugins.

That works for me, so going to pull this in.

@brandur
brandur merged commit f19ad67 into master Jul 22, 2026
15 checks passed
@brandur
brandur deleted the brandur-remove-hook-middleware-compatibility branch July 22, 2026 04:52
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