Skip to content

Conversation

@ivkalita
Copy link
Contributor

@ivkalita ivkalita commented Oct 23, 2025

Implements HTTP/2 transport layer with FrameProtocol interface, HTTP2Listener/Conn, and JSONProtocol. Supports concurrent clients via per-connection protocol instances.

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

There are some issues I would like to fix in the follow up PRs (this PR is already big):

  1. JSONProtocol - this should be replaced by a binary protocol of some kind (protobuf?). Added JSON protocol only to have e2e tests.
  2. The protocol blocks on reading. E.g., if a sender is slow or sends a message only partially the reader will be blocked until the connection is closed.
  3. The code is not instrumented at all.

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • Title matches the required conventional commits format, see here
    • Note that Promtail is considered to be feature complete, and future development for logs collection will be in Grafana Alloy. As such, feat PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
  • If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

Implements HTTP/2 transport layer with FrameProtocol interface,
HTTP2Listener/Conn, and JSONProtocol. Supports concurrent clients via
per-connection protocol instances.
@ivkalita ivkalita force-pushed the ivkalita/wire-transport-http2 branch from f7fa071 to 57c2f02 Compare October 24, 2025 08:10
@ivkalita ivkalita marked this pull request as ready for review October 24, 2025 08:25
@ivkalita ivkalita requested a review from a team as a code owner October 24, 2025 08:25
Copy link
Member

@rfratto rfratto left a comment

Choose a reason for hiding this comment

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

Looks like this is heading in the right direction. Once we serialize the physical plans as protobuf, I think we should also be serializing the frames as protobuf as well.

}

// NewHTTP2Listener creates a new HTTP/2 listener on the specified address.
func NewHTTP2Listener(
Copy link
Member

Choose a reason for hiding this comment

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

Could we make this work by exposing an http.Handler, so that way we can reuse the existing HTTP server Loki exposes?

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants