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

Implement a log file for callback timings #65

Merged
merged 2 commits into from
Jun 21, 2019
Merged

Implement a log file for callback timings #65

merged 2 commits into from
Jun 21, 2019

Conversation

pboucher
Copy link

No description provided.

Copy link

@victoriagrey victoriagrey left a comment

Choose a reason for hiding this comment

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

LGTM. Tested locally and everything looks in order.

# to disable.
# timing_log: on
timing_log: off

Choose a reason for hiding this comment

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

might be good to include an example line here with explanation of the different parts? not 100% necessary, but possibly helpful esp. for new users.

Copy link
Author

Choose a reason for hiding this comment

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

I'll document in the wiki.

end_time = datetime.datetime.now(SG_TIMEZONE.local)
duration = end_time - start_time
delay = start_time - event['created_at']
msg_format = "event_id=%d created_at=%s callback=%s start=%s end=%s duration=%s error=%s delay=%s"

Choose a reason for hiding this comment

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

i know that this is coming from a different place in the code -- but my only nitpick, and this is my python ignorance, is that i can't figure out why the timestamp has a comma for the decimals place but the rest of the timing info uses a period.

Copy link
Author

@pboucher pboucher Jun 21, 2019

Choose a reason for hiding this comment

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

This is the logging Formatter using asctime. I probably should have used ISO but at this point, I don't want to break compatibility if people are parsing it to throw logs in some aggregation system.

The datetimes in the message use ISO format.

I've modified the output format of the durations to properly represent days if present in a more parseable format.

Copy link

@plongitudes plongitudes left a comment

Choose a reason for hiding this comment

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

LGTM!

@pboucher pboucher merged commit 60ee32e into master Jun 21, 2019
@pboucher pboucher deleted the timing_logs branch June 21, 2019 23:20
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