New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Propagate RequestAttributes across Hystrix Thread boundary #2509

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
8 participants
@eacdy
Contributor

eacdy commented Dec 4, 2017

Sometimes we need to get request attribute in feign's interceptor or other where else. So it's necessary to copy the attributes managed by the Spring RequestContextHolder from the main thread to the threads managed by Hystrix thread pool.
Inspired by #1379

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Dec 4, 2017

Codecov Report

Merging #2509 into master will decrease coverage by 0.41%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2509      +/-   ##
==========================================
- Coverage   65.65%   65.23%   -0.42%     
==========================================
  Files         207      210       +3     
  Lines        7646     7695      +49     
  Branches      943      944       +1     
==========================================
  Hits         5020     5020              
- Misses       2268     2317      +49     
  Partials      358      358
Impacted Files Coverage Δ
...uest/HystrixRequestAttributeAutoConfiguration.java 0% <0%> (ø)
...st/RequestAttributeHystrixConcurrencyStrategy.java 0% <0%> (ø)
...rix/request/HystrixRequestAttributeProperties.java 0% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 05cd507...2f2b203. Read the comment docs.

codecov-io commented Dec 4, 2017

Codecov Report

Merging #2509 into master will decrease coverage by 0.41%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2509      +/-   ##
==========================================
- Coverage   65.65%   65.23%   -0.42%     
==========================================
  Files         207      210       +3     
  Lines        7646     7695      +49     
  Branches      943      944       +1     
==========================================
  Hits         5020     5020              
- Misses       2268     2317      +49     
  Partials      358      358
Impacted Files Coverage Δ
...uest/HystrixRequestAttributeAutoConfiguration.java 0% <0%> (ø)
...st/RequestAttributeHystrixConcurrencyStrategy.java 0% <0%> (ø)
...rix/request/HystrixRequestAttributeProperties.java 0% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 05cd507...2f2b203. Read the comment docs.

@ryanjbaxter

This comment has been minimized.

Show comment
Hide comment
@ryanjbaxter

ryanjbaxter Dec 4, 2017

Contributor

You will need to provide some tests

Contributor

ryanjbaxter commented Dec 4, 2017

You will need to provide some tests

@xiaods

This comment has been minimized.

Show comment
Hide comment
@xiaods

xiaods Dec 5, 2017

need testing

xiaods commented Dec 5, 2017

need testing

@newtalentxp

This comment has been minimized.

Show comment
Hide comment
@newtalentxp

newtalentxp Mar 17, 2018

need to simplify it

newtalentxp commented Mar 17, 2018

need to simplify it

@manavdewan1990

This comment has been minimized.

Show comment
Hide comment
@manavdewan1990

manavdewan1990 commented Apr 30, 2018

+1

@spencergibb

This comment has been minimized.

Show comment
Hide comment
@spencergibb

spencergibb Sep 5, 2018

Member

@eacdy can you rebase and add a test?

Member

spencergibb commented Sep 5, 2018

@eacdy can you rebase and add a test?

@spencergibb spencergibb added icebox and removed backlog labels Sep 5, 2018

@eacdy

This comment has been minimized.

Show comment
Hide comment
@eacdy

eacdy Sep 6, 2018

Contributor

@spencergibb Sorry, I don't have enough time. Could you help me rebase and add a test?

Contributor

eacdy commented Sep 6, 2018

@spencergibb Sorry, I don't have enough time. Could you help me rebase and add a test?

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