Skip to content
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

chore: account for early exit and stop timer #1895

Merged
merged 1 commit into from
Mar 6, 2025

Conversation

cmwylie19
Copy link
Contributor

Description

There are several places in the mutate process where the request will exit early before the final return of the response. This leads to metricCollector incrementing the count for mutation timeouts which is inaccurate. We need to stop the timer to ensure we have accurate metrics.

Related Issue

Fixes #1894

Relates to #

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Other (security config, docs update, etc)

Checklist before merging

@cmwylie19 cmwylie19 requested a review from a team as a code owner March 5, 2025 14:33
Copy link
Contributor

@samayer12 samayer12 left a comment

Choose a reason for hiding this comment

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

Relates to #1897

@samayer12 samayer12 added this pull request to the merge queue Mar 6, 2025
Merged via the queue into main with commit aa0e4a1 Mar 6, 2025
77 checks passed
@samayer12 samayer12 deleted the account_for_early_exit_mutate_processor branch March 6, 2025 15:15
github-merge-queue bot pushed a commit that referenced this pull request Mar 6, 2025
## Description

This is very hard to test because the Capability does not have actual
bindings. Because of that the test stops
[here](https://github.com/defenseunicorns/pepr/blob/4e8549a43bb911072bd0316f7dace7172e5eb1a0/src/lib/processors/mutate-processor.ts#L197)
and returns early. The only meaningful tests I could do was to ensure
the timer start and stop were called.

There is already too much mocking in this file I cannot get any
assertions to pass for base64 calls here.

**UPDATE**: was able to validate the first decodeData was called by
shuffling files around and mocking the call in `mutate-process.test.ts`.
The rest of the test is untestable due to no bindings in the
capabilities

Needs #1895 to pass.

## Related Issue

Fixes #1892 
<!-- or -->
Relates to #

## Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Other (security config, docs update, etc)

## Checklist before merging
- [ ] Unit,
[Journey](https://github.com/defenseunicorns/pepr/tree/main/journey),
[E2E Tests](https://github.com/defenseunicorns/pepr-excellent-examples),
[docs](https://github.com/defenseunicorns/pepr/tree/main/docs),
[adr](https://github.com/defenseunicorns/pepr/tree/main/adr) added or
updated as needed
- [ ] [Contributor Guide
Steps](https://docs.pepr.dev/main/contribute/#submitting-a-pull-request)
followed

---------

Signed-off-by: Case Wylie <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

Mutate Processor mutation_timeouts metrics does not account for early exit
2 participants