[BWC and API enforcement] Decorate the existing APIs with proper annotations (part 1) #9520

reta · 2023-08-23T20:14:09Z

Description

Decorate the existing APIs with proper annotations. This is merely the first part so we could work through the small changesets vs big bang pull request. The API validations are fully automated (as annotation processor #9304), here is the sample output (the annotation processor is not part of this pull request since it would generated noisy warnings right now):

error: The element org.opensearch.core.xcontent.XContentBuilder is part of the public APIs but is not maked as @PublicApi or @ExperimentalApi (referenced by org.opensearch.Version) 
error: The element org.opensearch.client.Client is part of the public APIs but is not maked as @PublicApi or @ExperimentalApi (referenced by org.opensearch.plugins.Plugin)  
error: The element org.opensearch.cluster.service.ClusterService is part of the public APIs but is not maked as @PublicApi or @ExperimentalApi (referenced by org.opensearch.plugins.Plugin) 
error: The element org.opensearch.threadpool.ThreadPool is part of the public APIs but is not maked as @PublicApi or @ExperimentalApi (referenced by org.opensearch.plugins.Plugin) 
error: The element org.opensearch.watcher.ResourceWatcherService is part of the public APIs but is not maked as @PublicApi or @ExperimentalApi (referenced by org.opensearch.plugins.Plugin) 
error: The element org.opensearch.script.ScriptService is part of the public APIs but is not maked as @PublicApi or @ExperimentalApi (referenced by org.opensearch.plugins.Plugin) 
error: The element org.opensearch.core.xcontent.NamedXContentRegistry is part of the public APIs but is not maked as @PublicApi or @ExperimentalApi (referenced by org.opensearch.plugins.Plugin) 
error: The element org.opensearch.env.Environment is part of the public APIs but is not maked as @PublicApi or @ExperimentalApi (referenced by org.opensearch.plugins.Plugin) 
error: The element org.opensearch.env.NodeEnvironment is part of the public APIs but is not maked as @PublicApi or @ExperimentalApi (referenced by org.opensearch.plugins.Plugin) 
error: The element org.opensearch.core.common.io.stream.NamedWriteableRegistry is part of the public APIs but is not maked as @PublicApi or @ExperimentalApi (referenced by org.opensearch.plugins.Plugin) 
error: The element org.opensearch.cluster.metadata.IndexNameExpressionResolver is part of the public APIs but is not maked as @PublicApi or @ExperimentalApi (referenced by org.opensearch.plugins.Plugin) 
error: The element org.opensearch.index.IndexModule is part of the public APIs but is not maked as @PublicApi or @ExperimentalApi (referenced by org.opensearch.plugins.Plugin) 
error: The element org.opensearch.core.common.io.stream.StreamInput is part of the public APIs but is not maked as @PublicApi or @ExperimentalApi (referenced by org.opensearch.common.settings.SettingsException) 
error: The element org.opensearch.core.common.unit.ByteSizeUnit is part of the public APIs but is not maked as @PublicApi or @ExperimentalApi (referenced by org.opensearch.common.settings.Settings.Builder) 
error: The element org.opensearch.core.xcontent.MediaType is part of the public APIs but is not maked as @PublicApi or @ExperimentalApi (referenced by org.opensearch.common.settings.Settings.Builder) 
error: The element org.opensearch.core.common.io.stream.StreamInput is part of the public APIs but is not maked as @PublicApi or @ExperimentalApi (referenced by org.opensearch.common.settings.Settings) 
error: The element org.opensearch.core.common.io.stream.StreamOutput is part of the public APIs but is not maked as @PublicApi or @ExperimentalApi (referenced by org.opensearch.common.settings.Settings) 
error: The element org.opensearch.core.xcontent.ToXContent.Params is part of the public APIs but is not maked as @PublicApi or @ExperimentalApi (referenced by org.opensearch.common.settings.Settings) 
error: The element org.opensearch.core.xcontent.XContentParser is part of the public APIs but is not maked as @PublicApi or @ExperimentalApi (referenced by org.opensearch.common.settings.Settings) 
error: The element org.opensearch.core.xcontent.XContentParser is part of the public APIs but is not maked as @PublicApi or @ExperimentalApi (referenced by org.opensearch.common.settings.Settings) 
error: The element org.opensearch.core.common.unit.ByteSizeUnit is part of the public APIs but is not maked as @PublicApi or @ExperimentalApi (referenced by org.opensearch.common.settings.Settings.Builder) 
error: The element org.opensearch.core.xcontent.MediaType is part of the public APIs but is not maked as @PublicApi or @ExperimentalApi (referenced by org.opensearch.common.settings.Settings.Builder) 
error: The element org.opensearch.core.xcontent.ToXContent.Params is part of the public APIs but is not maked as @PublicApi or @ExperimentalApi (referenced by org.opensearch.Version)

Related Issues

Part of #9303

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff
Commit changes are listed out in CHANGELOG.md file (See: Changelog)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

libs/common/src/main/java/org/opensearch/common/lifecycle/LifecycleComponent.java

github-actions · 2023-08-23T20:19:30Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/23337/
CommitID: d078196
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

libs/core/src/main/java/org/opensearch/core/ParseField.java

opensearch-trigger-bot · 2023-08-23T20:43:26Z

Compatibility status:

Checks if related components are compatible with change d1678ba

Incompatible components

Incompatible components: [https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/security-analytics.git]

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git]

github-actions · 2023-08-24T02:00:50Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/23365/
CommitID: c5a75b4
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

github-actions · 2023-08-24T12:14:25Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/23395/
CommitID: c5a75b4

codecov · 2023-08-24T12:15:18Z

Codecov Report

Merging #9520 (959b94c) into main (60787b8) will decrease coverage by 0.45%.
Report is 2 commits behind head on main.
The diff coverage is 87.50%.

@@             Coverage Diff              @@
##               main    #9520      +/-   ##
============================================
- Coverage     71.53%   71.09%   -0.45%     
+ Complexity    57897    57568     -329     
============================================
  Files          4782     4782              
  Lines        271332   271332              
  Branches      39614    39614              
============================================
- Hits         194107   192901    -1206     
- Misses        61255    62244     +989     
- Partials      15970    16187     +217

Files Changed	Coverage Δ
...ain/java/org/opensearch/common/unit/TimeValue.java	`86.25% <ø> (-1.25%)`	⬇️
...ibs/core/src/main/java/org/opensearch/Version.java	`77.77% <ø> (ø)`
.../src/main/java/org/opensearch/core/ParseField.java	`95.74% <ø> (ø)`
...ava/org/opensearch/core/action/ActionListener.java	`78.50% <0.00%> (-4.68%)`	⬇️
...g/opensearch/core/common/bytes/BytesReference.java	`74.28% <ø> (ø)`
.../core/common/io/stream/NamedWriteableRegistry.java	`100.00% <ø> (ø)`
.../opensearch/core/common/io/stream/StreamInput.java	`89.06% <ø> (-0.40%)`	⬇️
...opensearch/core/common/io/stream/StreamOutput.java	`95.68% <ø> (-0.16%)`	⬇️
.../opensearch/core/common/settings/SecureString.java	`75.75% <ø> (-3.04%)`	⬇️
...org/opensearch/core/common/unit/ByteSizeValue.java	`99.08% <ø> (ø)`
... and 38 more

... and 442 files with indirect coverage changes

github-actions · 2023-08-25T15:55:44Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/23500/
CommitID: 25dbda7

github-actions · 2023-08-28T20:33:08Z

Compatibility status:

Checks if related components are compatible with change 25dbda7

Incompatible components

Incompatible components: [https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/asynchronous-search.git]

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git]

nknize

Quick glance this looks good. Going to have a look tomorrow w/ fresh eyes.

nknize

I have some initial questions. The big one is around marking server classes as Public before we have their JPMS package home. Is that namespace enforced?

libs/common/src/main/java/org/opensearch/common/lifecycle/LifecycleComponent.java

libs/core/src/main/java/org/opensearch/core/ParseField.java

server/src/main/java/org/opensearch/bootstrap/BootstrapCheck.java

server/src/main/java/org/opensearch/bootstrap/BootstrapContext.java

server/src/main/java/org/opensearch/client/AdminClient.java

reta · 2023-08-29T15:33:19Z

I have some initial questions. The big one is around marking server classes as Public before we have their JPMS package home. Is that namespace enforced

Thanks @nknize , so JPMS won't help here: these classes we expose through Plugin interface to all plugin developers. We could hide them behind JPMS modules but the Plugin interface will become unusable. The ways we could approach them are a) reduce the Plugin interface (removing some hooks) b) introduce alternative interfaces with significantly smaller APIs blast radius c) something else?

We could always put @InternalApi to such classes but we'll be lying ourselves and most importantly, confuse the users (plugin implementers) with restrictions.

dblock · 2023-08-29T21:52:41Z

Thanks @nknize , so JPMS won't help here: these classes we expose through Plugin interface to all plugin developers.

The working alternative is https://github.com/opensearch-project/opensearch-sdk-java. None of the interfaces in those implementations drag in the kitchen sink. Import the SDK here and make that the interface you publish to plugins. We can remove the self-hosted nature of extensions and call that plugin v2, too. It's a 1-time upgrade for plugins to v2.

reta · 2023-08-29T22:02:52Z

The working alternative is https://github.com/opensearch-project/opensearch-sdk-java. None of the interfaces in those implementations drag in the kitchen sink.

Thanks @dblock , we already touched on that (as an example, see please https://github.com/opensearch-project/opensearch-sdk-java/blob/main/src/main/java/org/opensearch/sdk/api/AnalysisExtension.java for core public API exposure needed), but extensions (eventually) should be clean and green, but they are not yet. As we also discussed, plugins are here to stay and we have difficulties evolving the APIs.

nknize · 2023-08-29T22:09:37Z

so JPMS won't help here...

No I mean how about not annotating any server classes in this PR yet, until after we move the classes to the "rightful" namespace for the JPMS effort. e.g., SecureSettings is going to move to core so the namespace is going to change from o.o.common.settings to o.o.core.common.settings (or something similar). If we mark that w/ @PublicApi will the enforcement break when it's moved namespaces?

reta · 2023-08-30T01:32:52Z

If we mark that w/ @PublicApi will the enforcement break when it's moved namespaces?

Oh I see, it will break eventually but not right now. The enforcement is not yet plugged in since we do have a large chunk of files to annotate. Once the public API is 100% scoped, we could turn on the enforcement (last task in #8127)

nknize

One little version nit.. otherwise LGTM!

libs/core/src/main/java/org/opensearch/core/xcontent/MediaType.java

…tations (part 1) Signed-off-by: Andriy Redko <[email protected]>

Signed-off-by: Andriy Redko <[email protected]>

github-actions · 2023-08-30T15:22:43Z

Compatibility status:

Checks if related components are compatible with change 959b94c

Incompatible components

Incompatible components: [https://github.com/opensearch-project/cross-cluster-replication.git]

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/reporting.git]

github-actions · 2023-08-30T15:26:33Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.snapshots.CloneSnapshotIT.testShallowCloneNameAvailability

URL: https://build.ci.opensearch.org/job/gradle-check/23825/
CommitID: 959b94c
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

opensearch-trigger-bot · 2023-08-30T15:57:51Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch/backport-2.x
# Create a new branch
git switch --create backport/backport-9520-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 78eea275a3e1812a008e815d846ca871d7f09d20
# Push it to GitHub
git push --set-upstream origin backport/backport-9520-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-9520-to-2.x.

…tations (part 1) (opensearch-project#9520) * [BWC and API enforcement] Decorate the existing APIs with proper annotations (part 1) Signed-off-by: Andriy Redko <[email protected]> * Address code review comments Signed-off-by: Andriy Redko <[email protected]> --------- Signed-off-by: Andriy Redko <[email protected]> (cherry picked from commit 78eea27)

…tations (part 1) (#9520) (#9638) * [BWC and API enforcement] Decorate the existing APIs with proper annotations (part 1) Signed-off-by: Andriy Redko <[email protected]> * Address code review comments Signed-off-by: Andriy Redko <[email protected]> --------- Signed-off-by: Andriy Redko <[email protected]> (cherry picked from commit 78eea27) Signed-off-by: Andriy Redko <[email protected]>

…tations (part 1) (opensearch-project#9520) * [BWC and API enforcement] Decorate the existing APIs with proper annotations (part 1) Signed-off-by: Andriy Redko <[email protected]> * Address code review comments Signed-off-by: Andriy Redko <[email protected]> --------- Signed-off-by: Andriy Redko <[email protected]> Signed-off-by: Kaushal Kumar <[email protected]>

…tations (part 1) (opensearch-project#9520) * [BWC and API enforcement] Decorate the existing APIs with proper annotations (part 1) Signed-off-by: Andriy Redko <[email protected]> * Address code review comments Signed-off-by: Andriy Redko <[email protected]> --------- Signed-off-by: Andriy Redko <[email protected]> Signed-off-by: Ivan Brusic <[email protected]>

…tations (part 1) (opensearch-project#9520) * [BWC and API enforcement] Decorate the existing APIs with proper annotations (part 1) Signed-off-by: Andriy Redko <[email protected]> * Address code review comments Signed-off-by: Andriy Redko <[email protected]> --------- Signed-off-by: Andriy Redko <[email protected]> Signed-off-by: Shivansh Arora <[email protected]>

reta added the skip-changelog label Aug 23, 2023

reta commented Aug 23, 2023

View reviewed changes

libs/common/src/main/java/org/opensearch/common/lifecycle/LifecycleComponent.java Show resolved Hide resolved

reta commented Aug 23, 2023

View reviewed changes

libs/core/src/main/java/org/opensearch/core/ParseField.java Show resolved Hide resolved

reta force-pushed the issue-9303.1 branch from d078196 to c5a75b4 Compare August 23, 2023 20:26

reta added API Issues with external APIs and removed skip-changelog labels Aug 23, 2023

reta force-pushed the issue-9303.1 branch from c5a75b4 to 25dbda7 Compare August 25, 2023 15:10

reta requested a review from msfroh as a code owner August 25, 2023 15:10

nknize reviewed Aug 29, 2023

View reviewed changes

nknize requested changes Aug 29, 2023

View reviewed changes

nknize approved these changes Aug 30, 2023

View reviewed changes

libs/core/src/main/java/org/opensearch/core/xcontent/MediaType.java Outdated Show resolved Hide resolved

reta added 2 commits August 30, 2023 10:42

[BWC and API enforcement] Decorate the existing APIs with proper anno…

d484b7e

…tations (part 1) Signed-off-by: Andriy Redko <[email protected]>

Address code review comments

959b94c

Signed-off-by: Andriy Redko <[email protected]>

reta force-pushed the issue-9303.1 branch from 25dbda7 to 959b94c Compare August 30, 2023 14:42

reta merged commit 78eea27 into opensearch-project:main Aug 30, 2023
12 checks passed

reta added the backport 2.x Backport to 2.x branch label Aug 30, 2023

opensearch-trigger-bot bot added the backport-failed label Aug 30, 2023

reta mentioned this pull request Aug 30, 2023

[Backport] [2.x] [BWC and API enforcement] Decorate the existing APIs with proper annotations (part 1) (#9520) #9638

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BWC and API enforcement] Decorate the existing APIs with proper annotations (part 1) #9520

[BWC and API enforcement] Decorate the existing APIs with proper annotations (part 1) #9520

reta commented Aug 23, 2023

github-actions bot commented Aug 23, 2023

opensearch-trigger-bot bot commented Aug 23, 2023

github-actions bot commented Aug 24, 2023

github-actions bot commented Aug 24, 2023

codecov bot commented Aug 24, 2023 •

edited

Loading

github-actions bot commented Aug 25, 2023

github-actions bot commented Aug 28, 2023

nknize left a comment

nknize left a comment

reta commented Aug 29, 2023

dblock commented Aug 29, 2023 •

edited

Loading

reta commented Aug 29, 2023

nknize commented Aug 29, 2023

reta commented Aug 30, 2023 •

edited

Loading

nknize left a comment

github-actions bot commented Aug 30, 2023

github-actions bot commented Aug 30, 2023

opensearch-trigger-bot bot commented Aug 30, 2023

[BWC and API enforcement] Decorate the existing APIs with proper annotations (part 1) #9520

[BWC and API enforcement] Decorate the existing APIs with proper annotations (part 1) #9520

Conversation

reta commented Aug 23, 2023

Description

Related Issues

Check List

github-actions bot commented Aug 23, 2023

Gradle Check (Jenkins) Run Completed with:

opensearch-trigger-bot bot commented Aug 23, 2023

Compatibility status:

Incompatible components

Skipped components

Compatible components

github-actions bot commented Aug 24, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Aug 24, 2023

Gradle Check (Jenkins) Run Completed with:

codecov bot commented Aug 24, 2023 • edited Loading

Codecov Report

github-actions bot commented Aug 25, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Aug 28, 2023

Compatibility status:

Incompatible components

Skipped components

Compatible components

nknize left a comment

Choose a reason for hiding this comment

nknize left a comment

Choose a reason for hiding this comment

reta commented Aug 29, 2023

dblock commented Aug 29, 2023 • edited Loading

reta commented Aug 29, 2023

nknize commented Aug 29, 2023

reta commented Aug 30, 2023 • edited Loading

nknize left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 30, 2023

Compatibility status:

Incompatible components

Skipped components

Compatible components

github-actions bot commented Aug 30, 2023

Gradle Check (Jenkins) Run Completed with:

opensearch-trigger-bot bot commented Aug 30, 2023

codecov bot commented Aug 24, 2023 •

edited

Loading

dblock commented Aug 29, 2023 •

edited

Loading

reta commented Aug 30, 2023 •

edited

Loading