Skip to content

Conversation

@michaely520
Copy link
Contributor

What changed?

Errors that occur prior to the telemetry interceptor are not captured, meaning there is no log, the request isn't counted, and the error isn't counted. I've extracted the telemetry interceptors error handling into a shared obj that I've wired into the NamespaceHandover error handling for now. An alternative approach was to move the TelemetryInterceptor earlier in the chain, but this would have caused double counting on redirects, and adding another dimension to our metrics (pre/post redirect) here to address the double counting would cause other classes of issues

As of now, other interceptors that are added prior to the TelemetryInterceptor are also at risk of this. We can wrap PreTelemetry errors and have a new interceptor handle these if we want to more generically address this issue - view my commit history for "Experimental" (which I reverted) to see an example of this.

Why?

To specifically track metrics/logs on NamespaceHandover failures.

How did you test it?

  • built
  • run locally and tested manually
  • covered by existing tests
  • added new unit test(s)
  • added new functional test(s)

Potential risks

Potential metrics loss if for some reason telemetry metric error handling is not wired correctly after the refactor.

@michaely520 michaely520 requested review from a team as code owners October 3, 2025 21:50
@michaely520 michaely520 changed the title Ns handover error Emit Error Telemetry on Namespace Handover Failure Oct 3, 2025
@michaely520 michaely520 marked this pull request as draft October 3, 2025 21:59
@michaely520 michaely520 marked this pull request as ready for review October 6, 2025 20:52
@michaely520 michaely520 enabled auto-merge (squash) October 6, 2025 21:29
@michaely520 michaely520 merged commit 49e7be8 into main Oct 6, 2025
57 checks passed
@michaely520 michaely520 deleted the ns-handover-error branch October 6, 2025 21:54
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