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

AOT Optimizations #868

Merged
merged 58 commits into from
Nov 16, 2022
Merged

AOT Optimizations #868

merged 58 commits into from
Nov 16, 2022

Conversation

sdelamo
Copy link
Contributor

@sdelamo sdelamo commented Dec 16, 2021

No description provided.

@melix
Copy link
Contributor

melix commented Dec 21, 2021

The approach we used during the pairing session will not work. After thinking about this while having lunch, we are generating source code which is a bean. However that bean won't be processed by Micronaut annotation processors, so bean definitions won't be generated (AOT runs after that phase). Even if we made it so that it can generate such beans, then they wouldn't be used in any case if the "service loading" optimization is enabled, because we would scan for available beans before this class is generated, so it would never appear in the final list of beans.

There is, however, a different approach we can use, which is the one which is most used in core itself. There's a class called StaticOptimizations which is used to injects static optimizations at runtime. The DefaultOpenIdProviderMetadataFetcher should make use of this class:

public class DefaultOpenIdProviderMetadataFetcher implements OpenIdProviderMetadataFetcher {
    private static final Logger LOG = LoggerFactory.getLogger(DefaultOpenIdProviderMetadataFetcher.class);
    private final static Optimizations OPTIMIZATIONS = StaticOptimizations.get(Optimizations.class).orElse(new Optimizations(Collections.emptyMap()));

    private final HttpClient client;
    private final ObjectMapper objectMapper;
    private final OpenIdClientConfiguration openIdClientConfiguration;

    public static class Optimizations {
        private final Map<String, Supplier<DefaultOpenIdProviderMetadata>> suppliers;

        public Optimizations(Map<String, Supplier<DefaultOpenIdProviderMetadata>> suppliers) {
            this.suppliers = suppliers;
        }

        public Optional<Supplier<DefaultOpenIdProviderMetadata>> findMetadata(String name) {
            return Optional.ofNullable(suppliers.get(name));
        }
    }

    public DefaultOpenIdProviderMetadataFetcher(OpenIdClientConfiguration openIdClientConfiguration,
                                                ObjectMapper objectMapper,
                                                @Client HttpClient client) {
        this.openIdClientConfiguration = openIdClientConfiguration;
        this.objectMapper = objectMapper;
        this.client = client;
    }

    @Override
    @NonNull
    public DefaultOpenIdProviderMetadata fetch() {
        return OPTIMIZATIONS.findMetadata(openIdClientConfiguration.getName())
                .map(Supplier::get)
                .orElseGet(() -> openIdClientConfiguration.getIssuer()
                        .map(issuer -> {
                            try {
                                URL configurationUrl = new URL(issuer, StringUtils.prependUri(issuer.getPath(), openIdClientConfiguration.getConfigurationPath()));
                                if (LOG.isDebugEnabled()) {
                                    LOG.debug("Sending request for OpenID configuration for provider [{}] to URL [{}]", openIdClientConfiguration.getName(), configurationUrl);
                                }
                                //TODO this returns ReadTimeoutException - return issuerClient.toBlocking().retrieve(configurationUrl.toString(), DefaultOpenIdProviderMetadata.class);
                                String json = client.toBlocking().retrieve(configurationUrl.toString(), String.class);
                                return objectMapper.readValue(json, DefaultOpenIdProviderMetadata.class);

                            } catch (MalformedURLException e) {
                                throw new BeanInstantiationException("Failure parsing issuer URL " + issuer.toString(), e);
                            } catch (HttpClientResponseException e) {
                                throw new BeanInstantiationException("Failed to retrieve OpenID configuration for " + openIdClientConfiguration.getName(), e);

                            } catch (JsonProcessingException e) {
                                throw new BeanInstantiationException("JSON Processing Exception parsing issuer URL returned JSON " + issuer.toString(), e);
                            }
                        }).orElse(new DefaultOpenIdProviderMetadata())
                );
    }
}

then our code generator needs to create the injection of metadata via context.registerStaticInitializer() instead.

This addresses a couple problems:

- AOT cannot generate new beans, because they wouldn't be processed
by Micronaut annotation processors
- new beans wouldn't be visible to other optimizations, in particular
the one which preloads services

As a consequence, we rework the code generator so that it uses
the static optimizations facility instead.
@melix
Copy link
Contributor

melix commented Dec 21, 2021

I have pushed changes showing what I mean by using StaticOptimizations. Looking at the other issues now.

melix and others added 2 commits December 21, 2021 16:43
This fixes the issue with the environment bean not available.
@sdelamo sdelamo changed the title refactor: extract OpenIdProviderMetadataFetcher AOT Optimizations Dec 22, 2021
This commit introduces a new project, `test-suite-aot`, aimed at testing
that the AOT module works properly. Currently it doesn't do much except
making sure that the AOT code generator is called.
@sdelamo sdelamo added this to the 3.3.0 milestone Jan 3, 2022
sdelamo and others added 5 commits January 3, 2022 10:30
In Micronaut AOT 1.0.0-M6, loading the static optimizations has
been reworked and needs to be done using a dedicated API. This is
what this commit fixes. The API change was needed to make sure
that the optimizations are set _before_ they are actually read.
There was a possibility that the optimizations are set after,
typically in AWS functions.
@sdelamo sdelamo marked this pull request as ready for review November 15, 2022 07:36
@sdelamo
Copy link
Contributor Author

sdelamo commented Nov 15, 2022

For a controller such as:

package example.micronaut;

import io.micronaut.http.HttpStatus;
import io.micronaut.http.annotation.Controller;
import io.micronaut.http.annotation.Get;
import io.micronaut.http.annotation.Status;
import io.micronaut.security.oauth2.client.DefaultOpenIdProviderMetadata;
import io.micronaut.security.oauth2.client.OpenIdProviderMetadataFetcher;
import io.micronaut.security.token.jwt.signature.jwks.JwkSetFetcher;
import jakarta.annotation.security.PermitAll;

@Controller
public class HomeController {
    
    public HomeController(OpenIdProviderMetadataFetcher openIdProviderMetadataFetcher, JwkSetFetcher jwkSetFetcher) {
        DefaultOpenIdProviderMetadata metadata = openIdProviderMetadataFetcher.fetch();
        jwkSetFetcher.fetch(metadata.getJwksUri());
    }

    @PermitAll
    @Get
    @Status(HttpStatus.OK)
    void index() {
    }
}

which exercise every Micronaut Security AOT Optimization, we get 1s improvement.

Optimization TTFR
None 2,6420
OpenID Configuration 2,075
JWKSet 2,182
OpenID Configuration + JWKSet 1,569

Micronaut Security AOT Optimizations

Copy link
Contributor

@timyates timyates left a comment

Choose a reason for hiding this comment

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

Spotted we're publishing the test-suite-utils module to maven, I'll raise a separate PR

...later... #1137

test-suite-aot.sh Outdated Show resolved Hide resolved
Co-authored-by: Tim Yates <[email protected]>
dependencies {
...
..
aotPlugins("io.micronaut.security:micronaut-security-aot")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you will need a version here too, because I think we don't apply the Micronaut BOM to all configurations, but I may be wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a version and a link to releases 7bcfa42

https://micronaut-projects.github.io/micronaut-aot/latest/guide/[Micronaut AOT] is a framework which implements ahead-of-time (AOT) optimizations for Micronaut application and libraries. Those optimizations consist of computing at build time things which would normally be done at runtime]
____

Micronaut Security offers several ahead-of-time optimizations.
Copy link
Contributor

Choose a reason for hiding this comment

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

⭕ Maybe clarify the tradeoffs of using AOT: you will save requests at the application startup, which can save a few seconds, at the cost of having a binary which is bound to a specific configuration version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a asciidoc warning callout e420598

@sdelamo sdelamo removed this from the 3.3.0 milestone Nov 16, 2022
@sonarcloud
Copy link

sonarcloud bot commented Nov 16, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug C 1 Bug
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 19 Code Smells

23.3% 23.3% Coverage
0.0% 0.0% Duplication

@sdelamo sdelamo merged commit c946260 into 3.9.x Nov 16, 2022
@sdelamo sdelamo deleted the extract-fetcher branch November 16, 2022 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants