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 uint support to cratedb output #4117

Merged
merged 3 commits into from
May 8, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,15 @@ services:
- "389:389"
- "636:636"
crate:
image: crate/crate
ports:
- "4200:4200"
- "4230:4230"
command:
- crate
- -Cnetwork.host=0.0.0.0
- -Ctransport.host=localhost
- -Clicense.enterprise=false
environment:
- CRATE_HEAP_SIZE=128m
- JAVA_OPTS='-Xms256m -Xmx256m'
image: crate/crate
ports:
- "4200:4200"
- "4230:4230"
- "5432:5432"
command:
- crate
- -Cnetwork.host=0.0.0.0
- -Ctransport.host=localhost
- -Clicense.enterprise=false
environment:
- CRATE_HEAP_SIZE=128m
19 changes: 15 additions & 4 deletions plugins/outputs/cratedb/cratedb.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"encoding/binary"
"fmt"
"sort"
"strconv"
"strings"
"time"

Expand All @@ -16,6 +17,8 @@ import (
_ "github.com/jackc/pgx/stdlib"
)

const MaxInt64 = int64(^uint64(0) >> 1)

type CrateDB struct {
URL string
Timeout internal.Duration
Expand Down Expand Up @@ -115,11 +118,19 @@ func escapeValue(val interface{}) (string, error) {
switch t := val.(type) {
case string:
return escapeString(t, `'`), nil
// We don't handle uint, uint32 and uint64 here because CrateDB doesn't
// seem to support unsigned types. But it seems like input plugins don't
// produce those types, so it's hopefully ok.
case int, int32, int64, float32, float64:
case int64, float64:
return fmt.Sprint(t), nil
case uint64:
// The long type is the largest integer type in CrateDB and is the
// size of a signed int64. If our value is too large send the largest
// possible value.
if t <= uint64(MaxInt64) {
return strconv.FormatInt(int64(t), 10), nil
} else {
return strconv.FormatInt(MaxInt64, 10), nil
}
Copy link
Contributor

Choose a reason for hiding this comment

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

So the idea here is to silently truncate large numbers because CrateDB can't store them? I generally prefer to return an error in these situations, but I think there should be at least a comment explaining the reason for this code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is closely based on the behavior of the InfluxDB output but I can definitely leave a comment.

We could log an error here however in most cases the value would probably be oversized on every interval, so we would want to log only once. We can't return an error from Write() because then Telegraf would retry the metric.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK

case bool:
return strconv.FormatBool(t), nil
case time.Time:
// see https://crate.io/docs/crate/reference/sql/data_types.html#timestamp
return escapeValue(t.Format("2006-01-02T15:04:05.999-0700"))
Expand Down
8 changes: 4 additions & 4 deletions plugins/outputs/cratedb/cratedb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,12 @@ func Test_escapeValue(t *testing.T) {
{`foo`, `'foo'`},
{`foo'bar 'yeah`, `'foo''bar ''yeah'`},
// int types
{123, `123`}, // int
{int64(123), `123`},
{int32(123), `123`},
{uint64(123), `123`},
{uint64(MaxInt64) + 1, `9223372036854775807`},
{true, `true`},
{false, `false`},
// float types
{123.456, `123.456`},
{float32(123.456), `123.456`}, // floating point SNAFU
{float64(123.456), `123.456`},
// time.Time
{time.Date(2017, 8, 7, 16, 44, 52, 123*1000*1000, time.FixedZone("Dreamland", 5400)), `'2017-08-07T16:44:52.123+0130'`},
Expand Down