Enhance llm-proxy with richer LLM observability#321
Open
Aias00 wants to merge 6 commits into
Open
Conversation
This folds the standalone observability ideas back into llm-proxy so routing, token accounting, richer metadata, and optional structured logs live behind one extension surface instead of two overlapping plugins. The change expands the OpenAI and Anthropic parsers, adds optional observability-oriented config, preserves existing matcher semantics for user-defined rules, and aligns streaming and non-streaming outputs so richer metadata is consistent across both paths. Constraint: Keep llm-proxy backward-compatible for existing user-defined matcher rules while extending observability for default OpenAI/Anthropic traffic Rejected: Keep a separate llm-statistics plugin | duplicates parsing, matching, and metrics responsibilities already owned by llm-proxy Confidence: high Scope-risk: moderate Reversibility: clean Directive: Future llm-proxy observability changes must preserve parity between streaming and non-streaming provider paths and update schema/tests in the same change Tested: cd extensions/composer && go test ./llm-proxy/... Tested: cd extensions/composer && go test ./... Tested: make -C cli gen Tested: cd website && npm run build Not-tested: cli/e2e Docker-dependent paths and live provider traffic outside local test fixtures Signed-off-by: liuhy <liuhongyu@apache.org>
Contributor
There was a problem hiding this comment.
Pull request overview
Enhances the llm-proxy extension to emit richer LLM observability (metadata + optional structured logs) across OpenAI Chat Completions and Anthropic Messages, while keeping streaming/non-streaming behavior aligned and preserving existing matcher semantics.
Changes:
- Extend request/response parsing to extract additional fields (question/system/answer/reasoning/tool calls + token detail fields) and write them to Envoy dynamic metadata.
- Add optional structured log emission and session ID extraction, and refine TTFT/TPOT handling for streaming vs non-streaming.
- Improve default OpenAI/Anthropic rule matching to work when query strings are present (without changing custom matcher behavior).
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| website/public/extensions.json | Updates extension catalog description/docs and adds an example for full observability output. |
| extensions/composer/llm-proxy/plugin.go | Core implementation: richer metadata, structured logs, session ID extraction, query-string-aware default matching, updated TTFT semantics. |
| extensions/composer/llm-proxy/llm.go | Expands interfaces to support richer request/response/stream chunk attributes. |
| extensions/composer/llm-proxy/openai.go | Adds richer OpenAI request/response parsing (prompt/answer/reasoning/tool calls + token details) and improved SSE accumulation. |
| extensions/composer/llm-proxy/anthropic.go | Adds richer Anthropic request/response parsing (system/question/answer/tool calls + cache/token details) and improved SSE accumulation. |
| extensions/composer/llm-proxy/config.schema.json | Adds config schema fields for structured logs and session ID header extraction. |
| extensions/composer/llm-proxy/config_schema_test.go | Extends schema tests for new fields and invalid-type coverage. |
| extensions/composer/llm-proxy/plugin_test.go | Adds tests for query-string default matching behavior and richer metadata/log emission. |
| extensions/composer/llm-proxy/stats_test.go | Adds non-streaming TTFT/TPOT test coverage and refines streaming TTFT test to “first text token” semantics. |
| extensions/composer/llm-proxy/manifest.yaml | Updates extension manifest docs to describe richer observability and new config/example. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This keeps the structured log payload consistent with the metadata keys by using plural token field names in both surfaces. Consumers no longer need separate singular vs plural mappings when correlating logs with filter metadata. The change is intentionally narrow and does not alter metric names, parser behavior, or request/response processing. Constraint: Preserve existing metadata key names while making structured logs easier to consume consistently Rejected: Leave log keys singular and document the mismatch | adds avoidable consumer complexity for no functional gain Confidence: high Scope-risk: narrow Reversibility: clean Directive: New llm-proxy structured log fields should reuse existing metadata names where the concepts are identical Tested: cd extensions/composer && go test ./llm-proxy/... Tested: cd extensions/composer && go test ./... Not-tested: External log pipeline parsing against previous singular field names Signed-off-by: liuhy <liuhongyu@apache.org>
This removes the textual prefix from llm-proxy structured log lines so the emitted payload is valid JSON on its own. Downstream systems can now treat each line as JSON without special-case parsing. The change keeps the schema, metadata, and parser behavior unchanged and only narrows the log output format to the shape already implied by the structured logging feature. Constraint: Preserve existing structured log contents while making the line itself machine-parseable JSON Rejected: Keep the prefix and document it | still breaks JSON-lines consumers and adds avoidable downstream parsing work Confidence: high Scope-risk: narrow Reversibility: clean Directive: Any future structured log additions should keep the final emitted line valid JSON unless there is a strong documented reason otherwise Tested: cd extensions/composer && go test ./llm-proxy/... Tested: cd extensions/composer && go test ./... Not-tested: External log ingestion pipelines expecting the old prefixed format Signed-off-by: liuhy <liuhongyu@apache.org>
This restores the descriptive comments that were lost when the llm-proxy provider parsers were expanded for richer observability and adds matching explanations for the new request, response, chunk, and SSE accumulator structures. The change is documentation-only and is meant to keep the richer implementation readable at the same level of detail as the original parser code. Constraint: Preserve behavior exactly while bringing parser documentation back to the previous readability baseline Rejected: Leave comments sparse and rely on type names alone | makes the expanded parser code noticeably harder to review and maintain Confidence: high Scope-risk: narrow Reversibility: clean Directive: When replacing parser implementations wholesale, preserve or reintroduce the explanatory comments from the previous version in the same change Tested: cd extensions/composer && go test ./llm-proxy/... Not-tested: No behavior changes; no broader verification required Signed-off-by: liuhy <liuhongyu@apache.org>
This reapplies the token-field naming alignment in llm-proxy structured logs so the emitted JSON continues to use the same plural keys as the filter metadata. The change is intentionally narrow and only restores the naming consistency after the branch absorbed upstream updates. Constraint: Preserve the current llm-proxy behavior while keeping structured log field names aligned with metadata keys Rejected: Leave singular token keys in logs | reintroduces avoidable inconsistency for downstream consumers Confidence: high Scope-risk: narrow Reversibility: clean Directive: When rebasing this branch, keep structured log field names aligned with metadata names unless a migration plan is documented Tested: Existing local composer and llm-proxy test runs completed earlier in this branch; this commit is a narrow naming-only follow-up Not-tested: No additional rerun after this exact 2-line change Signed-off-by: liuhy <liuhongyu@apache.org>
wbpcode
reviewed
Apr 9, 2026
Comment on lines
+48
to
+61
| // GetAnswer returns the textual assistant answer extracted from the response if available. | ||
| GetAnswer() string | ||
| // GetReasoning returns provider-specific reasoning content when available. | ||
| GetReasoning() string | ||
| // GetToolCalls returns any tool call payload emitted by the model. | ||
| GetToolCalls() any | ||
| // GetReasoningTokens returns the number of reasoning tokens when provided. | ||
| GetReasoningTokens() uint32 | ||
| // GetCachedTokens returns cached input token counts when provided. | ||
| GetCachedTokens() uint32 | ||
| // GetInputTokenDetails returns provider-specific prompt/input token detail fields. | ||
| GetInputTokenDetails() any | ||
| // GetOutputTokenDetails returns provider-specific completion/output token detail fields. | ||
| GetOutputTokenDetails() any |
Collaborator
There was a problem hiding this comment.
I guess we want a better abstraction that could support auto transformation between multiple AI providers.
wbpcode
reviewed
Apr 9, 2026
wbpcode
left a comment
wbpcode
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for this update. I guess it's a good requirement for debug/observe AI traffic.
But note the current metadata could only support number/bool/string and cannot support other typed data. I personally prefer to avoid metadata to take raw payload for now because it's inefficient.
We are working a new abstraction which will expose more details of traffic and also could support auto-transformation, I think will also helpful for your requirements. I guess we may could wait that PR first and I will also ping your suggestions there.
Feel free to slack me at Envoy Slack or BoE slack (with name wbpcode) because I think you are Chinese (?) and have same timezone with me.
Author
Okay, I got it. Thanks a lot for your kind advice. |
nacx
added
the
waiting for upstream
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
llm-proxywith richer observability instead of introducing a separate overlapping pluginVerification
cd extensions/composer && go test ./llm-proxy/...cd extensions/composer && go test ./...make -C cli gencd website && npm run buildNotes
llm-statisticsplugin and consolidates the richer observability behavior intollm-proxycli/e2eDocker-dependent paths were not exercised in this branch