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

refactor(520): Change histogram units from nanos to fractional secs #522

Merged
merged 1 commit into from
Nov 4, 2023

Conversation

joshua-spacetime
Copy link
Collaborator

Fixes #520.

The default prometheus histogram buckets are meant to be interpreted as f64 seconds. Because previously we were using nanosecond units, all observations were being placed in the last (+inf) bucket. This change was made to avoid modifying the default bucket specification.

Description of Changes

API and ABI

  • This is a breaking change to the module ABI
  • This is a breaking change to the module API
  • This is a breaking change to the ClientAPI
  • This is a breaking change to the SDK API

If the API is breaking, please state below what will break

Expected complexity level and risk

How complicated do you think these changes are? Grade on a scale from 1 to 5,
where 1 is a trivial change, and 5 is a deep-reaching and complex change.

This complexity rating applies not only to the complexity apparent in the diff,
but also to its interactions with existing and future code.

If you answered more than a 2, explain what is complex about the PR,
and what other components it interacts with in potentially concerning ways.

@joshua-spacetime joshua-spacetime force-pushed the joshua/refactor/520/histogram-units branch from 4170afa to 5e6240c Compare November 3, 2023 19:12
Fixes #520.

The default prometheus histogram buckets are meant to be interpreted as f64 seconds.
Because previously we were using nanosecond units,
all observations were being placed in the last (+inf) bucket.
This change was made to avoid modifying the default bucket specification.
@joshua-spacetime joshua-spacetime force-pushed the joshua/refactor/520/histogram-units branch from 5e6240c to 9bd9e42 Compare November 4, 2023 00:42
Copy link
Contributor

@cloutiertyler cloutiertyler left a comment

Choose a reason for hiding this comment

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

LGTM

@joshua-spacetime joshua-spacetime merged commit 82fe0f7 into master Nov 4, 2023
@joshua-spacetime joshua-spacetime deleted the joshua/refactor/520/histogram-units branch November 4, 2023 01:19
kulakowski pushed a commit that referenced this pull request Nov 7, 2023
…522)

Fixes #520.

The default prometheus histogram buckets are meant to be interpreted as f64 seconds.
Because previously we were using nanosecond units,
all observations were being placed in the last (+inf) bucket.
This change was made to avoid modifying the default bucket specification.
kulakowski pushed a commit that referenced this pull request Nov 7, 2023
…522)

Fixes #520.

The default prometheus histogram buckets are meant to be interpreted as f64 seconds.
Because previously we were using nanosecond units,
all observations were being placed in the last (+inf) bucket.
This change was made to avoid modifying the default bucket specification.
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.

Change units of histogram metrics from nanoseconds to seconds
2 participants