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 input for receiving papertrail webhooks #2038

Merged
merged 5 commits into from
Apr 17, 2017
Merged

Conversation

rossmcdonald
Copy link
Contributor

@rossmcdonald rossmcdonald commented Nov 11, 2016

Required for all PRs:

  • CHANGELOG.md updated (we recommend not updating this until the PR has been approved by a maintainer)
  • Sign CLA (if not already signed)
  • README.md updated (if adding a new plugin)

Copy link
Contributor

@francois2metz francois2metz left a comment

Choose a reason for hiding this comment

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

Looks good!
You may also update the root README.me and plugins/inputs/webhooks/README.md to add a link to plugins/inputs/webhooks/papertrail/README.md.


## Events

See [here](http://help.papertrailapp.com/kb/how-it-works/web-hooks/#callback) for full documentation.
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer links without name here. You could make the full sentence clickable, such as [See the full documentation](http://help.papertrailapp.com/kb/how-it-works/web-hooks/#callback).

Events from Papertrail come in two forms:

* The event-based callback (shown in the example
[here](http://help.papertrailapp.com/kb/how-it-works/web-hooks/#callback)):
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here: shown in the exampleclickable

@nhaugo nhaugo requested a review from danielnelson April 4, 2017 20:46
@nhaugo nhaugo added this to the 1.3.0 milestone Apr 4, 2017
@nhaugo
Copy link
Contributor

nhaugo commented Apr 4, 2017

@danielnelson can you take a look at this

When an event is received, any point will look similar to:

```
papertrail,host=myserver.example.com,event=saved_search_name count=3 1453248892
Copy link
Contributor

Choose a reason for hiding this comment

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

Count should be an integer and the timestamp in nanoseconds


var payload Payload
// JSON payload is x-www-form-urlencoded, remove this string when unmarshaling
remove := "payload="
Copy link
Contributor

Choose a reason for hiding this comment

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

Should do real unencoding in case the JSON has special escaped characters.

type Count struct {
SourceName string `json:"source_name"`
SourceID int64 `json:"source_id"`
TimeSeries *map[int]int `json:"timeseries"`
Copy link
Contributor

Choose a reason for hiding this comment

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

int64?

payload := url.QueryEscape(sampleEventPayload)
resp := postWebhooks(pt, payload)
if resp.Code != http.StatusOK {
t.Errorf("POST new_item returned HTTP status code %v.\nExpected %v", resp.Code, http.StatusOK)
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove newlines in error

payload := url.QueryEscape(sampleCountPayload)
resp := postWebhooks(pt, payload)
if resp.Code != http.StatusOK {
t.Errorf("POST new_item returned HTTP status code %v.\nExpected %v", resp.Code, http.StatusOK)
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove newlines in error

@danielnelson
Copy link
Contributor

@francois2metz Would you like to take a second look at the updated code?

}

func (pt *PapertrailWebhook) eventHandler(w http.ResponseWriter, r *http.Request) {
if r.Method != "POST" {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if this check is really needed. This handle is already restricted to the POST method in the Register function.

@francois2metz
Copy link
Contributor

HI @danielnelson

Looks good 👍 , don't forget to update all the readme involved when adding a new webhook: the root README.me and plugins/inputs/webhooks/README.md.

@danielnelson danielnelson merged commit 70b3e76 into master Apr 17, 2017
@danielnelson danielnelson deleted the ross-papertrail-input branch April 17, 2017 20:49
vlamug pushed a commit to vlamug/telegraf that referenced this pull request May 30, 2017
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

Successfully merging this pull request may close these issues.

None yet

5 participants