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

docs: add reactor context propagation example #10220

Merged
merged 9 commits into from
Dec 11, 2023

Conversation

wetted
Copy link
Contributor

@wetted wetted commented Dec 5, 2023

closes #10219

I believe we agreed in the meeting that a kotlin example is not necessary (nor appropriate).

@wetted wetted requested review from dstepanov and sdelamo December 5, 2023 19:45
@wetted wetted self-assigned this Dec 5, 2023
@dstepanov
Copy link
Contributor

I would make a note before the context is got that it was modified in some way. Otherwise, it doesn't make sense to do.

@wetted
Copy link
Contributor Author

wetted commented Dec 5, 2023

I would make a note before the context is got that it was modified in some way. Otherwise, it doesn't make sense to do.

Sorry, I'm not sure I understand what you mean "that it was modified". (this is all new to me). Can you clarify for me further, please.

@dstepanov
Copy link
Contributor

@Get('/hello')
        Mono<String> hello() {
            // The propagated context is modified ( PropagatedContext.get().add / remove etc)
            PropagatedContext propagatedContext = PropagatedContext.get() // <1>
            return Mono.just('Hello, World')
                .contextWrite(ctx -> ReactorPropagation.addPropagatedContext(ctx, propagatedContext)) // <2>
        }

@wetted
Copy link
Contributor Author

wetted commented Dec 5, 2023

// The propagated context is modified ( PropagatedContext.get().add / remove etc)

I changed the adoc comment. See if that's OK.

@dstepanov
Copy link
Contributor

Looks fine; the other option would be to add some propagation context element to make it make more clear. Please take a look @sdelamo if it's understandable.

@wetted
Copy link
Contributor Author

wetted commented Dec 5, 2023

@dstepanov I updated the example to add an element. See if that's OK. Thanks for the feedback.

@wetted wetted requested a review from dstepanov December 5, 2023 21:18
Copy link

sonarqubecloud bot commented Dec 5, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Contributor

@sdelamo sdelamo left a comment

Choose a reason for hiding this comment

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

@dstepanov should we do this for Kotlin as well?

@dstepanov
Copy link
Contributor

For Kotlin we have KotlinCoroutinePropagation

@sdelamo
Copy link
Contributor

sdelamo commented Dec 8, 2023

@dstepanov do you think we should merge it in the current status?

@dstepanov
Copy link
Contributor

Looks good

@sdelamo sdelamo merged commit edda671 into 4.2.x Dec 11, 2023
16 checks passed
@sdelamo sdelamo deleted the engance-context-propagation-docs branch December 11, 2023 08:49
sdelamo added a commit that referenced this pull request Jan 5, 2024
* fix(deps): update dependency ch.qos.logback:logback-classic to v1.4.14 (#10191)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update test (#10187)

* KSP: Resolved type arguments shouldn't be marked as a type variable (#10193)

* Optimize overrides-method check (#10195)

* Fix error with duplicate builder methods (#10226)

* fix(deps): update dependency io.netty.incubator:netty-incubator-codec-http3 to v0.0.23.final (#10222)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update ROADMAP.adoc (#10213)

* Update ROADMAP.adoc

Close #10205

* Update ROADMAP.adoc

* fix(deps): update dependency com.github.javaparser:javaparser-symbol-solver-core to v3.25.7 (#10207)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* imp: don't send Content-length header for GET requests (#10218)

* Allow context-path to be an empty String (#10231)

* Allow context-path to be an empty String

* Update ConfigurableUriNamingStrategySpec

* Fix concurrent retrieval of expression resolvers (#10176) (#10229)

* Change to public so that java doc is generated for Configuration Reference link in manual. (#10233)

closes #10202

* KSP: Fix enum class values (#10240)

* docs: add reactor context propagation example (#10220)

* docs: add reactor context propagation example

closes #10219

* fix test failures

* add clarification about modified propagation context

* improve example

* fix code smell

* refactor to fix code smell

* change example to use custom context element

* refine example

* include example imports

* KSP: Exclude static properties (#10245)

* test: remove javax.persistence.Transient from Mapper (#10113)

@transient exclusion was removed in @transient since https://github.com/micronaut-projects/micronaut-core/pull/8072/files

* Update breaking changes for Micronaut 4  (#10248)

* Update breaks.adoc

update to include breaking library changes

* Update breaks.adoc

* fix(deps): update dependency io.micronaut.aws:micronaut-aws-bom to v4.1.2 (#10249)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* KSP: Workaround for inner anonymous classes (#10247)

* fix(deps): update dependency io.micronaut.rxjava2:micronaut-rxjava2-bom to v2.2.0 (#10241)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* fix(deps): update dependency io.micronaut.reactor:micronaut-reactor-bom to v3.2.0 (#10234)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Add links to guides for Micronaut Core (#10269)

* Allow Refreshable scope under test for Env.FUNCTION and ANDROID (#10264)

It was seen in the Aws module that `@MockLambdaTest` and `@MockBean` did not work as expected.

micronaut-projects/micronaut-aws#1870

This is because:

1. `@MockBean` uses the Refreshable scope
2. `@MockLambdaTest` adds Environment.FUNCTION to the test context
3. The Refresh scope is disabled for the FUNCTION environment

After this PR, RefreshScope is still disabled for Environment.FUNCTION...

...*UNLESS* Environment.TEST is also enabled.

* test. ThreadSelection accept event loop in the handler in blocking and auto scenarios (#10104)

* change comment copy

* test. ThreadSelection accept event loop in the handler in blocking and auto scenarios

The test is really flaky:

https://ge.micronaut.io/scans/tests?search.timeZoneId=Europe%2FMadrid&tests.container=io.micronaut.http.server.netty.threading.ThreadSelectionSpec

* disable test

---------

Co-authored-by: yawkat <jonas.konrad@oracle.com>

* [bug] EL fails when using primitive method arguments in expression (#10149)

* Reproducer

* Add Kotlin reproducer as well

* Unbox primitive

---------

Co-authored-by: Denis Stepanov <denis.s.stepanov@oracle.com>

* fix(deps): update netty monorepo to v4.1.103.final (#10270)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* fix(deps): update managed.ksp to v1.9.21-1.0.16 (#10268)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* [skip ci] Release v4.2.2

* Back to 4.2.3-SNAPSHOT

* Fix logged errors reported by fuzzing (#10273)

* Fix logged errors reported by fuzzing
There are two bugs fixed here:

(1) Empty URI handling in HateoasErrorResponseProcessor

```
10:18:18.775 [main] ERROR i.m.h.s.netty.RoutingInBoundHandler - Micronaut Server Error - No request state present. Cause: URI cannot be empty
java.lang.IllegalArgumentException: URI cannot be empty
	at io.micronaut.http.hateoas.DefaultLink.<init>(DefaultLink.java:49)
	at io.micronaut.http.hateoas.Link.of(Link.java:115)
	at io.micronaut.http.server.exceptions.response.HateoasErrorResponseProcessor.processResponse(HateoasErrorResponseProcessor.java:69)
	at io.micronaut.http.server.RouteExecutor.createDefaultErrorResponse(RouteExecutor.java:214)
	at io.micronaut.http.server.netty.RoutingInBoundHandler.writeResponse(RoutingInBoundHandler.java:229)
	at io.micronaut.http.server.netty.NettyRequestLifecycle.lambda$handleException$2(NettyRequestLifecycle.java:147)
	at io.micronaut.core.execution.ImperativeExecutionFlowImpl.onComplete(ImperativeExecutionFlowImpl.java:132)
	at io.micronaut.http.server.netty.NettyRequestLifecycle.handleException(NettyRequestLifecycle.java:147)
	at io.micronaut.http.server.netty.NettyRequestLifecycle.handleNormal(NettyRequestLifecycle.java:89)
	at io.micronaut.http.server.netty.RoutingInBoundHandler.accept(RoutingInBoundHandler.java:220)
[...]
```

Test input added to FuzzyInputSpec. FuzzyInputSpec has been adjusted to recognize logged errors.

(2) Bad sorting in MediaType.orderedOf

```
11:48:58.716 [41560@yawkat-oracle main] ERROR i.m.http.server.RouteExecutor - Unexpected error occurred: Comparison method violates its general contract!
java.lang.IllegalArgumentException: Comparison method violates its general contract!
	at java.base/java.util.TimSort.mergeLo(TimSort.java:781)
	at java.base/java.util.TimSort.mergeAt(TimSort.java:518)
	at java.base/java.util.TimSort.mergeForceCollapse(TimSort.java:461)
	at java.base/java.util.TimSort.sort(TimSort.java:254)
	at java.base/java.util.Arrays.sort(Arrays.java:1307)
	at java.base/java.util.ArrayList.sort(ArrayList.java:1721)
	at io.micronaut.http.MediaType.orderedOf(MediaType.java:870)
	at io.micronaut.http.netty.NettyHttpHeaders.accept(NettyHttpHeaders.java:301)
```

Created a new MediaTypeFuzzTest that hits this issue. The sample input (crash-22df7f6e72bba86bdb1fdbd4bf92372fd4fa6bbe) reproduces the issue and will be run as part of the normal micronaut-http test suite now. Setting the env variable JAZZER_FUZZ=1 will enable exploratory fuzzing.

(3) Added workaround for netty/netty#13730

This does not appear to be an issue in the real world, only with EmbeddedChannel. I've added a workaround for the issue so that it doesn't trigger my fuzz tests anymore.

---

None of these bugs appear security-relevant, (3) is not applicable outside a test env, (1) and (2) only produce additional error logs.

* annotation

* KSP: Fix `@Generated` processing (#10282)

* Initialize classes at build time that can deadlock graal compiler (#10283)

A class loading deadlock can occur when ConversionContext is initialised concurrently since it has static final fields that are sub interfaces which are also initialised causing a loop.

This doesn't manifest on JIT likely because these classes are never initialised concurrently, but a deadlock can occur in the Graal compiler.

A real fix would to be alter the code but unfortunately there is no way to do this in a backwards compatible way.

A workaround in Graal is to explicitly initialise these classes at build time which happens very early, avoiding the concurrent access that causes the deadlock.

* KSP: Do not store default empty string annotation value (#10284)

* fix(deps): update netty monorepo to v4.1.104.final (#10276)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* chore(deps): update gradle/gradle-build-action action to v2.11.0 (#10252)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Ensure responseWritten is called on discard (#10288)

FullOutboundHandler needs to call responseWritten on discard, otherwise RoutingInboundHandler won't clean up request body. This can happen if the channel is closed while it's not writable, so there's a response backlog.

* Javac: Extract type variable name using the element (#10293)

* Publish service ready / stopped in consistent order (#10325)

* Publish service ready / stopped in consistent order

* Update test-suite/src/test/groovy/io/micronaut/EventListenerSpec.groovy

Co-authored-by: Sergio del Amo <sergio.delamo@softamo.com>

* Update test-suite/src/test/groovy/io/micronaut/EventListenerSpec.groovy

Co-authored-by: Sergio del Amo <sergio.delamo@softamo.com>

---------

Co-authored-by: Sergio del Amo <sergio.delamo@softamo.com>

* [skip ci] Release v4.2.3

* Back to 4.2.4-SNAPSHOT

* test: StreamPressureSpec ignore on macOs

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Denis Stepanov <denis.s.stepanov@oracle.com>
Co-authored-by: Tim Yates <tim.yates@gmail.com>
Co-authored-by: Jeremy Grelle <grellej@unityfoundation.io>
Co-authored-by: rorueda <rorueda@gmail.com>
Co-authored-by: Dean Wette <wetted@objectcomputing.com>
Co-authored-by: hrothwell <46227616+hrothwell@users.noreply.github.com>
Co-authored-by: yawkat <jonas.konrad@oracle.com>
Co-authored-by: micronaut-build <micronaut-build-account@grails.org>
Co-authored-by: Graeme Rocher <graeme.rocher@oracle.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

docs: enhance reactor context propagation documentation
3 participants