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

Making stripe telemetry more specific in terms of units #697

Merged
merged 1 commit into from
Nov 13, 2018

Conversation

devshorts
Copy link

Related to #696 making a minor change to make the telemetry payload more specific in terms of timing units. I'm making the payload specific that it is in milliseconds and enforcing that in the ruby time difference. This is important so we can keep cross binding payloads consistent.

r? @ob-stripe @brandur-stripe

@brandur-stripe
Copy link
Contributor

Should we also truncate the ms number to an integer as well? IMO, it's fine to have seconds represented as a float, but it starts to seem a little off to leave on the extra precision when we've explicitly converted to a specific unit.

@devshorts
Copy link
Author

devshorts commented Nov 12, 2018

@brandur yeah, that works for me, especially since other languages will probably not have this metric as a float

@devshorts
Copy link
Author

r? @brandur-stripe updated

@brandur-stripe
Copy link
Contributor

@devshorts LGTM!

Lint's failing because the newline introduced on 223 has leading indentation in it. Do you want to just take that whole line out and re-push? Thanks!

ptal @devshorts

@devshorts
Copy link
Author

@brandur-stripe totes! thanks! pushed up

@brandur-stripe brandur-stripe merged commit d2c3a55 into master Nov 13, 2018
@brandur-stripe brandur-stripe deleted the akropp-telemetry-in-ms branch November 13, 2018 00:41
@brandur-stripe
Copy link
Contributor

Released as 3.31.1.

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.

None yet

4 participants