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

fix: make promises call scope.end() in measureFn #1

Merged
merged 3 commits into from
Aug 31, 2022

Conversation

andresusanto
Copy link
Contributor

Hi @maraisr, I've found a little issue where .end() is not being called by measureFn when we're measuring promises. It makes spans missing the end time and hence duration.

@maraisr
Copy link
Owner

maraisr commented Aug 31, 2022

hi @andresusanto 👋🏻 thanks for this! Wonder if we should write a test for this, probably should have caught it sooner.

@andresusanto
Copy link
Contributor Author

andresusanto commented Aug 31, 2022

hi @andresusanto 👋🏻 thanks for this! Wonder if we should write a test for this, probably should have caught it sooner.

sounds good @maraisr, I will add some additional tests to catch this behaviour

@maraisr
Copy link
Owner

maraisr commented Aug 31, 2022

if this is blocking ya'll, ill merge and release, and loop back to tests after? What say you?

@andresusanto
Copy link
Contributor Author

if this is blocking ya'll, ill merge and release, and loop back to tests after? What say you?

I've added the tests @maraisr, should be able to cover the edge case, cheers 🙌

image

@maraisr maraisr merged commit 09abab6 into maraisr:main Aug 31, 2022
@andresusanto
Copy link
Contributor Author

thanks for merging and releasing @maraisr !! 🙌

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.

None yet

2 participants