Skip to content

Conversation

@Asaf-m
Copy link

@Asaf-m Asaf-m commented Dec 13, 2020

No description provided.

@Asaf-m Asaf-m changed the title tox: package only against python 3.7. influx reporter: make reporting precision configurable. Dec 13, 2020
@Asaf-m Asaf-m force-pushed the feature/change_influx_reporter_to_milisecond_precision branch from 89dcf5e to 0a51875 Compare December 13, 2020 16:12
@Asaf-m
Copy link
Author

Asaf-m commented Dec 13, 2020

Here's an example of difference that can result from using higher precision. Below are the Ad performance saver steps monitoring graphs:
Before:
Screen Shot 2020-12-13 at 18 57 33
After (with millisecond precision):
Screen Shot 2020-12-13 at 18 58 51

Note that even in the "after" the graphs jump from 3(fetching) to 5(saving) skipping 4(processing). That's because the processing is so fast that it falls under the same millisecond. Using micro/nano second precision won't help as Grafana is hardcoded configured to millisecond precision (source).

Copy link
Author

@Asaf-m Asaf-m Dec 13, 2020

Choose a reason for hiding this comment

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

Note that the old code was rounding the time (same for events time at line 127). I thought this is an unexpected behavior so I removed the rounding part. In other words, for "seconds" precision for example, the code drops the milliseconds data.

@Asaf-m Asaf-m requested a review from Shimi December 13, 2020 17:08
@Shimi Shimi assigned Asaf-m and unassigned Shimi Dec 16, 2020
@Asaf-m Asaf-m force-pushed the feature/change_influx_reporter_to_milisecond_precision branch from 0a51875 to 3d1544e Compare December 16, 2020 11:21
Breaking changes in this version: supporting
only python 3.7.
@Asaf-m Asaf-m merged commit 0b7507d into master Dec 17, 2020
@Asaf-m Asaf-m deleted the feature/change_influx_reporter_to_milisecond_precision branch December 17, 2020 08:13
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