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

Use high-resolution host language timer #641

Merged
merged 8 commits into from
Apr 25, 2022
Merged

Use high-resolution host language timer #641

merged 8 commits into from
Apr 25, 2022

Conversation

jeffkreeftmeijer
Copy link
Member

@jeffkreeftmeijer jeffkreeftmeijer commented Apr 7, 2022

Instead of falling back to the agent’s internal timer, use a high-resolution host language timer to match timings in OpenTelemetry.

@jeffkreeftmeijer jeffkreeftmeijer self-assigned this Apr 7, 2022
@backlog-helper
Copy link

backlog-helper bot commented Apr 7, 2022

✔️ All good!

New issue guide | Backlog management | Rules | Feedback

In order to set times from the host language, instead of relying on the
agent's internal timing, this patch adds a hrTime (high-resolution time)
function based on the one shipped by OpenTelemetry, and functioning
through Node's built-in perf_hooks module.
Use the newly added hrTime function to set the time when starting and
completing spans, if no custom time has been passed.
Adding the new timer is marked as a patch release.
Copy link
Contributor

@unflxw unflxw left a comment

Choose a reason for hiding this comment

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

After this change, span.closeSpan (from the extension) is no longer used anywhere. Should it be removed?

@tombruijn tombruijn marked this pull request as ready for review April 8, 2022 12:05
@tombruijn
Copy link
Member

After this change, span.closeSpan (from the extension) is no longer used anywhere. Should it be removed?

It would help to avoid confusion in the future about which function to use if we remove it.

jeffkreeftmeijer and others added 5 commits April 8, 2022 15:45
Since all spans are closed with explicit timestamps (using
span.closeSpanWithTimestamp), span.closeSpan is no longer used, and can
be removed from the extension.
@jeffkreeftmeijer
Copy link
Member Author

jeffkreeftmeijer commented Apr 13, 2022

Took your suggestions, and removed the closeSpan function from the extension. Re-requested reviews.

@jeffkreeftmeijer jeffkreeftmeijer requested a review from unflxw April 13, 2022 08:53
@backlog-helper

This comment has been minimized.

7 similar comments
@backlog-helper

This comment has been minimized.

@backlog-helper

This comment has been minimized.

@backlog-helper

This comment has been minimized.

@backlog-helper

This comment has been minimized.

@backlog-helper

This comment has been minimized.

@backlog-helper

This comment has been minimized.

@backlog-helper
Copy link

While performing the daily checks some issues were found with this Pull Request.


New issue guide | Backlog management | Rules | Feedback

@jeffkreeftmeijer jeffkreeftmeijer merged commit fc9f2fd into main Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants