Skip to content

[Traces] Make service map max nodes and max edges values user-configurable #2472

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

Merged

Conversation

szkly
Copy link
Contributor

@szkly szkly commented Jul 10, 2025

Description

The aim of this PR is to make the max nodes and max edges (used in service map related queries) user-configurable, as they are currently (as of 3.1.0) constant values (500 and 1000 respectively), which can result in inaccurate service maps if service map indices contain more than 500 unique services or more than 1000 unique destinations per service.

I've registered these new settings alongside the existing ones:
image
The default values come from the existing (now deprecated) constants.

The following comparison shows the changes in service map, once the user updates the max node limit (observability:traceAnalyticsServiceMapMaxNodes):

  • with the default limit of 500:
image
  • with the updated limit of 5:
image

The next comparison shows the changes in service map if the user decides to update the maximum number of edges (observability:traceAnalyticsServiceMapMaxEdges) in a service map request:

  • with default limit of 1000:
image
  • with the updated limit of 1:
image

Issues Resolved

#2471

Check List

  • [] New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • Commits are signed per the DCO using --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.

@ps48 ps48 added enhancement New feature or request v1.3.20 Issues targeting release v1.3.20 v3.2.0 and removed v1.3.20 Issues targeting release v1.3.20 labels Jul 10, 2025
Copy link

codecov bot commented Jul 10, 2025

Codecov Report

Attention: Patch coverage is 72.72727% with 3 lines in your changes missing coverage. Please review.

Project coverage is 56.30%. Comparing base (c33aad8) to head (6a2d2ac).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...e_analytics/components/common/helper_functions.tsx 50.00% 2 Missing ⚠️
...ace_analytics/requests/queries/services_queries.ts 66.66% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2472   +/-   ##
=======================================
  Coverage   56.29%   56.30%           
=======================================
  Files         398      398           
  Lines       16121    16130    +9     
  Branches     4490     4490           
=======================================
+ Hits         9076     9082    +6     
- Misses       6967     6970    +3     
  Partials       78       78           
Flag Coverage Δ
dashboards-observability 56.30% <72.72%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@TackAdam
Copy link
Collaborator

Hi @szkly , LGTM. Thank you for your contribution!

I noticed the user-settings currently doesn’t have any testing coverage, did you have any plan on adding that? If not, we can take it up as a follow-up.

We will update documentation to reflect these new setting being available for adjustment on next release.

@szkly
Copy link
Contributor Author

szkly commented Jul 11, 2025

Hi @szkly , LGTM. Thank you for your contribution!

Hi @TackAdam, no problem, happy to help!

I noticed the user-settings currently doesn’t have any testing coverage, did you have any plan on adding that? If not, we can take it up as a follow-up.

No, I focused entirely on these changes, but I could take a stab at it in another PR.

We will update documentation to reflect these new setting being available for adjustment on next release.

Thanks, sounds good.

@ps48 ps48 merged commit 044e0b0 into opensearch-project:main Jul 14, 2025
26 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request v3.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants