fix(opentelemetry): inject additional_attributes in log phase#13265
Conversation
| plugins: | ||
| - opentelemetry | ||
| - key-auth |
There was a problem hiding this comment.
uses key-auth instead of jwt-auth for easier testing setup since the logic is the same (opentelemetry's priority > key-auth's priority)
There was a problem hiding this comment.
Pull request overview
Fixes missing OpenTelemetry span attributes sourced from APISIX variables that are only populated after the rewrite phase (e.g., auth-derived consumer_name, balancer-derived vars), by moving attribute injection later in the request lifecycle.
Changes:
- Move
additional_attributesandadditional_header_prefix_attributesinjection fromrewritetologphase in theopentelemetryplugin. - Add a regression test ensuring
consumer_nameis exported as a span attribute when set by an auth plugin.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
apisix/plugins/opentelemetry.lua |
Shifts additional attribute/header-derived attribute injection to log phase and applies them via span:set_attributes() before finishing the span. |
t/plugin/opentelemetry.t |
Adds tests that create a consumer + key-auth protected route and validates consumer_name appears in exported OTLP span data. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| === TEST 25: check consumer_name in span attributes | ||
| --- exec | ||
| tail -n 1 ci/pod/otelcol-contrib/data-otlp.json |
There was a problem hiding this comment.
Using tail -n 1 plus a broad regex can make the test flaky if another span is exported after the request (or if exports are reordered). To make this deterministic, match the exported record for the triggering request (e.g., grep/filter for the specific X-Request-Id value in the JSON or parse and assert on the span whose request-id attribute matches 01010101010101010101010101010102).
| tail -n 1 ci/pod/otelcol-contrib/data-otlp.json | |
| grep '01010101010101010101010101010102' ci/pod/otelcol-contrib/data-otlp.json |
There was a problem hiding this comment.
followed test convention from previous test cases
| "plugins": { | ||
| "serverless-pre-function": { | ||
| "phase": "rewrite", | ||
| "functions": ["return function(conf, ctx) ngx.req.set_header('x-injected-by-plugin', 'test-value') end"] | ||
| }, | ||
| "opentelemetry": { | ||
| "sampler": { | ||
| "name": "always_on" | ||
| }, | ||
| "additional_header_prefix_attributes": [ | ||
| "x-injected-*" | ||
| ] | ||
| } | ||
| }, |
There was a problem hiding this comment.
The test intent is “header added by lower-priority plugin”, but serverless-pre-function priority may not be obviously lower (and could change), which weakens the regression signal. Consider using a plugin with a well-known, clearly lower priority than opentelemetry that can set request headers (e.g., proxy-rewrite), or otherwise ensure the chosen plugin is definitively executed after opentelemetry in rewrite.
There was a problem hiding this comment.
serverless-pre-function has lower priority then opentelemetry
Description
The
opentelemetryplugin supportsadditional_attributesto inject APISIX variables (e.g.consumer_name,balancer_ip) as span attributes. However, these attributes were silently missing from traces when the variable is set by a lower-priority plugin.Root cause:
additional_attributeswere collected during therewritephase, where theopentelemetryplugin runs first (priority12009). At that point, authentication plugins likejwt-auth(priority2510) have not yet executed, so variables such asconsumer_nameare stillniland silently skipped byinject_attributes. The same applies tobalancer_ip, which is only set after the balancer phase.Fix: Move
additional_attributesandadditional_header_prefix_attributesinjection to thelogphase, where all variables are fully populated. This reuses the existingspan:set_attributes()pattern already present in the logphase for
http.status_codeandapisix.response_source.Which issue(s) this PR fixes:
Fixes #12873
Checklist