fix: no longer drop request if stream is dropped in loki.source.api by kalleep · Pull Request #4834 · grafana/alloy

kalleep · 2025-11-13T15:49:54Z

PR Description

This pr solves several issues in loki.source.api.

No longer cancel request if stream is dropped by relabel rules, this is clearly a return instead of continue bug.
Add function ForceShutdown that will cancel all request before shutting down server.
- When component is shutting down we are not doing any draining like in loki.source.file. And IMO we should not do it here. It's better to just cancel all in-flight requests so that caller can retry.
Send batches instead of individual entries from request handler to component. This makes it a bit more transaction safe where we don't partially ingest entries if request is either canceled or a shutdown happens.

Which issue(s) this PR fixes

Notes to the Reviewer

PR Checklist

CHANGELOG.md updated
Documentation added
Tests updated
Config converters updated

Copilot

Pull Request Overview

This PR fixes a critical bug in loki.source.api where requests were incorrectly cancelled when relabel rules dropped a specific stream (instead of continuing to process remaining streams). Additionally, it refactors the component to send batches of entries rather than individual entries, improving transactional safety during shutdown scenarios.

Key changes:

Fixed bug where return should have been continue when a stream is dropped by relabel rules
Added ForceShutdown() method to cancel all in-flight requests before server shutdown
Refactored to send batches of entries instead of individual entries for better atomicity

Reviewed Changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`internal/component/loki/source/api/internal/lokipush/push_api_server.go`	Fixed the relabel rules bug (`continue` instead of `return`), added `ForceShutdown()`, and changed to batch sending
`internal/component/loki/source/api/internal/lokipush/push_api_server_test.go`	Added `fakeBatchReceiver` test helper to support batch-based testing
`internal/component/loki/source/api/api.go`	Changed handler from `LogsReceiver` to `LogsBatchReceiver` and updated `Run()` to process batches
`internal/component/loki/source/api/api_test.go`	Updated tests to remove manual cleanup now handled by context cancellation
`internal/component/common/loki/receiver.go`	New file containing receiver interfaces, extracted from `entry_handler.go`
`internal/component/common/loki/entry.go`	New file containing `Entry` type definition, extracted from `entry_handler.go`
`internal/component/common/loki/entry_handler.go`	Removed receiver and entry code that was moved to separate files
`internal/component/common/loki/entry_handler_test.go`	Added test coverage for label middleware functionality
`CHANGELOG.md`	Added entry documenting the bug fix

internal/component/common/loki/receiver.go

internal/component/loki/source/api/internal/lokipush/push_api_server_test.go

internal/component/loki/source/api/internal/lokipush/push_api_server.go

thampiotr · 2025-11-13T16:40:02Z

internal/component/loki/source/api/internal/lokipush/push_api_server.go

+		}
+
+		if lastErr != nil {
+			level.Warn(s.logger).Log("msg", "at least one entry in the push request failed to process", "err", lastErr.Error())


I think it would be cheap and easy to add a counter of how many entries failed out of how many in the batch. And that would be useful.

Hm after checking what kind of metrics other exposes we could add two metrics:

loki_source_api_entries_processed - this would be the total number of entries in a request

loki_source_api_entries_written - this would be the total number of entries that we have forwarded

With that you could derive how many are dropped either by relabeling or by invalid labels, WDYT?

I went with my suggestion. Let me know what you think :)

thampiotr · 2025-11-13T16:41:31Z

internal/component/loki/source/api/internal/lokipush/push_api_server.go

-				w.WriteHeader(http.StatusServiceUnavailable)
-				return
-			}
+			entries = append(entries, e)


Might want to reuse the slices in a pool if this comes up in allocations.

Agree, do you think we should do that now or wait and see if it shows up?

If we go down the pooling path could we try to avoid sending it through a channel? It's fine today because we know the implementation is an unbuffered channel but it feels risky to depend upon that given the server can't enforce it.

Let's wait to see if this comes up in profiles.

github-actions · 2025-11-14T08:56:07Z

💻 Deploy preview deleted (fix: no longer drop request if stream is dropped in loki.source.api).

thampiotr · 2025-11-17T11:30:16Z

docs/sources/reference/components/loki/loki.source.api.md

+* `loki_source_api_entries_processed` (counter): Total number of log entries processed.
+* `loki_source_api_entries_written` (counter): Total number of log entries forwarded.


I think we do want some metrics, but we also don't want to have too many metrics. Should we settle on only one off them? I think loki_source_api_entries_written would be better option to keep. The idea is to be able to spot some major issues and then continue debugging using other means like logging, profiles or local repro.

Sure, only kept loki_source_api_entries_written

thampiotr

LGMT % adding only one debug metric instead of two

stopping server

This will ensure that all entries in a request is sent down the pipeline.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…4834) * fix: add ForceShutdown that will cancel in-flight requests before stopping server * Split into multiple files and add LogsBatchReceiver * don't drop request when relabel rules drops a specific stream * fix: use loki.LogsBatchReceiver to ensure all entries in a request is sent down the pipeline * add changelog * add checks for entries and use sync once to close channel

* fix: no longer drop request if stream is dropped in loki.source.api (#4834) * fix: add ForceShutdown that will cancel in-flight requests before stopping server * Split into multiple files and add LogsBatchReceiver * don't drop request when relabel rules drops a specific stream * fix: use loki.LogsBatchReceiver to ensure all entries in a request is sent down the pipeline * add changelog * add checks for entries and use sync once to close channel * update changelog for next rc * Fix flaky tests: port in for loki source api tests and logs integration test (#4875) * Fix port in use flakyness for loki source api tests * Pin loki container version for integration tests * Add a new mimir.alerts.kubernetes component (#3448) * Add a new mimir.alerts.kubernetes component * Sync Mimir periodically, test the case of a CRD deletion * Add TODOs * Longer test timeout * Check if pods are running * Apply suggestions from code review Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com> * Fix metric doc * Fix changelog --------- Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com> * Remove experimental flag from stage.windowsevent (#4879) --------- Co-authored-by: Karl Persson <23356117+kalleep@users.noreply.github.com> Co-authored-by: Kyle Eckhart <kgeckhart@users.noreply.github.com> Co-authored-by: Paulin Todev <paulin.todev@gmail.com> Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>

…rafana#4834) * fix: add ForceShutdown that will cancel in-flight requests before stopping server * Split into multiple files and add LogsBatchReceiver * don't drop request when relabel rules drops a specific stream * fix: use loki.LogsBatchReceiver to ensure all entries in a request is sent down the pipeline * add changelog * add checks for entries and use sync once to close channel

kalleep requested a review from a team as a code owner November 13, 2025 15:49

kalleep requested a review from Copilot November 13, 2025 15:50

Copilot started reviewing on behalf of kalleep November 13, 2025 15:50 View session

Copilot finished reviewing on behalf of kalleep November 13, 2025 15:53

Copilot AI reviewed Nov 13, 2025

View reviewed changes

thampiotr reviewed Nov 13, 2025

View reviewed changes

kalleep requested a review from clayton-cornell as a code owner November 14, 2025 08:54

kalleep requested a review from thampiotr November 14, 2025 10:20

kalleep force-pushed the kalleep/loki-source-api-fixes branch 2 times, most recently from 2b5b401 to 1dfccfc Compare November 17, 2025 08:02

thampiotr reviewed Nov 17, 2025

View reviewed changes

thampiotr approved these changes Nov 17, 2025

View reviewed changes

kalleep and others added 14 commits November 17, 2025 13:01

fix: Add ForceShutdown that will cancel in-flight requests before

a3db273

stopping server

Split into multiple files and add LogsBatchReceiver

ce5997e

Don't drop request when relabel rules drops a specific stream

74e7cba

fix: use loki.LogsBatchReceiver

d839a12

This will ensure that all entries in a request is sent down the pipeline.

add changelog

fcab7f5

Update comment

5dec84a

Update internal/component/common/loki/receiver.go

3423ca1

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

add checks for entries and use sync once to close channel

914dfc3

lint

1158a45

Add metrics

73c819e

Update docs

743ddb6

remove unused variable

3e1ee5b

fix test

12b9986

remove one metric

73e08d5

kalleep force-pushed the kalleep/loki-source-api-fixes branch from a66e3fc to 73e08d5 Compare November 17, 2025 12:02

kalleep merged commit 74653ac into main Nov 17, 2025
43 of 46 checks passed

kalleep deleted the kalleep/loki-source-api-fixes branch November 17, 2025 13:38

jharvey10 mentioned this pull request Nov 18, 2025

v1.12 rc.1 content #4877

Merged

github-actions bot added the frozen-due-to-age label Dec 19, 2025

github-actions bot locked as resolved and limited conversation to collaborators Dec 19, 2025

		* `loki_source_api_entries_processed` (counter): Total number of log entries processed.
		* `loki_source_api_entries_written` (counter): Total number of log entries forwarded.

Conversation

kalleep commented Nov 13, 2025

PR Description

Which issue(s) this PR fixes

Notes to the Reviewer

PR Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kalleep Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thampiotr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kalleep Nov 14, 2025 •

edited

Loading

github-actions bot commented Nov 14, 2025 •

edited

Loading