Skip to content

PPL Alerting: changing max PPL Monitor name length to 100 - #962

Merged
toepkerd merged 1 commit into
opensearch-project:mainfrom
toepkerd:main
May 26, 2026
Merged

PPL Alerting: changing max PPL Monitor name length to 100#962
toepkerd merged 1 commit into
opensearch-project:mainfrom
toepkerd:main

Conversation

@toepkerd

@toepkerd toepkerd commented May 22, 2026

Copy link
Copy Markdown
Collaborator

Description

Changes max PPL Monitor name length from 30 to 100. No PPL Alerting functionality is affected.

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff.
  • Public documentation issue/PR created.

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.

Signed-off-by: Dennis Toepker <toepkerd@amazon.com>
@github-actions

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 No relevant tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ No major issues detected

@github-actions

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Verify database schema compatibility

Increasing the max name length from 30 to 100 characters may cause database schema
issues if the underlying storage has a fixed column width of 30 characters. Verify
that all database columns storing monitor, trigger, and notification action names
can accommodate 100 characters to prevent truncation or insertion failures.

src/main/kotlin/org/opensearch/commons/alerting/model/Monitor.kt [331]

 const val ALERTING_MAX_NAME_LENGTH = 100 // max length of any name for monitors, triggers, notif actions, etc
+// Note: Ensure database schema supports this length increase
Suggestion importance[1-10]: 7

__

Why: This is a valid concern about potential database schema compatibility issues when increasing ALERTING_MAX_NAME_LENGTH from 30 to 100 characters. However, it's a verification request rather than identifying a concrete bug, so it doesn't warrant the highest score. The suggestion correctly identifies a potential integration issue that should be checked.

Medium

@TackAdam TackAdam left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM, thanks for update.

@toepkerd
toepkerd merged commit 851f49b into opensearch-project:main May 26, 2026
11 checks passed
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