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

Propagate request start/end lazily #1687

Merged

Conversation

danielmitterdorfer
Copy link
Member

With this commit we only propagate request start and end time to the parent context if there actually is one. Previously we unconditionally propagated the request context which could lead to misleading errors when there was an open request context but no request was issued. This can happen when a mandatory parameter is missing and the runner was throwing exception. That exception would get masked by a KeyError because no request start was present yet and thus obfuscate the root cause.

With this commit we only propagate request start and end time to the
parent context if there actually is one. Previously we unconditionally
propagated the request context which could lead to misleading errors
when there was an open request context but no request was issued. This
can happen when a mandatory parameter is missing and the runner was
throwing exception. That exception would get masked by a `KeyError`
because no request start was present yet and thus obfuscate the root
cause.
@danielmitterdorfer danielmitterdorfer added bug Something's wrong :Load Driver Changes that affect the core of the load driver such as scheduling, the measurement approach etc. labels Mar 24, 2023
@danielmitterdorfer danielmitterdorfer added this to the 2.8.0 milestone Mar 24, 2023
@danielmitterdorfer danielmitterdorfer self-assigned this Mar 24, 2023
@dliappis dliappis requested a review from pquentin March 24, 2023 10:40
Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

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

Awesome, this should really help with debugging errors!

@danielmitterdorfer danielmitterdorfer merged commit b39af39 into elastic:master Apr 17, 2023
@danielmitterdorfer danielmitterdorfer deleted the ctx-lazy-propagate branch April 17, 2023 15:07
@danielmitterdorfer
Copy link
Member Author

Thanks for the review Quentin!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something's wrong :Load Driver Changes that affect the core of the load driver such as scheduling, the measurement approach etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants