[META] Java Annotations for BWC and API enforcement

[nknize]

Is your feature request related to a problem? Please describe.
We provide @opensearch.internal @opensearch.experimental and @opensearch.api to define code that provides a certain set of BWC guarantees. The problem is, this isn't enforceable without maintainers reviewing and manually rejecting PRs.

Describe the solution you'd like
Similar to Apache Spark's annotations, OpenSearch should introduce a new o.o.core.annotation package that (at minimum) defines enforceable java annotations as follows:

@Internal
@Api
@Experimental

And provides a mechanism to define new annotations.

• [BWC and API enforcement] Define the initial set of annotations, their meaning and relations between them #9222
• [BWC and API enforcement] Enforcing the presence of API annotations at build time #12871
• [BWC and API enforcement] Decorate the existing APIs with proper annotations #9303
• [BWC and API enforcement] Introduce checks for enforcing the API restrictions #9304
• [BWC and API enforcement] Introduce checks for enforcing the API backward compatibility #9305

Describe alternatives you've considered
We're currently using javadoc annotations. Let's make these more enforceable through java public @interface Api {}.

[reta]

@nknize do you mind if I pick it up? I think we could start from the Plugin APIs (those are by definition public APIs) and go up the dependency graph to build the complete picture. Sounds like a plan?

[nknize]

Thank you for taking this one @reta! It will be super helpful!

[reta]

Had a time to give it some thoughts, I envision this feature as a 4 step process:

1. Define the initial set of annotations, their meaning and relations between them
2. Decorate the existing APIs with proper annotations
3. Introduce checks for enforcing the API restrictions
4. Introduce checks for enforcing the API backward compatibility

Define the initial set of annotations, their meaning and relations between them

As @nknize suggested, I think it would make sense to start with this 4 annotations (please see their description for the guarantees):

/**
 * Stable public APIs that retain source and binary compatibility within a minor release.
 * These interfaces can change from one major release to another major release
 * (e.g. from 1.0 to 2.0). The types marked with this annotations could only expose
 * other {@link PublicApi} or {@link ExperimentalApi} types as public members.
 */
@Documented
@Target({ ElementType.TYPE, ElementType.PACKAGE, ElementType.METHOD, ElementType.CONSTRUCTOR })
public @interface PublicApi {

}

/**
 * Experimental APIs that may not retain source and binary compatibility within major,
 * minor or patch releases. The types marked with this annotations could only expose
 * other {@link PublicApi} or {@link ExperimentalApi} types as public members.
 */
@Documented
@Target({ ElementType.TYPE, ElementType.PACKAGE, ElementType.METHOD, ElementType.CONSTRUCTOR })
public @interface ExperimentalApi {

}

/**
 * Internal APIs that have no compatibility guarantees and should not be used outside
 * of OpenSearch core components.
 */
@Documented
@Target({ ElementType.TYPE, ElementType.PACKAGE, ElementType.METHOD, ElementType.CONSTRUCTOR })
public @interface InternalApi {

}

/**
 * Marks the public APIs as deprecated and scheduled for removal in one of the upcoming
 * major release. The types marked with this annotations could only be other {@link PublicApi}s.
 */
@Documented
@Target({ ElementType.TYPE, ElementType.PACKAGE, ElementType.METHOD, ElementType.CONSTRUCTOR })
public @interface DeprecatedApi {
    String since();
    String forRemoval();
}

Alternative arrangement under Api class:

public final class Api {
    private Api() {
    }

    /**
     * Experimental APIs that may not retain source and binary compatibility within major,
     * minor or patch releases. The types marked with this annotations could only expose
     * other {@link Public} or {@link Experimental} types as public members.
     */
    @Documented
    @Target({ ElementType.TYPE, ElementType.PACKAGE, ElementType.METHOD, ElementType.CONSTRUCTOR })
    public @interface Experimental {

    }

    /**
     * Internal APIs that have no compatibility guarantees and should be not used outside
     * of OpenSearch core components.
     */
    @Documented
    @Target({ ElementType.TYPE, ElementType.PACKAGE, ElementType.METHOD, ElementType.CONSTRUCTOR })
    public @interface Internal {

    }

    /**
     * Stable public APIs that retain source and binary compatibility within a major release.
     * These interfaces can change from one major release to another major release
     * (e.g. from 1.0 to 2.0). The types marked with this annotations could only expose
     * other {@link Public} or {@link Experimental} types as public members.
     */
    @Documented
    @Target({ ElementType.TYPE, ElementType.PACKAGE, ElementType.METHOD, ElementType.CONSTRUCTOR })
    public @interface Public {

    }

    /**
     * Marks the public APIs as deprecated and scheduled for removal in one of the upcoming
     * major release. The types marked with this annotations could only be other {@link Public}s.
     */
    @Documented
    @Target({ ElementType.TYPE, ElementType.PACKAGE, ElementType.METHOD, ElementType.CONSTRUCTOR })
    public @interface Deprecated {
        String since();
        String forRemoval();
    }
}

Decorate the existing APIs with proper annotations

The natural starting point is org.opensearch.plugins.* that by definition should be public and available to 3rd party developers (I suspect the footprint will be huge).

Introduce checks for enforcing the API restrictions

Ideally, the public API should never expose anything that is internal. To enforce that (or at least to understand the limits of how far we can go), we have to introspect each type annotated with @Api or @Exprimental and make sure it leaks only @Api or @Exprimental types as well (exceptions, method return values, method arguments, annotations). How can we do that?

• craft annotation processor

  @SupportedAnnotationTypes("org.opensearch.common.annotation.*")
  public class ApiAnnotationProcessor extends AbstractProcessor {
      @Override
      public SourceVersion getSupportedSourceVersion() {
          return SourceVersion.latest();
      }
  
      @Override
      public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment round) {
          final Set<? extends Element> elements = round.getElementsAnnotatedWithAny(Set.of(Api.class, Experimental.class));
          processingEnv.getMessager().printMessage(Kind.NOTE, "Processing Api annotations");
  
          for (var element : elements) {
              if (element instanceof TypeElement) {
                  final TypeElement type = (TypeElement) element;
                  process(type);
              } 
               ....
          } 
  
          return false;
      }
  }

• use something like ArchUnit and introduce the API level tests (pseudo rule is below but it is very close to what we could have as real one)

  public class ApiRules {
      @ArchTest
      public static final ArchRule PUBLIC_API_METHODS_USE_ONLY_PUBLIC_OR_EXPERIMENTAL_API_TYPES =
          freeze(
              methods()
                  .that()
                  .areDeclaredInClassesThat().resideInAPackage("org.opensearch..")
                  .and().arePublic()
                  .should(
                      haveArgsOrReturnTypes()
                          .that()
                          .resideOutsideOfPackage("org.opensearch..")
                          .or()
                              .resideInAPackage("org.opensearch..")
                              .and(areAnnotatedWithAtLeastOneOf(Public.class, Experimental.class)))
                  .as("Return and argument types of methods annotated with @Api or @Experimental must be annotated with @Api or @Experimental."));
  }

Introduce checks for enforcing the API backward compatibility

This is the final goal, once we have the API outlined (annotated with @Api), we could enforce the compatibility rules that have to be applied between version. The japicmp, that was explored here #9044, is one of the most advanced tools we could use for these purposes.

[peternied]

@reta Thanks for digging in to this, really like the approach, couple of standouts, happy to move these comments to another issue if you are going to put together an RFC(s).

• I would prefer a more descriptive annotation name @PublicSupportedApi, other names @SemverBreakingApi, @BackwardsCompatiableApi.
• What kinds of things do we want to put the @Internal annotation on vs items with no annotation, are there examples?
• There is a concern about marking the Api as ready when it isn't, thoughts on how we will gate this?
• OK we need to make a breaking change to an API, what do we do?

[peternied]

One more:

• Rather than work backward from all of plugins namespace, maybe we can subdivide that effort further, by starting with the APIs consumed by the Extensions SDK [repo], as those interfaces have had the most recent thought about exposing. If there are components that should not be exposed we would need to alter the SDK design.

[andrross]

@reta What will this look like in a post-JPMS world? If my understanding is correct, everything exported by a JPMS module would be implicitly an @Api and probably not need an annotation, whereas everything not exported would be inaccessible to downstream dependencies and also not need an annotation. The @Experimental annotation would probably still be useful though. Is that right?

[reta]

@reta What will this look like in a post-JPMS world? If my understanding is correct, everything exported by a JPMS module would be implicitly an @Api and probably not need an annotation, whereas everything not exported would be inaccessible to downstream dependencies and also not need an annotation. The @Experimental annotation would probably still be useful though. Is that right?

Thanks @andrross , yes, the @Api & @Experimental in JPMS world would fall into "export to everyone" category, however the catch here is that exports are by packages, not individual classes so the @Internal may still be useful in some places, at least till we move the affected classes under *.internal.* packages.