Skip to content

Fix: Align error rate samples to request rate samples#656

Open
tejhan wants to merge 1 commit into
Azure:mainfrom
tejhan:alignPrometheusErrorSamples
Open

Fix: Align error rate samples to request rate samples#656
tejhan wants to merge 1 commit into
Azure:mainfrom
tejhan:alignPrometheusErrorSamples

Conversation

@tejhan
Copy link
Copy Markdown
Collaborator

@tejhan tejhan commented May 12, 2026

Description

The Request & Error chart joined the two Prometheus queries by array index. In scenarios where the error query returns a different # of samples or timestamps, the values would be unaligned, causing Error data to either incorrectly populate the chart or some data being dropped.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Performance improvement
  • Code refactoring
  • CI/CD changes
  • Other: **___**

Changes Made

  • Added errorByTimestamp to map return values specifically to timestamps.

Testing

  • Unit tests pass
  • Integration tests pass
  • Manual testing completed
  • Performance tested (if applicable)
  • Accessibility tested (if applicable)

Test Cases

Describe the test cases that were run:

  1. Open an existing AKSD project that contains a deployment that populates requests & error charts. Invoke a network change / load to change these values & observe data populating the chart in proper chronological timed order. (As other charts are functionally doing)

Screenshots/Videos

If applicable, add screenshots or videos to demonstrate the changes.

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published

Breaking Changes

If this is a breaking change, describe:

  • What breaks
  • How to migrate
  • Timeline for deprecation

Performance Impact

  • No performance impact
  • Performance improved
  • Performance degraded (explain why this is acceptable)

Documentation Updates

  • README.md updated
  • API documentation updated
  • Code comments updated
  • User guide updated
  • No documentation updates needed

Reviewer Notes

Any specific areas you'd like reviewers to focus on or questions you have.

Signed-off-by: tejhan-diallo <tejhan.diallo@gmail.com>
@tejhan tejhan self-assigned this May 12, 2026
Copilot AI review requested due to automatic review settings May 12, 2026 01:00
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes incorrect Request & Error chart plotting by aligning Prometheus error-rate samples to request-rate samples by timestamp instead of array index, preventing misaligned/missing points when the two queries return different sample sets.

Changes:

  • Build a timestamp-keyed map of error-rate samples (errorByTimestamp) to align error values to request timestamps.
  • Update request/error series construction to use timestamp lookup with a 0 fallback when an error sample is missing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +402 to 414
// Create a map of error values keyed by timestamp for correct alignment with
// request values, since Prometheus may return different timestamps for each query.
const errorByTimestamp = new Map<number, number>();
if (errorResults.length > 0 && errorResults[0].values) {
errorResults[0].values.forEach((v: [number, string]) => {
errorByTimestamp.set(v[0], safeParseFloat(v[1]));
});
}
requestResults[0].values.forEach((v: [number, string]) => {
const timestamp = new Date(v[0] * 1000).toLocaleTimeString();
const requestRate = safeParseFloat(v[1]);
const errorRate =
errorResults.length > 0 && errorResults[0].values[idx]
? safeParseFloat(errorResults[0].values[idx][1])
: 0;
const errorRate = errorByTimestamp.get(v[0]) ?? 0;

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.

2 participants