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

[dogstatsd] use monotonic clock source when available for timers #615

Merged
merged 1 commit into from
Jan 8, 2021

Conversation

jd
Copy link
Contributor

@jd jd commented Oct 29, 2020

This makes sure the computed time is correct by using a monotonic clock source.

@jd jd requested a review from a team as a code owner October 29, 2020 10:03
Kyle-Verhoog
Kyle-Verhoog previously approved these changes Nov 12, 2020
@jd
Copy link
Contributor Author

jd commented Jan 6, 2021

@DataDog/integrations-tools-and-libraries can I get a review on this?

@@ -13,7 +13,10 @@
# stdlib
from contextlib import contextmanager
from functools import wraps
from time import time
try:
from time import monotonic as time # type: ignore[attr-defined]
Copy link
Contributor

Choose a reason for hiding this comment

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

That's kinda problematic here, since the time is used in several places to compute the actual timestamp of a metric, and not in a difference of end - start

You'd need to separate the usages here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I totally missed that. Will update.

@jd
Copy link
Contributor Author

jd commented Jan 7, 2021

Not sure the failure are related to the PR?

@zippolyte
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@zippolyte
Copy link
Contributor

The python3 no indeed, as they fail on master too for some reason, but the py2 failure seems related as master works ok

try:
yield
finally:
end = time()
end = monotonic()
self.timing(metric_name, end - start, end, tags=tags,
Copy link
Contributor

Choose a reason for hiding this comment

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

ooh the end time is used here too as timestamp

This makes sure the computed time is correct by using a monotonic clock source.
@zippolyte
Copy link
Contributor

/azp run DataDog.datadogpy.integration

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@zippolyte zippolyte added the changelog/Fixed Fixed features results into a bug fix version bump label Jan 8, 2021
@zippolyte zippolyte changed the title fix(statsd): use monotonic clock source when available for timers [dogstatsd] use monotonic clock source when available for timers Jan 8, 2021
@zippolyte zippolyte merged commit 33c35f6 into DataDog:master Jan 8, 2021
@jd jd deleted the use-monotonic branch January 8, 2021 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog/Fixed Fixed features results into a bug fix version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants