Add the ability to configure logback (MDC) to show `identity_id` and `X-Request-ID` in logs files when available. #7461

sunix · 2017-11-20T14:14:19Z

What does this PR do?

Add the ability to configure logback to show identity_id and X-Request-ID in logs files when available.

Add X-Request-ID http header in the logs through MDC
Add identity_id from keycloack when available in the logs through MDC

What issues does this PR fix or reference?

redhat-developer/rh-che#312

Release Notes

Add the ability to configure logback to show identity_id and X-Request-ID in logs files when available.

Docs PR

TODO

skabashnyuk · 2017-11-20T14:21:25Z

identity_id and X-Request-ID - what is the usecase for this value?
Shall we move this filter in separate artifact and reuse it on the agent as well?

sunix · 2017-11-22T08:08:06Z

Identity_id : to track the user related logs in a multi-user configuration
request_id : to track a request that goes through multiple sub system

I'm going to move the filter for request id somewhere else

skabashnyuk · 2017-11-22T08:46:33Z

Identity_id - ok. What does this value X-Request-Id mean for Che?

sunix · 2017-11-22T09:44:29Z

For Che it means that wherever the cluster environnement it is installed and where routers are using X-Request-Id, Che would be able to log these request-Ids out of the box

skabashnyuk · 2017-11-22T10:02:02Z

What about the separate artifact and reuse it on the agent?
I thought about adding web application name to MDC too. It would be good place to do that. WDYT?

sunix · 2017-11-22T10:37:41Z

OK I'm OK to create a separate artifact, I was looking at an existing one and https://github.com/eclipse/che/tree/master/core/commons/che-core-commons-j2ee/src/main/java/org/eclipse/che/filter was looking good by its name but it seems to be included only in assembly-ide-war. Let me know if you have a prefer place and name for this artifact.

sunix · 2017-11-22T10:43:29Z

About the web application name, I'm not sure MDC is the right place: MDC works per thread and pretty much like a threadLocal would work.

skabashnyuk · 2017-11-22T11:15:50Z

what if core/commons/che-core-commons-logback ?

sunix · 2017-11-22T17:08:54Z

I've updated the PR but for some reasons i can't get into the multi-user app, will investigate tomorow

skabashnyuk · 2017-11-22T19:51:19Z

core/commons/che-core-commons-logback/pom.xml

+        <dependency>
+            <groupId>org.slf4j</groupId>
+            <artifactId>slf4j-api</artifactId>
+            <scope>provided</scope>


this artifact is not provided. please remove this scope.

skabashnyuk · 2017-11-22T19:51:27Z

...mons-logback/src/main/java/org/eclipse/che/commons/logback/filter/RequestIdLoggerFilter.java

+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletRequest;
+import org.slf4j.MDC;
+


What about test?

skabashnyuk · 2017-11-22T19:52:57Z

...n/java/org/eclipse/che/multiuser/keycloak/server/KeycloakEnvironmentInitalizationFilter.java

@@ -103,6 +106,8 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
      }
    }

+    putIdentityIdToMDC(subject);


I think you can replace it with just MDC.put(IDENTITY_ID_MDC_KEY, subject.getUserId());

wait. It should not be here.
I propose to make a separate filter in core/commons/che-core-commons-logback/ that will put subject id in mdc.

Signed-off-by: Sun Tan <sutan@redhat.com>

sunix · 2017-12-04T11:58:14Z

@skabashnyuk I've updated this PR can you check and approve if ok ? thanks

sunix · 2017-12-04T13:56:38Z

thanks

skabashnyuk · 2017-12-04T14:23:10Z

@sunix will you provide che6 pr?

sunix added the kind/enhancement A feature request - must adhere to the feature request template. label Nov 20, 2017

sunix requested review from riuvshin, skabashnyuk and vparfonov as code owners November 20, 2017 14:14

benoitf added the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Nov 20, 2017

sunix force-pushed the identity-request-id-mdc branch from 8021e02 to 31d2e4d Compare November 22, 2017 17:07

skabashnyuk reviewed Nov 22, 2017

View reviewed changes

sunix force-pushed the identity-request-id-mdc branch from 31d2e4d to 3a3dac2 Compare December 4, 2017 11:29

sunix added 2 commits December 4, 2017 11:51

Adding request_id header to the logs through MDC Context

04bed28

Signed-off-by: Sun Tan <sutan@redhat.com>

Adding identity_id from keycloak to the logs through MDC Context

fe36833

Signed-off-by: Sun Tan <sutan@redhat.com>

sunix force-pushed the identity-request-id-mdc branch from 3a3dac2 to fe36833 Compare December 4, 2017 11:56

skabashnyuk approved these changes Dec 4, 2017

View reviewed changes

sunix merged commit 26725e5 into eclipse-che:master Dec 4, 2017

benoitf removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Dec 4, 2017

benoitf added this to the 5.22.0 milestone Dec 4, 2017

sunix mentioned this pull request Jan 5, 2018

[Cherry-pick che6] Add the ability to configure logback (MDC) to show identity_id and X-Request-ID in logs files when available. #8191

Merged

gorkem mentioned this pull request Jan 11, 2018

Workspace.Next #8265

Closed

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add the ability to configure logback (MDC) to show `identity_id` and `X-Request-ID` in logs files when available. #7461

Add the ability to configure logback (MDC) to show `identity_id` and `X-Request-ID` in logs files when available. #7461

sunix commented Nov 20, 2017

skabashnyuk commented Nov 20, 2017

sunix commented Nov 22, 2017 •

edited

Loading

skabashnyuk commented Nov 22, 2017 •

edited

Loading

sunix commented Nov 22, 2017

skabashnyuk commented Nov 22, 2017

sunix commented Nov 22, 2017

sunix commented Nov 22, 2017

skabashnyuk commented Nov 22, 2017

sunix commented Nov 22, 2017

skabashnyuk Nov 22, 2017

skabashnyuk Nov 22, 2017

skabashnyuk Nov 22, 2017

skabashnyuk Nov 22, 2017

skabashnyuk Nov 22, 2017

sunix commented Dec 4, 2017

sunix commented Dec 4, 2017

skabashnyuk commented Dec 4, 2017

Add the ability to configure logback (MDC) to show identity_id and X-Request-ID in logs files when available. #7461

Add the ability to configure logback (MDC) to show identity_id and X-Request-ID in logs files when available. #7461

Conversation

sunix commented Nov 20, 2017

What does this PR do?

What issues does this PR fix or reference?

Release Notes

Docs PR

skabashnyuk commented Nov 20, 2017

sunix commented Nov 22, 2017 • edited Loading

skabashnyuk commented Nov 22, 2017 • edited Loading

sunix commented Nov 22, 2017

skabashnyuk commented Nov 22, 2017

sunix commented Nov 22, 2017

sunix commented Nov 22, 2017

skabashnyuk commented Nov 22, 2017

sunix commented Nov 22, 2017

skabashnyuk Nov 22, 2017

Choose a reason for hiding this comment

skabashnyuk Nov 22, 2017

Choose a reason for hiding this comment

skabashnyuk Nov 22, 2017

Choose a reason for hiding this comment

skabashnyuk Nov 22, 2017

Choose a reason for hiding this comment

skabashnyuk Nov 22, 2017

Choose a reason for hiding this comment

sunix commented Dec 4, 2017

sunix commented Dec 4, 2017

skabashnyuk commented Dec 4, 2017

Add the ability to configure logback (MDC) to show `identity_id` and `X-Request-ID` in logs files when available. #7461

Add the ability to configure logback (MDC) to show `identity_id` and `X-Request-ID` in logs files when available. #7461

sunix commented Nov 22, 2017 •

edited

Loading

skabashnyuk commented Nov 22, 2017 •

edited

Loading