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 support to parse JSON array (JSON Parser and HTTPJson) #1965

Merged
merged 1 commit into from
Nov 15, 2016

Conversation

johnrengelman
Copy link
Contributor

Required for all PRs:

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

@johnrengelman
Copy link
Contributor Author

Our use case here is calling a web endpoint that already parses some data by "environment" which is tag we'd like to put on these metrics. This allows the JSON parser to parse the same metric with variable tags.

@johnrengelman johnrengelman changed the title Add support to parse JSON array. [WIP] Add support to parse JSON array. Oct 27, 2016
@johnrengelman johnrengelman changed the title [WIP] Add support to parse JSON array. Add support to parse JSON array (JSON Parser and HTTPJson) Oct 28, 2016
sparrc
sparrc previously requested changes Nov 2, 2016

## specifies if the incoming JSON data is an array of metric data (true)
## to parse or a single object (false)
array = false
Copy link
Contributor

Choose a reason for hiding this comment

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

couldn't the parser just figure this out without a configuration option?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, probably. I'll give that a shot.

Copy link
Contributor

@sparrc sparrc Nov 3, 2016

Choose a reason for hiding this comment

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

@johnrengelman you can use the bytes package to detect it with something like this:

func isarray(buf []byte) bool {
    ia := bytes.IndexByte(buf, '[')
    ib := bytes.IndexByte(buf, '{')
    if ia > -1 && ia < ib {
        return true
    } else {
        return false
    }
}

@sparrc sparrc added this to the 1.2.0 milestone Nov 3, 2016
@johnrengelman johnrengelman force-pushed the parse-json-array branch 3 times, most recently from f1325da to f585e60 Compare November 3, 2016 19:21
@johnrengelman
Copy link
Contributor Author

@sparrc updated w/ your feedback. Good call on that. Must nicer. I guess I was initially thinking that explicit configuration would be appropriate, but I think just enforcing this convention works best since there's no other way to handle a root array.

```toml
[[inputs.exec]]
## Commands array
commands = ["/tmp/test.sh", "/usr/bin/mycollector --foo=bar"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove test.sh? I think the example will be a little clearer with just one command.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed an update for this.

```toml
[[inputs.exec]]
## Commands array
commands = [/usr/bin/mycollector --foo=bar"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing a quote.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

:sigh: and fixed.

@sparrc sparrc dismissed their stale review November 15, 2016 17:38

dgnorton will finish review

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.

3 participants