Skip to content

Conversation

@rapha
Copy link

@rapha rapha commented Jun 9, 2025

This is very rudimentary so far. Also currently only int64 values are supported for metrics.

typo fix

Co-authored-by: Josh Miller <notjoshmiller@gmail.com>
= GaugeKind
| SumKind
{ monotonic :: Monotonicity
, aggregation :: AggregationTemporality
Copy link
Contributor

Choose a reason for hiding this comment

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

It's generally a bad practice to add field selectors to sum type constructors, and GHC even has a warning for it: https://downloads.haskell.org/ghc/latest/docs/users_guide/using-warnings.html#ghc-flag-Wpartial-fields

I would suggest moving these fields into separate records.

Copy link
Author

Choose a reason for hiding this comment

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

Done

@velveteer
Copy link
Contributor

This needs documentation, at the very least comments that help me understand how to use these modules. Referencing the OTel spec would be great, but not necessary.

Version,
Meter,
Description,
UnitOfMeasure,
Copy link
Contributor

Choose a reason for hiding this comment

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

You probably want to export the constructors for most of the types you've defined:

For example:

Suggested change
UnitOfMeasure,
UnitOfMeasure (..),

Copy link
Author

Choose a reason for hiding this comment

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

Done


module OpenTelemetry.Metric.Point (
GaugePoint,
SumPoint,
Copy link
Contributor

Choose a reason for hiding this comment

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

Same feedback applies to this module (exports and docs)

Copy link
Author

Choose a reason for hiding this comment

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

Done

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.

2 participants