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

uint64 support #3946

Merged
merged 1 commit into from
Mar 28, 2018
Merged

uint64 support #3946

merged 1 commit into from
Mar 28, 2018

Conversation

ragzilla
Copy link
Contributor

@ragzilla ragzilla commented Mar 28, 2018

Didn't see an appropriate README to update.

Pull adds uint64 support for InfluxDB line protocols (in and out), and to telegraf/metric. telegraf metric uint64 support is gated behind a build flag until infludata/influxdb mainline uint64 is generally available to ensure behavior is consistent with how telegraf has behaved in the past.

That will likely be a breaking change in the future unless the API for AddField and convertField are extended so the caller can specify whether or not uint64 storage and, by extension output, is desired.

Required for all PRs:

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

@ragzilla
Copy link
Contributor Author

@danielnelson went with the influxdb approach of gating the feature behind a build flag.

@@ -16,6 +16,12 @@ func Metric(v telegraf.Metric, err error) telegraf.Metric {
return v
}

const (
Uint64Overflow uint64 = 9223372036854775808
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be larger than Uint64Max? We need to test 18446744073709551616.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was to verify Int64Max+1 properly typed to uint64. If we attempt to define a const that’s Uint64Max+1 it’d error on compile.

@danielnelson danielnelson merged commit a320f91 into influxdata:master Mar 28, 2018
@danielnelson danielnelson added this to the 1.6.0 milestone Mar 28, 2018
@danielnelson danielnelson added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Mar 28, 2018
@danielnelson
Copy link
Contributor

I did some follow-up work, if you can review: #3948

maxunt pushed a commit that referenced this pull request Jun 26, 2018
@ragzilla ragzilla deleted the uint64 branch January 30, 2019 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants