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 #190] Add httpjson tags support #275

Closed
wants to merge 3 commits into from
Closed
Changes from 1 commit
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
Next Next commit
[Fix #190] Add httpjson tags support
palkan committed Oct 17, 2015
commit 763ff0be81c32acef7cc95b808bda0d85c7d6835
17 changes: 16 additions & 1 deletion plugins/httpjson/httpjson.go
Original file line number Diff line number Diff line change
@@ -22,6 +22,7 @@ type Service struct {
Name string
Servers []string
Method string
Tags []string
Parameters map[string]string
}

@@ -61,6 +62,12 @@ var sampleConfig = `
# HTTP method to use (case-sensitive)
method = "GET"

# List of tag names to extract from server response
tags = [
Copy link
Contributor

Choose a reason for hiding this comment

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

comment this by default

"my_tag_1",
"my_tag_2"
]

# HTTP parameters (all values must be strings)
[httpjson.services.parameters]
event_type = "cpu_spike"
@@ -126,7 +133,7 @@ func (h *HttpJson) gatherServer(acc plugins.Accumulator, service Service, server
return err
}

var jsonOut interface{}
var jsonOut map[string]interface{}
Copy link
Contributor

Choose a reason for hiding this comment

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

Won't this break the entire plugin if there is a non-string key in the top-level of the JSON?

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, response can contain JSON array. We should have a test case for this)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, this plugin wasn't aware of array JSON. I think this should go to another PR (along with support for string values, do you know why the plugin supports only floats, btw?).

Copy link
Contributor

Choose a reason for hiding this comment

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

@palkan supporting strings could become very complicated, there tends to be a lot of metadata in JSON that users probably don't want. Also storing strings as InfluxDB values has bad compression and isn't very useful.

if err = json.Unmarshal([]byte(resp), &jsonOut); err != nil {
return errors.New("Error decoding JSON response")
}
@@ -135,6 +142,14 @@ func (h *HttpJson) gatherServer(acc plugins.Accumulator, service Service, server
"server": serverURL,
}

for _, tag := range service.Tags {
switch v := jsonOut[tag].(type) {
case string:
tags[tag] = v
Copy link
Contributor

Choose a reason for hiding this comment

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

indentation, should all be tabs in Go

}
delete(jsonOut, tag)
}

processResponse(acc, service.Name, tags, jsonOut)
return nil
}
41 changes: 39 additions & 2 deletions plugins/httpjson/httpjson_test.go
Original file line number Diff line number Diff line change
@@ -28,6 +28,14 @@ const validJSON = `
}
}`

const validJSONTags = `
{
"value": 15,
"role": "master",
"build": "123"
}`


const invalidJSON = "I don't think this is JSON"

const empty = ""
@@ -87,15 +95,19 @@ func genMockHttpJson(response string, statusCode int) *HttpJson {
},
Service{
Servers: []string{
"http://server1.example.com/metrics/",
"http://server2.example.com/metrics/",
"http://server3.example.com/metrics/",
"http://server4.example.com/metrics/",
},
Name: "other_webapp",
Method: "POST",
Parameters: map[string]string{
"httpParam1": "12",
"httpParam2": "the second parameter",
},
Tags: []string{
"role",
"build",
},
},
},
}
@@ -185,3 +197,28 @@ func TestHttpJsonEmptyResponse(t *testing.T) {
assert.Equal(t, len(strings.Split(err.Error(), "\n")), 4)
assert.Equal(t, 0, len(acc.Points))
}

// Test that the proper values are ignored or collected
func TestHttpJson200Tags(t *testing.T) {
httpjson := genMockHttpJson(validJSONTags, 200)

var acc testutil.Accumulator
err := httpjson.Gather(&acc)
require.NoError(t, err)

assert.Equal(t, 4, len(acc.Points))

for _, service := range httpjson.Services {
if service.Name == "other_webapp" {
for _, srv := range service.Servers {
require.NoError(t,
acc.ValidateTaggedValue(
fmt.Sprintf("%s_value", service.Name),
15.0,
map[string]string{"server": srv, "role": "master", "build": "123"},
),
)
}
}
}
}