pFad - Phone/Frame/Anonymizer/Declutterfier! Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

URL: http://github.com/apache/apisix/pull/13315

ps://github.githubassets.com/assets/actions-902e75f4f51a80db.css" /> fix(opentelemetry): preserve booleans, handle multi-value headers, tighten test by shreemaan-abhishek · Pull Request #13315 · apache/apisix · GitHub
Skip to content

fix(opentelemetry): preserve booleans, handle multi-value headers, tighten test#13315

Open
shreemaan-abhishek wants to merge 2 commits intoapache:masterfrom
shreemaan-abhishek:fix/otel-additional-attrs
Open

fix(opentelemetry): preserve booleans, handle multi-value headers, tighten test#13315
shreemaan-abhishek wants to merge 2 commits intoapache:masterfrom
shreemaan-abhishek:fix/otel-additional-attrs

Conversation

@shreemaan-abhishek
Copy link
Copy Markdown
Contributor

Description

Three follow-ups to #13146 (which started coercing additional_attributes values to strings via tostring() before passing them to attr.string()). The patch as merged still has three discrete gaps:

1. Boolean false is silently dropped

inject_attributes() gates insertion on if val then. Lua treats false as falsy, so any additional_attribute whose resolved value is the boolean false (from an nginx variable, or set explicitly by a plugin) never makes it into the span at all. Switching to if val ~= nil then is the standard Lua idiom for "exists, including false".

2. Multi-value headers stringify to "table: 0x..."

When additional_attributes references a header source via prefix-match (X-Foo-*), the value comes from core.request.headers(api_ctx) which wraps ngx.req.get_headers(). That can return a Lua table for multi-value headers (multiple Set-Cookie, multiple X-Forwarded-For, etc.). tostring(some_table) then emits "table: 0x7f..." and that's what ends up as the OTel attribute value — useless.

This PR extracts a coerce_attr_value(val) helper that:

  • Returns nil for nil (caller skips the attribute).
  • Joins table values with ", " (table.concat).
  • Falls through to tostring() for everything else.

Both the prefix-match path and the keyed path go through the helper.

3. TEST 22 is too loose to detect a regression

The new TEST 22's response_body assertion is:

qr/.*opentelemetry-lua.*/

That's the OTel exporter's library-name marker; every OTel export contains it, regardless of whether additional_attributes actually made it into the span. The test would still pass with the regression #13146 was supposed to fix.

Tighten it to assert each of request_time and bytes_sent appears as a "stringValue" in the export:

qr/.*opentelemetry-lua.*"key":"request_time","value":\{"stringValue":"[^"]+"\}.*"key":"bytes_sent","value":\{"stringValue":"[^"]+"\}.*/s

upstream_response_time is intentionally not asserted: TEST 21 calls /opentracing which is served directly without proxy_pass, so $upstream_response_time is nil and the plugin correctly omits the attribute. Asserting it would fail for the wrong reason.

Which issue(s) this PR fixes:

Follow-up to #13146; no separate issue.

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change — TEST 22 in t/plugin/opentelemetry.t is now a real regression guard for the string-coercion contract.
  • I have updated the documentation to reflect this change — no doc impact; the plugin's documented contract is unchanged.
  • I have verified that this change is backward compatible

…ghten test

Three follow-ups to apache#13146 (which started coercing additional_attributes
values to strings before sending them to OTel attr.string):

1. inject_attributes() gated insertion on `if val then`, which silently
   drops boolean false (Lua falsy) — regressing any user whose nginx
   variable resolves to a boolean false flag. Switch to
   `if val ~= nil then` so false survives.

2. ngx.req.get_headers() returns a Lua table for multi-value headers
   (e.g. multiple Set-Cookie / X-Forwarded-For entries). tostring()
   over a table emits 'table: 0x...' and that's what was landing in
   the span attribute. Extract a coerce_attr_value() helper that joins
   multi-value entries with ', ' and skips when the source is missing
   entirely.

3. TEST 22's response_body regex was qr/.*opentelemetry-lua.*/ which
   matches the OTLP exporter marker even when the numeric attributes
   are entirely absent from the export — i.e. the test would still
   pass with the regression apache#13146 was supposed to fix. Tighten it to
   assert each of request_time / bytes_sent appears as a stringValue.
   (upstream_response_time is intentionally not asserted: TEST 21
   serves /opentracing directly without proxying, so
   $upstream_response_time is nil and the plugin correctly omits the
   attribute.)

Signed-off-by: Shreemaan Abhishek <shreemaan.abhishek@gmail.com>
Signed-off-by: Abhishek Choudhary <shreemaan.abhishek@gmail.com>
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. bug Something isn't working labels Apr 29, 2026
AlinsRan
AlinsRan previously approved these changes Apr 30, 2026
membphis
membphis previously approved these changes Apr 30, 2026
…ional-attrs

Signed-off-by: Abhishek Choudhary <shreemaan.abhishek@gmail.com>
@shreemaan-abhishek shreemaan-abhishek dismissed stale reviews from membphis and AlinsRan via 3a70010 April 30, 2026 04:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

pFad - Phonifier reborn

Pfad - The Proxy pFad © 2024 Your Company Name. All rights reserved.





Check this box to remove all script contents from the fetched content.



Check this box to remove all images from the fetched content.


Check this box to remove all CSS styles from the fetched content.


Check this box to keep images inefficiently compressed and original size.

Note: This service is not intended for secure transactions such as banking, social media, email, or purchasing. Use at your own risk. We assume no liability whatsoever for broken pages.


Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy