[FEATURE] Integrate remote metadata SDK client with alerting plugin (#2046)#2047
Conversation
…search-project#2046) - Add opensearch-remote-metadata-sdk dependency - Add multi_tenancy_enabled and remote metadata settings - Create SdkClient in createComponents and return as component - Register all new settings - Add unit tests for settings registration and defaults Signed-off-by: Mohammed Aghil Puthiyottil <57040494+MohammedAghil@users.noreply.github.com>
MohammedAghil
requested review from
AWSHurneyt,
amsiglan,
bowenlan-amzn,
eirsep,
engechas,
getsaurabh02,
goyamegh,
jowg-amazon,
lezzago,
praveensameneni,
rishabhmaurya,
riysaxen-amzn,
sbcd90 and
toepkerd
as code owners
| ) | ||
|
|
||
| val MULTI_TENANCY_ENABLED: Setting<Boolean> = Setting.boolSetting( | ||
| "$REMOTE_METADATA_KEY_PREFIX.multi_tenancy_enabled", |
There was a problem hiding this comment.
why is multi_tenancy_enabled not common variable coming from remote metadata sdk?
why is the setting itself different on each plugin? shouldn't all plugins have a single settingname?
There was a problem hiding this comment.
This is a common pattern followed in other plugins too. ml-commons, one of the first plugin to adopt the remote metadata sdk used similar pattern : https://github.com/opensearch-project/ml-commons/blob/82da99868a0074374fa7274ceaa8d0ca291e736e/plugin/src/main/java/org/opensearch/ml/settings/MLCommonsSettings.java#L317-L335.
| ) | ||
|
|
||
| val REMOTE_METADATA_SERVICE_NAME: Setting<String?> = Setting.simpleString( | ||
| "$REMOTE_METADATA_KEY_PREFIX.$REMOTE_METADATA_SERVICE_NAME_KEY", |
There was a problem hiding this comment.
why is there no default value for this for opensearch clusters for all these settings?
There was a problem hiding this comment.
These settings defaults to an empty string. When the values are empty, the sdk client falls back to the local OpenSearch cluster client — so the default behavior is unchanged for standard open search clusters. The settings only need explicit values when configuring a remote metadata store.
| Setting.Property.Final | ||
| ) | ||
|
|
||
| val REMOTE_METADATA_REGION: Setting<String?> = Setting.simpleString( |
There was a problem hiding this comment.
can this be null for opensearch cluster deployment?
There was a problem hiding this comment.
Yes, this is only needed if we are setting a remote metadata store.
| ) | ||
|
|
||
| val REMOTE_METADATA_ENDPOINT: Setting<String?> = Setting.simpleString( | ||
| "$REMOTE_METADATA_KEY_PREFIX.$REMOTE_METADATA_ENDPOINT_KEY", |
There was a problem hiding this comment.
can this be null for opensearch cluster deployment?
There was a problem hiding this comment.
Yes it will by default use the system indices client provided in https://github.com/opensearch-project/alerting/pull/2047/changes#diff-e3fd9aa75de7c973b2c9e6938d97987c46f11356fdba50ee1557462e63de9e61R341 which is the existing behaviour
| Setting.Property.Dynamic | ||
| ) | ||
|
|
||
| val MULTI_TENANCY_ENABLED: Setting<Boolean> = Setting.boolSetting( |
There was a problem hiding this comment.
can this be changed via _cluster/settings API?
if yes, that doesn't seem desirable to me..
this should be only changeable via static XML.
There was a problem hiding this comment.
No, it cannot be changed at runtime. The setting uses Setting.Property.Final which makes it immutable after node startup — it can only be set in opensearch.yml
…in (opensearch-project#2046) (opensearch-project#2047)" This reverts commit 7fae816.
…in (opensearch-project#2046) (opensearch-project#2047)" This reverts commit 7fae816. Signed-off-by: Manaswini Ragamouni <ragamanu@amazon.com>
…in (opensearch-project#2046) (opensearch-project#2047)" This reverts commit 7fae816. Signed-off-by: Manaswini Ragamouni <ragamanu@amazon.com>
…in (opensearch-project#2046) (opensearch-project#2047)" This reverts commit 7fae816. Signed-off-by: Manaswini Ragamouni <ragamanu@amazon.com>
* Revert "Inject SdkClient into transport actions for SDK persistence support (#2052)" This reverts commit 551e26e. Signed-off-by: Manaswini Ragamouni <ragamanu@amazon.com> * Revert "feat: Integrate remote metadata SDK client with alerting plugin (#2046) (#2047)" This reverts commit 7fae816. Signed-off-by: Manaswini Ragamouni <ragamanu@amazon.com> --------- Signed-off-by: Manaswini Ragamouni <ragamanu@amazon.com> Co-authored-by: Manaswini Ragamouni <ragamanu@amazon.com>
4 participants
Description
This integrates the remote metadata SDK client with the alerting plugin, enabling support for remote metadata stores as an alternative to local OpenSearch indices for storing plugin metadata.
A new
plugins.alerting.multi_tenancy_enabledboolean setting is added to control theTENANT_AWARE_KEYpassed to the SDK client. Four additional settings (remote_metadata_type, remote_metadata_endpoint,remote_metadata_region, remote_metadata_service_name) configure the remote metadata backend. All settings are registered in getSettings() and the SdkClient is created in createComponents() and returned as a plugin
component.
When multi_tenancy_enabled is false (default), the SDK client operates as a passthrough to the local OpenSearch client with no behavior change. When enabled, the SDK client routes metadata operations to the configured remote store.
Related Issues
Resolves #2046
Check List
--signoff.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.