Skip to content

diagnostics: disable nginx reverse proxy buffering on configd streams - #10818

Open
Greelan wants to merge 3 commits into
opnsense:masterfrom
Greelan:sse-proxy-buffering
Open

diagnostics: disable nginx reverse proxy buffering on configd streams#10818
Greelan wants to merge 3 commits into
opnsense:masterfrom
Greelan:sse-proxy-buffering

Conversation

@Greelan

@Greelan Greelan commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Important notices

Before you submit a pull request, we ask you kindly to acknowledge the following:

If AI was used, please disclose:

  • Model used:
  • Extent of AI involvement:

Describe the problem

The dashboard traffic and CPU widgets and the live firewall log stutter behind a buffering nginx reverse proxy.


Describe the proposed solution

These event streams follow the same pattern as the live log widget, so adding the same X-Accel-Buffering header as was added for the live log widget in #10086.

The alternative is to do it centrally in ApiControllerBase.php.


Related issue

N/A

@Greelan
Greelan force-pushed the sse-proxy-buffering branch from a0a79b6 to 5e22d2e Compare August 28, 2026 23:44
@AdSchellevis

Copy link
Copy Markdown
Member

Maybe its better to merge a header before

Personally I do find their header a bit obscure as we're already asking to not cache this stream with standard headers, in practice other webservers will usually just drop this anyway.

@Greelan

Greelan commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

Right, that was the alternative I mentioned. Wasn't sure you would want Base touched. Anyway, done now.

@AdSchellevis

Copy link
Copy Markdown
Member

in this case it might be better, but is X-Accel-Buffering really connected to a certain type? I would expect streams should never be cached in between..

@Greelan

Greelan commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author

I was focused on SSE. I think the only other use is for CSV exports, where buffering may actually help?

@AdSchellevis

Copy link
Copy Markdown
Member

I think all current callers use the same type, but if at some point it seems to make sense to filter these, we can have a look then. For now I would prefer a single strategy and wait for feedback. When there is a valid reason to not do this, we should probably add the tag with a yes and filter on the header itself.

@Greelan

Greelan commented Aug 30, 2026

Copy link
Copy Markdown
Contributor Author

This is what I was referring to:

return $this->configdStream(
'system diag log_stream',
[0, 0, $searchPhrase, $module, $scope, $severities, $validFrom],
[
'Content-Type: text/csv',
'Content-Disposition: attachment; filename=' . $scope . '.log',
'Content-Transfer-Encoding: binary',
'Pragma: no-cache',
'Expires: 0'
],

But I can remove the type test if that is still your preference.

@AdSchellevis

Copy link
Copy Markdown
Member

let's remove the test, if someone complains, we'll see

@Greelan

Greelan commented Aug 30, 2026

Copy link
Copy Markdown
Contributor Author

OK, done.

@Greelan Greelan changed the title diagnostics: disable nginx reverse proxy buffering on traffic, cpu and firewall SSE streams diagnostics: disable nginx reverse proxy buffering on configd streams Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants