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

Add support for more numeric types to oximeter #4091

Merged
merged 2 commits into from
Sep 16, 2023

Conversation

bnaecker
Copy link
Collaborator

  • Adds support for all integral widths to fields
  • Adds support for all integral widths to gauge and cumulative types
  • Adds support for all integral widths to the histogram types
  • Reworks methods for generating log-linear bins. This supports generating histograms with bin counts other than 10 and using bases other than 10. The handling of floating point values has also been reworked, including several stringent tests against a reference implementation for linearly-spaced values (NumPy). This is all pretty tricky given the well-known difficulties of comparing floating point types for equality, and we may want to avoid using floats for histogram supports in the future. Cautionary warnings in the type's documentation have been added in the meantime.
  • Adds support for new types to the ClickHouse client, including tests for serialization of fields and measurements to the actual tables.
  • Adds some USDT probes at the start / end of queries, including raw SQL.

@bnaecker
Copy link
Collaborator Author

Resolves #4058. Although this PR is pretty huge, it's also mostly mechanical. I've added support for: fields and measurements for all the integer types; cumulative u64 and f64 measurements; histograms for all integer types; and histograms for f32s. The meat of the PR is in the histogram code, to support bases and bins-per-magnitude other than an even 10.

This also kind of brings up an interesting question: should we support floating-point types at all? They're obviously tricky, but also very useful. Without them, a unit / resolution is basically required for anything meaningful, and those are not yet in place. There are a few interacting problems with them: difficulty of comparison; rounding errors; and ClickHouse's pretty lax formatting rules, which seem to print at most 7-8 digits of precision. I've opted to keep them so far, but I'm happy to revisit that.

@bnaecker bnaecker force-pushed the more-oximeter-integer-types branch 3 times, most recently from 21de2d3 to 97b37fb Compare September 14, 2023 21:53
- Adds support for all integral widths to fields
- Adds support for all integral widths to gauge and cumulative types
- Adds support for all integral widths to the histogram types
- Reworks methods for generating log-linear bins. This supports
  generating histograms with bin counts other than 10 and using bases
  other than 10. The handling of floating point values has also been
  reworked, including several stringent tests against a reference
  implementation for linearly-spaced values (NumPy). This is all pretty
  tricky given the well-known difficulties of comparing floating point
  types for equality, and we may want to avoid using floats for
  histogram supports in the future. Cautionary warnings in the type's
  documentation have been added in the meantime.
- Adds support for new types to the ClickHouse client, including tests
  for serialization of fields and measurements to the actual tables.
- Adds some USDT probes at the start / end of queries, including raw
  SQL.
@bnaecker bnaecker force-pushed the more-oximeter-integer-types branch from 97b37fb to 7af2469 Compare September 15, 2023 20:41
oximeter/db/src/client.rs Show resolved Hide resolved
oximeter/oximeter/src/traits.rs Outdated Show resolved Hide resolved
@bnaecker bnaecker enabled auto-merge (squash) September 16, 2023 02:47
@bnaecker bnaecker merged commit 23eee1a into main Sep 16, 2023
22 of 23 checks passed
@bnaecker bnaecker deleted the more-oximeter-integer-types branch September 16, 2023 04:07
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