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

fix(metrics): :impl Atomic<u64> for AtomicU64 #189

Closed
wants to merge 1 commit into from

Conversation

koushiro
Copy link
Contributor

fix the error when using Gauge<u64, AtomicU64> type, revert the impl Atomic<u64> fro AtomicU64 from #105

fix the error when using Gauge<u64, AtomicU64> type,
revert the `impl Atomic<u64> fro AtomicU64` from #105

Signed-off-by: koushiro <koushiro.cqx@gmail.com>
@mxinden
Copy link
Member

mxinden commented Feb 28, 2024

If I recall correctly, I removed this implementation, given that the protobuf definition does not support u64, but only i64. With your patch, how would one encode a u64::MAX in a protobuf?

https://github.com/OpenObservability/OpenMetrics/blob/1386544931307dff279688f332890c31b6c5de36/proto/openmetrics_data_model.proto#L100-L107

@koushiro
Copy link
Contributor Author

Ok, If the protobuf definition does not support u64, why not add impl Atomic<u32> for AtomicU32, at least we could use Gauge<u32, AtomicU32>

@koushiro koushiro closed this Mar 1, 2024
@koushiro koushiro deleted the fix-gauge-u64 branch March 1, 2024 05:30
@mxinden
Copy link
Member

mxinden commented Mar 4, 2024

Ok, If the protobuf definition does not support u64, why not add impl Atomic<u32> for AtomicU32, at least we could use Gauge<u32, AtomicU32>

Sorry for the late reply. I am fine with adding u32 to Gauge @koushiro.

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

2 participants