Skip to content

add node role to metrics metadata - #1104

Merged
rishabh6788 merged 1 commit into
opensearch-project:mainfrom
rishabh6788:main
Aug 10, 2026
Merged

add node role to metrics metadata#1104
rishabh6788 merged 1 commit into
opensearch-project:mainfrom
rishabh6788:main

Conversation

@rishabh6788

Copy link
Copy Markdown
Collaborator

Description

add node role to metrics metadata to easily analyze metrics for clusters with multiple node roles.

Issues Resolved

[List any issues this PR will resolve]

Testing

  • New functionality includes testing

[Describe how this change was tested]


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: Rishabh Singh <sngri@amazon.com>
@github-actions

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ Recommended focus areas for review

Possible KeyError

node_stats["roles"] is accessed unconditionally. If the node stats sample does not include a roles field (e.g., in older cluster versions or unexpected API responses), this will raise a KeyError and break metrics recording. Consider using node_stats.get("roles") with a sensible default to make this more defensive.

roles = node_stats["roles"]

@github-actions

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Safely access roles field to prevent KeyError

Directly indexing node_stats["roles"] will raise a KeyError for nodes/clusters that
do not return the roles field (e.g., older OpenSearch versions or restricted
permissions). Use .get("roles", []) to safely handle missing keys and avoid breaking
metrics collection.

osbenchmark/telemetry.py [967]

 node_name = node_stats["name"]
-roles = node_stats["roles"]
+roles = node_stats.get("roles", [])
 metrics_store_meta_data = {
     "cluster": self.cluster_name,
     "node_name": node_name,
     "roles": roles
 }
Suggestion importance[1-10]: 6

__

Why: Using .get("roles", []) is a reasonable defensive measure against missing roles keys in node stats responses, though the roles field is typically present in OpenSearch node stats responses.

Low

@rishabh6788
rishabh6788 merged commit 0f054da into opensearch-project:main Aug 10, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants