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: skip spans with negative duration #16

Merged
merged 1 commit into from
Sep 23, 2022
Merged

Conversation

srikanthccv
Copy link
Member

No description provided.

@ankitnayan ankitnayan merged commit d12fe28 into main Sep 23, 2022
@ankitnayan ankitnayan deleted the clock-drift-issue branch September 23, 2022 08:17
@makeavish
Copy link
Member

Why don't we use uint64 for latency, we are doing the same for traces

@srikanthccv
Copy link
Member Author

The traces logic suffers from unsigned integer overflow issues. Try setting the end time before start and see what does durationNano contain.

@makeavish
Copy link
Member

Oh, I assumed negative integers are converted to 0 when converting uint64.
Will update traces logic to set durationNano zero for negative cases, should we do same in metrics?

@srikanthccv
Copy link
Member Author

I believe we are not doing that anywhere else in the metrics. This was the only place and we now skip such spans from including in the metrics.

@makeavish
Copy link
Member

@ankitnayan What would you suggest? Should we convert spans with negative duration to zero duration for traces and metrics?

@srikanthccv
Copy link
Member Author

I think we should convert and keep zero value for spans otherwise it produces an incorrect result for any calc made on the durationNano column. We can't also skip because it breaks the trace.

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