Adarsh879 opened a new pull request, #13326:
URL: https://github.com/apache/apisix/pull/13326
### Description
`loki-logger` mutates `conf.log_labels` in place when substituting
`$variable` placeholders, then captures that shared table in the
batch-processor flush closure. The early return at `add_entry` causes
all subsequent requests on the same worker to skip the resolution
block, so every later entry flushes with the **first** request's
resolved labels. Symptom: log entries where `log_labels` and
`log_format` disagree on the same line (#12485), or a stream label
that carries a value from a different request than the one whose
body it accompanies.
The bug affects `loki-logger` at any scope where multiple requests
share the same conf and a `$variable` in `log_labels` resolves to
different values per request — so it surfaces in real deployments
both at global-rule and at service scope.
#### Root cause (pre-fix)
```lua
if batch_processor_manager:add_entry(conf, entry, max_pending_entries) then
return -- subsequent requests bypass everything
below
end
local labels = conf.log_labels -- alias, NOT a copy
for key, value in pairs(labels) do
local new_val, err, n_resolved = core.utils.resolve_var(value, ctx.var)
if not err and n_resolved > 0 then
labels[key] = new_val -- mutates conf.log_labels
end
end
local func = function(entries) -- closure captures the shared table
local data = { streams = { { stream = labels, values = ... } } }
...
end
```
#### Fix
- Resolve labels **per request** into a fresh table; never mutate
`conf.log_labels`.
- Attach the resolved labels to each entry (`entry.loki_labels`).
- At flush time, group entries by their resolved label set and emit
one Loki stream per group within the push (Loki's `streams` API
already supports this, so batching is preserved).
The change is fully contained to `apisix/plugins/loki-logger.lua` —
nothing in `plugin.lua`, `batch-processor-manager.lua`, or other
plugins is touched.
#### Test
`t/plugin/loki-logger.t` TEST 17–19 (modeled on the existing TEST
12–14 pattern):
1. Configure a route with `log_labels: { custom_label: "$arg_X" }`
and `batch_max_size: 1`.
2. Hit `/hello?X=alpha` then `/hello?X=beta`.
3. Query Loki for `{custom_label="beta"}`. Without the fix the
second request's value never reaches Loki (the worker resolved
`"alpha"` once and reused it forever), so the assertion fails.
With the fix, both stream labels exist.
Verified locally:
| Plugin source | Result |
|----------------------------------------|----------|
| Pre-fix (master before this PR) | TEST 19 fails: `no entries with
custom_label=beta` |
| Post-fix (this PR) | All 54 subtests pass |
#### Which issue(s) this PR fixes:
Fixes #12485
### Checklist
- [x] I have explained the need for this PR and the problem it solves
- [x] I have explained the changes or the new features added to this PR
- [x] I have added tests corresponding to this change
- [x] I have updated the documentation to reflect this change *(no doc
change needed; bug fix, no surface change)*
- [x] I have verified that this change is backward compatible
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
