-
Notifications
You must be signed in to change notification settings - Fork 132
[FEATURE] Integrate remote metadata SDK client with alerting plugin (#2046) #2047
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,10 @@ package org.opensearch.alerting.settings | |
| import org.opensearch.alerting.AlertingPlugin | ||
| import org.opensearch.common.settings.Setting | ||
| import org.opensearch.common.unit.TimeValue | ||
| import org.opensearch.remote.metadata.common.CommonValue.REMOTE_METADATA_ENDPOINT_KEY | ||
| import org.opensearch.remote.metadata.common.CommonValue.REMOTE_METADATA_REGION_KEY | ||
| import org.opensearch.remote.metadata.common.CommonValue.REMOTE_METADATA_SERVICE_NAME_KEY | ||
| import org.opensearch.remote.metadata.common.CommonValue.REMOTE_METADATA_TYPE_KEY | ||
| import java.util.concurrent.TimeUnit | ||
| import java.util.function.Function | ||
|
|
||
|
|
@@ -19,6 +23,7 @@ class AlertingSettings { | |
| companion object { | ||
| const val DEFAULT_MAX_ACTIONABLE_ALERT_COUNT = 50L | ||
| const val DEFAULT_FINDINGS_INDEXING_BATCH_SIZE = 1000 | ||
| private const val REMOTE_METADATA_KEY_PREFIX = "plugins.alerting" | ||
| const val DEFAULT_PERCOLATE_QUERY_NUM_DOCS_IN_MEMORY = 50000 | ||
| const val DEFAULT_PERCOLATE_QUERY_DOCS_SIZE_MEMORY_PERCENTAGE_LIMIT = 10 | ||
| const val DEFAULT_DOC_LEVEL_MONITOR_SHARD_FETCH_SIZE = 10000 | ||
|
|
@@ -311,5 +316,36 @@ class AlertingSettings { | |
| Setting.Property.NodeScope, | ||
| Setting.Property.Dynamic | ||
| ) | ||
|
|
||
| val MULTI_TENANCY_ENABLED: Setting<Boolean> = Setting.boolSetting( | ||
| "$REMOTE_METADATA_KEY_PREFIX.multi_tenancy_enabled", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is why is the setting itself different on each plugin? shouldn't all plugins have a single settingname?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| false, | ||
| Setting.Property.NodeScope, | ||
| Setting.Property.Final | ||
| ) | ||
|
|
||
| val REMOTE_METADATA_STORE_TYPE: Setting<String?> = Setting.simpleString( | ||
| "$REMOTE_METADATA_KEY_PREFIX.$REMOTE_METADATA_TYPE_KEY", | ||
| Setting.Property.NodeScope, | ||
| Setting.Property.Final | ||
| ) | ||
|
|
||
| val REMOTE_METADATA_ENDPOINT: Setting<String?> = Setting.simpleString( | ||
| "$REMOTE_METADATA_KEY_PREFIX.$REMOTE_METADATA_ENDPOINT_KEY", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can this be null for opensearch cluster deployment?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.NodeScope, | ||
| Setting.Property.Final | ||
| ) | ||
|
|
||
| val REMOTE_METADATA_REGION: Setting<String?> = Setting.simpleString( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can this be null for opensearch cluster deployment?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this is only needed if we are setting a remote metadata store. |
||
| "$REMOTE_METADATA_KEY_PREFIX.$REMOTE_METADATA_REGION_KEY", | ||
| Setting.Property.NodeScope, | ||
| Setting.Property.Final | ||
| ) | ||
|
|
||
| val REMOTE_METADATA_SERVICE_NAME: Setting<String?> = Setting.simpleString( | ||
| "$REMOTE_METADATA_KEY_PREFIX.$REMOTE_METADATA_SERVICE_NAME_KEY", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is there no default value for this for opensearch clusters for all these settings?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.NodeScope, | ||
| Setting.Property.Final | ||
| ) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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