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.

Sign up for GitHub

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

[Instrumental] Underscore metric name output #1607

Merged
merged 24 commits into from
Aug 30, 2016
Merged

[Instrumental] Underscore metric name output #1607

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
a27d5ee
separate hello and authenticate functions, force connection close at …
janxious Jun 21, 2016
05a0186
update changelog, though this will need to be updated again to merge …
janxious Jun 21, 2016
298d928
bump instrumental agent version
janxious Jun 21, 2016
b64d5f7
fix test to deal with better better connect/reconnect logic and chang…
janxious Jun 23, 2016
149dea9
Update CHANGELOG.md
janxious Jun 23, 2016
bb35a5a
go fmt
janxious Jun 25, 2016
b4c8ef6
Split out Instrumental tests for invalid metric and value.
jqr Jun 27, 2016
66dc5a4
Ensure nothing remains on the wire after final test.
jqr Jun 27, 2016
eb9744d
Force valid metric names by replacing invalid parts with underscores.
jqr Jun 27, 2016
227ffbb
Multiple invalid characters being joined into a single udnerscore.
jqr Jun 27, 2016
42f1a14
Adjust comment to what happens.
jqr Jun 27, 2016
3d0f5d2
undo split hello and auth commands, to reduce roundtrips
janxious Jun 27, 2016
dec9535
Merge pull request #1 from Instrumental/connect-disconnect-logic
jqr Jun 27, 2016
8d75740
Merge pull request #2 from Instrumental/underscore-metric-names
jqr Jun 27, 2016
202022d
Split out Instrumental tests for invalid metric and value.
jqr Jun 27, 2016
bfe5246
Ensure nothing remains on the wire after final test.
jqr Jun 27, 2016
59a9e7c
Force valid metric names by replacing invalid parts with underscores.
jqr Jun 27, 2016
b21c80d
Multiple invalid characters being joined into a single udnerscore.
jqr Jun 27, 2016
d62c27a
add an entry to CHANGELOG for easy merging upstream
janxious Aug 8, 2016
3ad34b8
go fmt variable alignment
janxious Aug 9, 2016
269a10c
minor changes to include latest instrumental plugin commits
mediocretes Aug 18, 2016
4918023
Merge branch 'master' of github.com:influxdata/telegraf into isd
mediocretes Aug 18, 2016
b4f2550
remove some bugfixes from changelog which now more properly are in a …
mediocretes Aug 18, 2016
21b9b04
remove headers and whitespace should should have been removed with th…
mediocretes Aug 18, 2016
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ consistent with the behavior of `collection_jitter`.
- [#1323](https://github.com/influxdata/telegraf/issues/1323): Processes plugin: fix potential error with /proc/net/stat directory.
- [#1322](https://github.com/influxdata/telegraf/issues/1322): Fix rare RHEL 5.2 panic in gopsutil diskio gathering function.
- [#1586](https://github.com/influxdata/telegraf/pull/1586): Remove IF NOT EXISTS from influxdb output database creation.
- [#1607](https://github.com/influxdata/telegraf/pull/1607): Massage metric names in Instrumental output plugin
- [#1600](https://github.com/influxdata/telegraf/issues/1600): Fix quoting with text values in postgresql_extensible plugin.
- [#1425](https://github.com/influxdata/telegraf/issues/1425): Fix win_perf_counter "index out of range" panic.
- [#1634](https://github.com/influxdata/telegraf/issues/1634): Fix ntpq panic when field is missing.
Expand Down
16 changes: 13 additions & 3 deletions plugins/outputs/instrumental/instrumental.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ const (
)

var (
StatIncludesBadChar = regexp.MustCompile("[^[:alnum:][:blank:]-_.]")
ValueIncludesBadChar = regexp.MustCompile("[^[:digit:].]")
MetricNameReplacer = regexp.MustCompile("[^-[:alnum:]_.]+")
)

var sampleConfig = `
Expand Down Expand Up @@ -131,8 +132,17 @@ func (i *Instrumental) Write(metrics []telegraf.Metric) error {
}

for _, stat := range stats {
if !StatIncludesBadChar.MatchString(stat) {
points = append(points, fmt.Sprintf("%s %s", metricType, stat))
// decompose "metric.name value time"
splitStat := strings.SplitN(stat, " ", 3)
metric := splitStat[0]
value := splitStat[1]
time := splitStat[2]

// replace invalid components of metric name with underscore
clean_metric := MetricNameReplacer.ReplaceAllString(metric, "_")

if !ValueIncludesBadChar.MatchString(value) {
Copy link
Contributor

Choose a reason for hiding this comment

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

do we still need to be checking this? how would it ever be true if we already replaced the bad characters above?

Copy link
Contributor

Choose a reason for hiding this comment

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

The above cleans up the metric name, this line rejects bad values. If someone sends a name with bad characters, we just turn them to underscores, but if someone sends a bad value, we simply drop it.

I think the bad value that triggered this change was a time (as in HH:MM).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sparrc bumps

points = append(points, fmt.Sprintf("%s %s %s %s", metricType, clean_metric, value, time))
} else if i.Debug {
log.Printf("Unable to send bad stat: %s", stat)
}
Expand Down
22 changes: 18 additions & 4 deletions plugins/outputs/instrumental/instrumental_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,21 +49,28 @@ func TestWrite(t *testing.T) {
map[string]interface{}{"value": float64(3.14)},
time.Date(2010, time.November, 10, 23, 0, 0, 0, time.UTC),
)
// We will drop metrics that simply won't be accepted by Instrumental
// We will modify metric names that won't be accepted by Instrumental
m4, _ := telegraf.NewMetric(
"bad_metric_name",
map[string]string{"host": "192.168.0.1:8888::123", "metric_type": "counter"},
map[string]interface{}{"value": 1},
time.Date(2010, time.November, 10, 23, 0, 0, 0, time.UTC),
)
// We will drop metric values that won't be accepted by Instrumental
m5, _ := telegraf.NewMetric(
"bad_values",
map[string]string{"host": "192.168.0.1", "metric_type": "counter"},
map[string]interface{}{"value": "\" 3:30\""},
time.Date(2010, time.November, 10, 23, 0, 0, 0, time.UTC),
)
m5, _ := telegraf.NewMetric(
m6, _ := telegraf.NewMetric(
"my_counter",
map[string]string{"host": "192.168.0.1", "metric_type": "counter"},
map[string]interface{}{"value": float64(3.14)},
time.Date(2010, time.November, 10, 23, 0, 0, 0, time.UTC),
)

metrics = []telegraf.Metric{m3, m4, m5}
metrics = []telegraf.Metric{m3, m4, m5, m6}
i.Write(metrics)

wg.Wait()
Expand Down Expand Up @@ -101,8 +108,15 @@ func TCPServer(t *testing.T, wg *sync.WaitGroup) {

data3, _ := tp.ReadLine()
assert.Equal(t, "increment my.prefix.192_168_0_1.my_histogram 3.14 1289430000", data3)

data4, _ := tp.ReadLine()
assert.Equal(t, "increment my.prefix.192_168_0_1.my_counter 3.14 1289430000", data4)
assert.Equal(t, "increment my.prefix.192_168_0_1_8888_123.bad_metric_name 1 1289430000", data4)

data5, _ := tp.ReadLine()
assert.Equal(t, "increment my.prefix.192_168_0_1.my_counter 3.14 1289430000", data5)

data6, _ := tp.ReadLine()
assert.Equal(t, "", data6)

conn.Close()
}