REVIEW: Add servlet filter to add userId to MDC #816

Merged
merged 2 commits into from Apr 16, 2013

Conversation

Projects
None yet
3 participants
Contributor

adreghiciu commented Apr 15, 2013

+1

Contributor

cstamas commented Apr 15, 2013

+1

Btw, what about threads running as scheduled tasks (actually all non-jetty HTTP related threads)?

Contributor

jdillon commented Apr 15, 2013

@cstamas scheduled tasks specifically change user ctx to the system "task user". For other cases I can not say atm if there is user ctx or not. I can guess however that given the large number of threads we have w/o a system-wide thread-mgt strategy that changing this for all threads would be non-trivial.

I mentioned this a few times, and you are the first to say anything. I'm not sure its worth the trouble to add this, but if you know of any specific places where a non-jetty thread actually carries user context please let me know so I can look at it closer.

Contributor

cstamas commented Apr 15, 2013

For scheduled tasks with the ctx you mention:

// Subject subject = this.securitySystem.runAs( new SimplePrincipalCollection("admin", "") );

For other pools in general (like Auto-Routing, Proxy Remote Status Checker, etc), they all use same thread factory: org.sonatype.nexus.threads.NexusThreadFactory. So, if we want, we could set those here.

Contributor

jdillon commented Apr 15, 2013

public Thread newThread( Runnable r )

would have to wrap the runnable to set/unset the mdc key when the thread is started. but I can't say if the context will be correct if the threads are used in a pool and reused by whatever system created them. so it is still not simple IMO to simply do this generically. If there was however an abstraction to "do some work" and not "create a thread" then it could have been generic to set/unset MDC keys around the execution of "do some work".

Merge branch 'master' into NEXUS-5662-userid-mdc
Conflicts:
	plugins/siesta/nexus-siesta-plugin/src/main/java/org/sonatype/nexus/plugins/siesta/SiestaModule.java

jdillon added a commit that referenced this pull request Apr 16, 2013

Merge pull request #816 from sonatype/NEXUS-5662-userid-mdc
REVIEW: Add servlet filter to add userId to MDC

@jdillon jdillon merged commit adbad87 into master Apr 16, 2013

@jdillon jdillon deleted the NEXUS-5662-userid-mdc branch Apr 16, 2013

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