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

Provide a reactive actuator endpoint for (username indexed) session repositories #32046

Closed
wants to merge 2 commits into from

Conversation

vpavic
Copy link
Contributor

@vpavic vpavic commented Aug 9, 2022

At present, Actuator sessions endpoint is supported only on a Servlet stack and also requires an indexed session repository. With Spring Session moving to non-indexed session repositories as a default for some session stores, this means that sessions endpoint won't be available unless users opt into a (non-default) indexed session repository.

This commit updates SessionEndpoint so that it is able to work with a non-indexed session repository. In such setup, it exposes operations for fetching session by id and deleting the session.

Additionally, this also adds support for reactive stack by introducing ReactiveSessionEndpoint and its auto-configuration support.

Closes gh-10827

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 9, 2022
wilkinsona and others added 2 commits August 12, 2022 18:20
At present, Actuator sessions endpoint is supported only on a Servlet stack and also requires an indexed session repository. With Spring Session moving to non-indexed session repositories as a default for some session stores, this means that sessions endpoint won't be available unless users opt into a (non-default) indexed session repository.

This commit updates SessionEndpoint so that it is able to work with a non-indexed session repository. In such setup, it exposes operations for fetching session by id and deleting the session.

Additionally, this also adds support for reactive stack by introducing ReactiveSessionEndpoint and its auto-configuration support.

Closes spring-projectsgh-10827
@vpavic vpavic marked this pull request as ready for review August 12, 2022 16:47
@vpavic
Copy link
Contributor Author

vpavic commented Aug 12, 2022

This is now ready for review.

Note @wilkinsona that :spring-boot-project:spring-boot-docs:asciidoctor* tasks are failing due a problem that seems related with the two different @Endpoints having the same id (as discussed in #10827 (comment)). You should see this failure in the CI as well:

> Task :spring-boot-project:spring-boot-docs:asciidoctor
2022-08-12T18:45:28.138+02:00 [main] WARN FilenoUtil : Native subprocess control requires open access to the JDK IO subsystem
Pass '--add-opens java.base/sun.nio.ch=ALL-UNNAMED --add-opens java.base/java.io=ALL-UNNAMED' to enable.
Exception in thread "main" java.lang.IllegalStateException: Duplicate key management.endpoint.sessions.enabled (attempted merging values io.spring.asciidoctor.springboot.ConfigurationProperty@272f5373 and io.spring.asciidoctor.springboot.ConfigurationProperty@799557c8)
        at java.base/java.util.stream.Collectors.duplicateKeyException(Collectors.java:135)
        at java.base/java.util.stream.Collectors.lambda$uniqKeysMapAccumulator$1(Collectors.java:182)
        at java.base/java.util.stream.ReduceOps$3ReducingSink.accept(ReduceOps.java:169)
        at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1625)
        at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
        at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
        at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921)
        at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
        at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682)
        at io.spring.asciidoctor.springboot.ConfigurationProperties.<init>(ConfigurationProperties.java:49)
        at io.spring.asciidoctor.springboot.ConfigurationProperties.fromClasspath(ConfigurationProperties.java:98)
        at io.spring.asciidoctor.springboot.ConfigurationPropertyValidator.<init>(ConfigurationPropertyValidator.java:39)
        at io.spring.asciidoctor.springboot.ConfigurationPropertyInlineMacroProcessor.<init>(ConfigurationPropertyInlineMacroProcessor.java:37)
        at io.spring.asciidoctor.springboot.SpringBootExtensionRegistry.register(SpringBootExtensionRegistry.java:37)
        at org.asciidoctor.jruby.extension.internal.ExtensionRegistryExecutor.registerAllExtensions(ExtensionRegistryExecutor.java:21)
        at org.asciidoctor.jruby.internal.JRubyAsciidoctor.registerExtensions(JRubyAsciidoctor.java:104)
        at org.asciidoctor.jruby.internal.JRubyAsciidoctor.processRegistrations(JRubyAsciidoctor.java:89)
        at org.asciidoctor.jruby.internal.JRubyAsciidoctor.create(JRubyAsciidoctor.java:69)
        at org.asciidoctor.jruby.internal.JRubyAsciidoctor.create(JRubyAsciidoctor.java:65)
        at org.asciidoctor.jruby.AsciidoctorJRuby$Factory.create(AsciidoctorJRuby.java:29)
        at org.asciidoctor.gradle.remote.AsciidoctorJavaExec.getAsciidoctorInstance(AsciidoctorJavaExec.groovy:101)
        at org.asciidoctor.gradle.remote.AsciidoctorJavaExec.run(AsciidoctorJavaExec.groovy:59)
        at org.asciidoctor.gradle.remote.AsciidoctorJavaExec.main(AsciidoctorJavaExec.groovy:49)

* @author Vedran Pavic
* @since 3.0.0
*/
public final class SessionDescriptor {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what's the policy in the cases, but I wanted to keep this package private - JSON serialization required getters to be public but Checkstyle complained if those were public and SessionDescriptor class wasn't.

Comment on lines +42 to +44
private final SessionRepository<? extends Session> sessionRepository;

private final FindByIndexNameSessionRepository<? extends Session> indexedSessionRepository;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that I opted to inject SessionRepository and FindByIndexNameSessionRepository separately, even though they inject the same session repository. This seemed cleaner as there no need to do instanceof anywhere and also note that we're considering make FindByIndexNameSessionRepository a standalone interface (see spring-projects/spring-session#2115).

@wilkinsona wilkinsona added this to the 3.0.x milestone Sep 1, 2022
@wilkinsona wilkinsona added type: enhancement A general enhancement for: merge-with-amendments Needs some changes when we merge and removed status: waiting-for-triage An issue we've not yet triaged labels Sep 1, 2022
@wilkinsona
Copy link
Member

As part of merging this, we'll need to figure out what to do about the two endpoints with the same ID. This may require some changes in the configuration properties annotation processor.

@vpavic
Copy link
Contributor Author

vpavic commented Sep 5, 2022

FWIW, I think sessions endpoint won't be the only endpoint that needs two different implementations which share the same id. #32024 has not been triaged yet, but I don't see any other way around the limitation expressed there outside of doing something similar like with sessions endpoint.

Also, is it safe to assume that #32054 solution outlined in that issue (and included in this PR) stays as it is?

@philwebb philwebb modified the milestones: 3.0.x, 3.1.x Oct 17, 2022
@wilkinsona wilkinsona modified the milestones: 3.1.x, 3.x Apr 17, 2023
@vpavic
Copy link
Contributor Author

vpavic commented Sep 25, 2023

Any updates on this?

Status quo means that the default session repository for the most popular session data store (Redis) does not have actuator endpoint capability.

@mhalbritter
Copy link
Contributor

I've updated the PR, it's now using the ReactiveFindByIndexNameSessionRepository: https://github.com/mhalbritter/spring-boot/tree/pr/32046

I'll take a look at the problem with the two endpoint ids which are the same.

@mhalbritter
Copy link
Contributor

mhalbritter commented Jan 18, 2024

PR for ignoring the duplicate property: spring-io/spring-asciidoctor-extensions#95

Edit: There's a better approach, which i've integrated into my branch.

mhalbritter pushed a commit to mhalbritter/spring-boot that referenced this pull request Jan 19, 2024
At present, Actuator sessions endpoint is supported only on a Servlet stack and also requires an indexed session repository. With Spring Session moving to non-indexed session repositories as a default for some session stores, this means that sessions endpoint won't be available unless users opt into a (non-default) indexed session repository.

This commit updates SessionEndpoint so that it is able to work with a non-indexed session repository. In such setup, it exposes operations for fetching session by id and deleting the session.

Additionally, this also adds support for reactive stack by introducing ReactiveSessionEndpoint and its auto-configuration support.

See spring-projectsgh-32046
mhalbritter added a commit to mhalbritter/spring-boot that referenced this pull request Jan 19, 2024
mhalbritter added a commit to mhalbritter/spring-boot that referenced this pull request Jan 19, 2024
mhalbritter added a commit to mhalbritter/spring-boot that referenced this pull request Jan 19, 2024
@mhalbritter mhalbritter self-assigned this Jan 22, 2024
mhalbritter pushed a commit that referenced this pull request Jan 22, 2024
At present, Actuator sessions endpoint is supported only on a Servlet stack and also requires an indexed session repository. With Spring Session moving to non-indexed session repositories as a default for some session stores, this means that sessions endpoint won't be available unless users opt into a (non-default) indexed session repository.

This commit updates SessionEndpoint so that it is able to work with a non-indexed session repository. In such setup, it exposes operations for fetching session by id and deleting the session.

Additionally, this also adds support for reactive stack by introducing ReactiveSessionEndpoint and its auto-configuration support.

See gh-32046
mhalbritter added a commit that referenced this pull request Jan 22, 2024
@mhalbritter mhalbritter modified the milestones: 3.x, 3.3.0-M2 Jan 22, 2024
@mhalbritter
Copy link
Contributor

Thanks @vpavic!

@vpavic vpavic deleted the gh-10827 branch January 22, 2024 13:51
@mhalbritter mhalbritter changed the title Provide an Actuator endpoint for non-indexed session repositories Provide an reactive actuator endpoint for indexed session repositories Jan 22, 2024
@mhalbritter mhalbritter changed the title Provide an reactive actuator endpoint for indexed session repositories Provide a reactive actuator endpoint for (username indexed) session repositories Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: merge-with-amendments Needs some changes when we merge type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide an Actuator endpoint for non-indexed session repositories
5 participants