Skip to content

Propagate request start/end lazily#1687

Merged
danielmitterdorfer merged 2 commits intoelastic:masterfrom
danielmitterdorfer:ctx-lazy-propagate
Apr 17, 2023
Merged

Propagate request start/end lazily#1687
danielmitterdorfer merged 2 commits intoelastic:masterfrom
danielmitterdorfer:ctx-lazy-propagate

Conversation

@danielmitterdorfer
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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