Skip to content
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

Add RequestContextStorage.hook() #2723

Merged
merged 2 commits into from
May 19, 2020
Merged

Conversation

trustin
Copy link
Member

@trustin trustin commented May 18, 2020

Motivation:

There's currently no way to perform an additional operation on top of
the RequestContextStorage implementation discovered via ServiceLoader.

Modifications:

  • Added RequestContextStorage.hook(Function) so that a user is allowed
    to modify the current RequestContextStorage.
  • RequestContextStorage is now Unwrappable.
    • Added RequestContextStorageWrapper

Result:

  • A user can install a custom hook in runtime, usually at startup time,
    like other frameworks.

Motivation:

There's currently no way to perform an additional operation on top of
the `RequestContextStorage` implementation discovered via `ServiceLoader`.

Modifications:

- Added `RequestContextStorage.hook(Function)` so that a user is allowed
  to modify the current `RequestContextStorage`.
- `RequestContextStorage` is now `Unwrappable`.
  - Added `RequestContextStorageWrapper`

Result:

- A user can install a custom hook in runtime, usually at startup time,
  like other frameworks.
@trustin trustin added this to the 0.99.6 milestone May 18, 2020
@trustin trustin requested a review from anuraaga May 18, 2020 06:49
@trustin trustin requested review from ikhoon and minwoox as code owners May 18, 2020 06:49
Copy link
Contributor

@ikhoon ikhoon left a comment

Choose a reason for hiding this comment

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

Thanks! @trustin

Copy link
Member

@minwoox minwoox left a comment

Choose a reason for hiding this comment

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

Thanks!

@codecov
Copy link

codecov bot commented May 18, 2020

Codecov Report

Merging #2723 into master will decrease coverage by 0.03%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2723      +/-   ##
============================================
- Coverage     72.63%   72.59%   -0.04%     
+ Complexity    11621    11617       -4     
============================================
  Files          1022     1023       +1     
  Lines         45434    45446      +12     
  Branches       5662     5663       +1     
============================================
- Hits          32999    32993       -6     
- Misses         9532     9551      +19     
+ Partials       2903     2902       -1     
Impacted Files Coverage Δ Complexity Δ
...linecorp/armeria/common/RequestContextStorage.java 33.33% <0.00%> (-66.67%) 1.00 <0.00> (ø)
...p/armeria/common/RequestContextStorageWrapper.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...rp/armeria/internal/common/RequestContextUtil.java 43.75% <0.00%> (-2.92%) 12.00 <0.00> (ø)
...corp/armeria/client/DefaultEventLoopScheduler.java 79.09% <0.00%> (-10.00%) 29.00% <0.00%> (-5.00%)
...inecorp/armeria/client/AbstractEventLoopState.java 92.30% <0.00%> (-7.70%) 6.00% <0.00%> (-1.00%)
...com/linecorp/armeria/client/OneEventLoopState.java 66.66% <0.00%> (-4.77%) 6.00% <0.00%> (-1.00%)
.../com/linecorp/armeria/server/docs/ServiceInfo.java 78.94% <0.00%> (-1.76%) 20.00% <0.00%> (-1.00%)
...inecorp/armeria/server/HttpResponseSubscriber.java 77.16% <0.00%> (-1.19%) 53.00% <0.00%> (-2.00%)
...inecorp/armeria/server/grpc/ArmeriaServerCall.java 87.59% <0.00%> (-1.17%) 85.00% <0.00%> (ø%)
...p/armeria/internal/common/eureka/InstanceInfo.java 51.75% <0.00%> (-0.88%) 23.00% <0.00%> (-1.00%)
... and 10 more

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 7afa5bd...b96b6ae. Read the comment docs.

@trustin trustin merged commit e568538 into line:master May 19, 2020
@trustin trustin deleted the ctx_storage_hook branch May 19, 2020 06:06
@trustin trustin linked an issue May 19, 2020 that may be closed by this pull request
fmguerreiro pushed a commit to fmguerreiro/armeria that referenced this pull request Sep 19, 2020
Motivation:

There's currently no way to perform an additional operation on top of
the `RequestContextStorage` implementation discovered via `ServiceLoader`.

Modifications:

- Added `RequestContextStorage.hook(Function)` so that a user is allowed
  to modify the current `RequestContextStorage`.
- `RequestContextStorage` is now `Unwrappable`.
  - Added `RequestContextStorageWrapper`

Result:

- A user can install a custom hook in runtime, usually at startup time,
  like other frameworks.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide a way to perform additional work in Logging{Client,Service}
3 participants