Skip to content

Wired Up Reporting to Shim#2689

Merged
AndreKurait merged 8 commits intoopensearch-project:mainfrom
akshay2000:reporting-wiring
Apr 15, 2026
Merged

Wired Up Reporting to Shim#2689
AndreKurait merged 8 commits intoopensearch-project:mainfrom
akshay2000:reporting-wiring

Conversation

@akshay2000
Copy link
Copy Markdown
Collaborator

@akshay2000 akshay2000 commented Apr 13, 2026

Description

Wires the existing MetricsReceiver and ReportingSink into MultiTargetRoutingHandler so every dual-target request automatically produces a ValidationDocument with hit-count drift, latency delta, and optional request/response bodies. Reporting is enabled via a --reportingConfig JSON CLI flag and is wired into the dev sandbox dual-target shims. All new constructor params default to null for backward compatibility.

Testing

  • Unit tests

Check List

  • New functionality includes testing

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.

@akshay2000 akshay2000 had a problem deploying to migrations-cicd-require-approval April 13, 2026 08:57 — with GitHub Actions Error
@akshay2000 akshay2000 had a problem deploying to migrations-cicd-require-approval April 13, 2026 08:57 — with GitHub Actions Error
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 13, 2026

Codecov Report

❌ Patch coverage is 85.13514% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.46%. Comparing base (abe6949) to head (91de63e).
⚠️ Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
...opensearch/migrations/transform/shim/ShimMain.java 76.92% 2 Missing and 7 partials ⚠️
...ransform/shim/netty/MultiTargetRoutingHandler.java 92.30% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2689      +/-   ##
============================================
+ Coverage     73.28%   73.46%   +0.17%     
  Complexity      106      106              
============================================
  Files           721      721              
  Lines         33372    33441      +69     
  Branches       2910     2915       +5     
============================================
+ Hits          24457    24566     +109     
+ Misses         7582     7535      -47     
- Partials       1333     1340       +7     
Flag Coverage Δ
gradle 69.89% <85.13%> (+0.26%) ⬆️
node 90.97% <ø> (ø)
python 77.77% <ø> (+0.02%) ⬆️

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.

@akshay2000 akshay2000 had a problem deploying to migrations-cicd-require-approval April 13, 2026 09:10 — with GitHub Actions Error
@akshay2000 akshay2000 had a problem deploying to migrations-cicd-require-approval April 13, 2026 09:10 — with GitHub Actions Error
@akshay2000 akshay2000 had a problem deploying to migrations-cicd-require-approval April 13, 2026 09:43 — with GitHub Actions Error
@akshay2000 akshay2000 had a problem deploying to migrations-cicd-require-approval April 13, 2026 09:43 — with GitHub Actions Error
@akshay2000 akshay2000 had a problem deploying to migrations-cicd-require-approval April 13, 2026 10:45 — with GitHub Actions Error
@akshay2000 akshay2000 had a problem deploying to migrations-cicd-require-approval April 13, 2026 10:45 — with GitHub Actions Error
Signed-off-by: Akshay Zade <zadaksha@amazon.com>
Signed-off-by: Akshay Zade <zadaksha@amazon.com>
Signed-off-by: Akshay Zade <zadaksha@amazon.com>
@akshay2000 akshay2000 had a problem deploying to migrations-cicd-require-approval April 14, 2026 04:43 — with GitHub Actions Error
@akshay2000 akshay2000 had a problem deploying to migrations-cicd-require-approval April 14, 2026 04:43 — with GitHub Actions Error
Signed-off-by: Akshay Zade <zadaksha@amazon.com>
@akshay2000 akshay2000 had a problem deploying to migrations-cicd-require-approval April 14, 2026 05:24 — with GitHub Actions Error
@akshay2000 akshay2000 had a problem deploying to migrations-cicd-require-approval April 14, 2026 05:24 — with GitHub Actions Error
@akshay2000 akshay2000 marked this pull request as ready for review April 14, 2026 05:39
@akshay2000 akshay2000 had a problem deploying to migrations-cicd-require-approval April 14, 2026 14:28 — with GitHub Actions Error
@akshay2000 akshay2000 had a problem deploying to migrations-cicd-require-approval April 14, 2026 14:28 — with GitHub Actions Error
Signed-off-by: Akshay Zade <zadaksha@amazon.com>
@akshay2000 akshay2000 had a problem deploying to migrations-cicd-require-approval April 14, 2026 15:18 — with GitHub Actions Error
@akshay2000 akshay2000 had a problem deploying to migrations-cicd-require-approval April 14, 2026 15:18 — with GitHub Actions Error
Map<String, Object> responseRequestMap = hasCursorMark ? targetRequestMap : requestMap;
futures.put(name, dispatchToTarget(target, targetRequestMap, responseRequestMap, dispatchCtx));

collectTransformData(target, name, targetRequestMap,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How is metricsMap not empty at this point? I would expect the responses are async and we have to wait on futures before the responses are populated

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The metrics are coming from the request translations. For example, if a request translation notices some limitation, (like offsets not being supported by OpenSearch inside aggregation buckets) the transform will emit a metric. So the metrics will be available as soon as the request transform is run and we come back from GraalVM.

Copy link
Copy Markdown
Member

@AndreKurait AndreKurait Apr 14, 2026

Choose a reason for hiding this comment

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

The pr overview here says

so every dual-target request automatically produces a ValidationDocument 
with hit-count drift, latency delta, and optional request/response bodies.

Is this accurate?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

That is correct. (Obviously the user must pass --reportingConfig to enable this.) The list is not exhaustive. Do you see an accuracy issue here?

Signed-off-by: Andre Kurait <akurait@amazon.com>
@AndreKurait AndreKurait had a problem deploying to migrations-cicd-require-approval April 15, 2026 02:22 — with GitHub Actions Error
@AndreKurait AndreKurait had a problem deploying to migrations-cicd-require-approval April 15, 2026 02:22 — with GitHub Actions Error
@AndreKurait AndreKurait enabled auto-merge April 15, 2026 02:22
…ures

Signed-off-by: Andre Kurait <akurait@amazon.com>
@AndreKurait AndreKurait had a problem deploying to migrations-cicd-require-approval April 15, 2026 02:45 — with GitHub Actions Error
@AndreKurait AndreKurait had a problem deploying to migrations-cicd-require-approval April 15, 2026 02:45 — with GitHub Actions Error
@akshay2000 akshay2000 had a problem deploying to migrations-cicd-require-approval April 15, 2026 04:27 — with GitHub Actions Error
@akshay2000 akshay2000 had a problem deploying to migrations-cicd-require-approval April 15, 2026 04:27 — with GitHub Actions Error
Signed-off-by: Akshay Zade <zadaksha@amazon.com>
@akshay2000 akshay2000 requested a deployment to migrations-cicd-require-approval April 15, 2026 04:41 — with GitHub Actions Waiting
@akshay2000 akshay2000 requested a deployment to migrations-cicd-require-approval April 15, 2026 04:41 — with GitHub Actions Waiting
@AndreKurait AndreKurait merged commit 8bd941c into opensearch-project:main Apr 15, 2026
69 of 71 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.

3 participants