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

Facade Gauge int64 vs float #279

Closed
TheAngryByrd opened this issue Feb 20, 2018 · 8 comments
Closed

Facade Gauge int64 vs float #279

TheAngryByrd opened this issue Feb 20, 2018 · 8 comments

Comments

@TheAngryByrd
Copy link

Hey there!

I'm using the logary facade in one of my libraries however I noticed the gauge takes a int64 instead of a float. I'm wonder if this an oversight or a deliberate choice as the Gauge in logary proper is a float.

@haf
Copy link
Member

haf commented Feb 20, 2018

It's based on influxdata/influxdb#3479 (comment) - the fact that int64 is less finicky to work with an better specified. The idea behind fraction was that you'd never need to round, since storing 8*2 bytes as a struct is not really a large memory footprint.

I need (and would welcome contributors to) go through the new v5 API before release; and perhaps make it more like the Facade in places.

/cc @lust4life

@TheAngryByrd
Copy link
Author

Ah ok. So would be the recommended approach for using fractional values with gauge in the facade?

@haf
Copy link
Member

haf commented Feb 21, 2018

It's most likely an oversight though. Unless you have specific requirements, like in the linked thread it should not matter whether you use float or int64, except float being easier to reason about for you as a programmer.

We can't change the public API of the Gauge just yet, but you can create another DU case with float and fraction.

That said, in Logary proper we've moved to boxed objects, to allow each thing (processing step, or target) to deal with it in its own way. The best way forward is to synchronise floats/fractions between them. Feel free to have a stab at that.

@haf
Copy link
Member

haf commented Mar 17, 2018

I think we should go with float throughout. It's good enough and much easier to work with than the scaled/annotated int64.

@haf
Copy link
Member

haf commented Mar 29, 2018

I've changed it to be a float in Facade v3 now.

@haf haf closed this as completed Mar 29, 2018
@lust4life
Copy link
Contributor

sorry for late reply, too busy at work's stuff.

Gauge in logary proper is a float. is because I use a client for prometheus which only have float/double as its gauge value, for the sake of convenience I change it from int to float . didn't expect them to be so different, learned from influxdata/influxdb#3479 .

I think we should go with float throughout. It's good enough and much easier to work with than the scaled/annotated int64.

i agree with it, since gauge is just a predefined type like any other normal object passing through message context. if an annotated int64 is need, the logging api has no limit with that.

@haf
Copy link
Member

haf commented Apr 1, 2018

Have a look at 3b70c05 — I think it strikes a balance between the Value type of before and the dichotomy we need between high-res values and floating points.

@lust4life
Copy link
Contributor

lust4life commented Apr 3, 2018

nice. make a gauge value have an explicit type (Value) can provider more constraint and convenience for pipe processing. and we have an enhanced formatting mechanism to deal with them.

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

No branches or pull requests

3 participants