Skip to content

Adding fix for #SLING-12406#16

Open
pat-lego wants to merge 1 commit intoapache:masterfrom
pat-lego:main2
Open

Adding fix for #SLING-12406#16
pat-lego wants to merge 1 commit intoapache:masterfrom
pat-lego:main2

Conversation

@pat-lego
Copy link
Contributor

@pat-lego pat-lego commented Aug 9, 2024

No description provided.

@Override
public boolean handleSecurity(HttpServletRequest request,
HttpServletResponse response) {
if(request.getHeader("x-request-id") != null) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion the value of "x-request-id" should be configurable. If blank, don't set the thread name.

Also note that on Jetty the thread name includes the request method and path, with this change this information is lost, unless it is in the special header.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The request path and method would still persist as we are not overriding the function call made here https://github.com/apache/sling-org-apache-sling-engine/blob/master/src/main/java/org/apache/sling/engine/impl/SlingMainServlet.java#L107 but instead we are simply adding the ability to trace the authentication logs to the edge logs allowing for visibility into the authentication process.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 4, 2024

@jsedding
Copy link

I agree with the premise of logging a request ID. However, I would prefer an approach based on logback's MDC, which is more versatile and keeps this as an orthogonal concern. I think @nscendoni is working towards such an approach.

@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants