[opa] Correlate OPA server log with Polaris requestId; add observability for non-200 errors from OPA server#5011
Merged
Merged
Conversation
dimas-b
reviewed
Jul 8, 2026
dimas-b
left a comment
dimas-b
left a comment
Contributor
There was a problem hiding this comment.
Thanks for more contributions, @gracechen09 !
|
|
||
| private ImmutableContext buildContext() { | ||
| return ImmutableContext.builder().requestId(UUID.randomUUID().toString()).build(); | ||
| String requestId = MDC.get("requestId"); |
Contributor
There was a problem hiding this comment.
I do not think using MDC for carry data between Polaris components is a good idea.
Would it be possible to inject RequestIdSupplier instead?
Contributor
Author
There was a problem hiding this comment.
yeah using RequestIdSupplier makes sense!
dimas-b
reviewed
Jul 9, 2026
| OpaAuthorizationConfig opaConfig, | ||
| Clock clock, | ||
| AsyncExec asyncExec, | ||
| Instance<RequestIdSupplier> requestIdSupplier) { |
Contributor
There was a problem hiding this comment.
Why bother with Instance? ServiceProducers make this bean available in the request scope unconditionally.
Contributor
Author
There was a problem hiding this comment.
good point, just realized that OpaPolarisAuthorizer is built per request! I simplified the code to inject RequestIdSupplier directly in the factory
dimas-b
previously approved these changes
Jul 14, 2026
github-project-automation
Bot
moved this from PRs In Progress
to Ready to merge
in Basic Kanban Board
Contributor
|
@gracechen09 : Please resolve merge conflicts. |
dimas-b
approved these changes
Jul 15, 2026
github-project-automation
Bot
moved this from Ready to merge
to Done
in Basic Kanban Board
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
2 participants
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.
This PR contains two changes:
Today, requestId is randomly generated for OPA context, this PR changes it to the actual requestId so that OPA operators can correlate Polaris request with OPA server's decision logs
When OPA returns non-200 errors, Polaris server only logs the exception but no information about the status code. For example, one can't find out whether it's an authorization failure or server unreachable failure from server logs. Therefore, this PR logs the status code for non-200 errors to make it easy to construct alerts based on different error code
Checklist
CHANGELOG.md(if needed)site/content/in-dev/unreleased(if needed)